automaticmode / active_workflow

Polyglot workflows without leaving the comfort of your technology stack.
https://www.activeworkflow.org
Other
830 stars 67 forks source link

Remote Agent API: empty strings in errors or logs cause errors #7

Closed MarcelPa closed 3 years ago

MarcelPa commented 4 years ago

Hey everyone,

I have found a rather odd error that will be raised when using remote agents via the remote agent API. If the remote agent responds to a check (maybe even a receive, I did not check that) with empty strings in the errors or logs, Active Workflow will throw an error: Exception during check. Validation failed: Message can't be blank. See this gist for the more details.

I have fixed it by removing all empty strings that would be returned in my remote agent. Still, this seems a little fragile to me on the Active Workflow side.

If this behaviour is intended, do not hesitate to close this issue. It may come in handy for anyone that encounters the same problem :-)

Cheers Marcel

vidas commented 4 years ago

Hi @MarcelPa, thanks for submitting this issue.

This behavior is intended - mostly just to avoid ambiguity even if losing in ergonomics. But just to avoid surprises in the API and make necessary clarifications/changes I would like to ask for your opinion: What was the expected behavior when returning empty strings? Should they be stored as empty log entries? Ignored? Something else?

This knowledge is very important because we plan to add support for structured logs in the near future, i.e. you will be able to log arbitrary hashes (dicts? maps?). There will be multiple cases similar to the one with empty strings and proper handling should go along user expectations.

As for general Remote Agent API fragility - there is room to grow, working on it (all the changes will be backward-compatible).

Thank you! Vidas

MarcelPa commented 4 years ago

Hey @vidas , great question actually, I did not think about that.

I think I would prefer empty log entries to appear, which would have made me fixing me agent the way I did anyway. I can see that this would add ambiguity to the logs of ActiveWorkflow (especially in the case, where an ActiveWorkflow user is not equal to the developer of the remote agent). To find some middle ground: mention this behaviour in the RemoteAPI Docs and add a more explicit error message (as a Python dev, my time to find the actual problem was quite long :-D )

Structured logs sound interesting. Do you plan to add logging levels? I think that could make debugging agents in a production environment easier (again, thats a devs perspective).

Cheers Marcel

vidas commented 4 years ago

Hi @MarcelPa.

Logging levels are already partially implemented - having log and error is one aspect of that, but I agree that a user should be able to select an arbitrary log level. More work is necessary on the frontend/API side of the thing to make use of the logging level. This is very much in the plans.

Thank you! Vidas

raspberry-lef commented 3 years ago

Hello, @MarcelPa,

The Remote Agent documentation has been updated (a little while ago) to mention that empty strings are not allowed in log messages as you suggested. There is also a helper Python library now that one could use when writing agents ActiveWorkflow Agent Python.

Thank you again for submitting the issue.