robotcodedev / robotcode

RobotFramework support for Visual Studio Code
https://robotcode.io
Apache License 2.0
173 stars 14 forks source link

[BUG] Robot Code crashes with a variables file containing a Dict[str, Callable] #125

Closed mardukbp closed 1 year ago

mardukbp commented 1 year ago

Describe the bug Given test.robot

*** Settings ***
Variables    variables.py

with variables.py

def is_valid_name():
    return True

pet = {"name": is_valid_name}

Robot Code crashes with "A process in the process pool was terminated abruptly while the future was running or pending"

To Reproduce Steps to reproduce the behavior:

  1. Create the above files
  2. Open their parent directory in VS Code
  3. Wait for Robot Code to finish loading
  4. See the error

Expected behavior Robot Code should not crash.

Screenshots/ Videos None

Logs Copy the messages from VSCode "Output" view for RobotCode and RobotCode Language Server for the specific folder/workspace.

RobotCode

Activate RobotCode Extension. Try to activate python extension Python Extension is active create Language client: RobotCode Language Server mode=pipe for folder "test" trying to start Language client: RobotCode Language Server mode=pipe for folder "test" client for file:///c%3A/Users/mardukb/Desktop/test starting. client for file:///c%3A/Users/mardukb/Desktop/test running. client for file:///c%3A/Users/mardukb/Desktop/test started. refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot refresh uri file:///c%3A/Users/mardukb/Desktop/test/test.robot

RobotCode Language Server

empty

Desktop (please complete the following information):

Additional context None

d-biehl commented 1 year ago

I cannot reproduce it, does this happens everytime?

It seems that this

pet = {"name": is_valid_name}

can be a problem, because the value is a function instance and you cannot serialize a function. Is that really supposed to be a function?

d-biehl commented 1 year ago

ok, did not read it correctly you want to have a function in a dict ;-)

but why?

mardukbp commented 1 year ago

When testing REST APIs it can be useful to define the specification for the response body using a dictionary of functions. The reason for using functions is composition of reusable abstractions. And yes, I took inspiration from clojure.spec :)

But anyway, the robot executable has no problem with such a dictionary in a variables file and as a user of Robot Code I expect that what is valid for robot is valid for Robot Code.

And in any case I definitely expect Robot Code to not crash ;)

mardukbp commented 1 year ago

I have thought about it and I think Robot Code needs to serialize the exported variables in order to show the tooltip for a variable. I think for a function it is reasonable to use str(f) which produces '<function f at 0x000001DADB3EAF80>'. Using inspect.signature would produce nicer output, but that only works if the function has type hints.

d-biehl commented 1 year ago

Exactly ;-)

Or to remove not simple types like this for serialization. The main problem here is the caching mechanism. I will find a way ;-)

d-biehl commented 1 year ago

After analysing this a bit deeper, I think the best solution at the moment, is to disable the values in hints for dicts and lists because I can't predict what types are in the dict and if a dict has a dict as an element and then another dict etc it gets complicated.

Maybe a better solution will be found in the future. But it might be possible to show more type information about the variables, that's why I created an Enhancement Request #128.

Hope this fits for you?

mardukbp commented 1 year ago

Thanks a lot Daniel! I think that for dictionaries having auto-completion for the keys (as long as they are strings) would be more helpful than a tooltip. This can be used to navigate through nested dictionaries.

And I think that for both dicts and lists it would be useful to show the type in the tooltip (e.g. type(var)) so that you know how to access their elements.