Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better validation of action events #8127

Closed
5 tasks done
chdorner opened this issue Mar 5, 2021 · 2 comments · Fixed by #8730
Closed
5 tasks done

Better validation of action events #8127

chdorner opened this issue Mar 5, 2021 · 2 comments · Fixed by #8730
Assignees
Labels
area:rasa-oss/custom-actions area:rasa-oss 🎡 Anything related to the open source Rasa framework cse-issues effort:atom-squad/4 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@chdorner
Copy link
Contributor

chdorner commented Mar 5, 2021

Rasa version: Tested on 2.3.4, bug should still exist on currently latest main@eb408bb2d216de3a1cd1f72799d7b80eceeb1fd2

Rasa SDK version (if used & relevant): 2.3.1

Rasa X version (if used & relevant): 0.36.0 and later (not relevant however)

Python version: 3.8.6

Operating system (windows, osx, ...): Mac OS 11.2.1

Issue:
This came out of this Rasa X issue (see @tmbo's comment here for a summary).

The validation of action responses are quite optimistic, allowing actions to create invalid events (i.e. "entities": {} instead of the valid "entities": []), resulting on a bunch of different issues downstream in Rasa X, since so far Rasa X seems to trust that events sent over the broker are valid.

Here's a handy example repo made by @rgstephens to illustrate the issue: RasaHQ/action-execute-convo-issue. Note that this is actually a bug in the action code that should, however, be caught by Rasa OSS and have the error (invalid event) logged.

Definition of done

  • amend json schema in action_response_format_spec method to cover in detail every top level key in the top 3 most important events: UserUttered, SlotSet, ActionExecuted, while the rest 20 events can be covered on the surface, using oneOf json schema constraint (more details in Notion)
  • add event validation to server.py
  • add tests
  • submit new issue to update docs with detailed description of the schema
  • submit separate issue to tackle response validation in action.py
@chdorner chdorner added type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. area:rasa-oss 🎡 Anything related to the open source Rasa framework area:rasa-oss/custom-actions labels Mar 5, 2021
@tmbo
Copy link
Member

tmbo commented Mar 5, 2021

Same validation should be applied to events getting send to the http server of rasa open source (eg in the append events endpoint)

@b-quachtran
Copy link
Contributor

@TyDunn A customer is reporting that this issue is affecting them, any chance this can get bumped up in priority?

@TyDunn TyDunn added the effort:atom-squad/4 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. label May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss/custom-actions area:rasa-oss 🎡 Anything related to the open source Rasa framework cse-issues effort:atom-squad/4 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants