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

Improve exception reporting for CLI commands #6856

Merged
merged 13 commits into from
Oct 2, 2020
Merged

Conversation

tmbo
Copy link
Member

@tmbo tmbo commented Sep 30, 2020

Motivation:

  • We print unecessary stack traces for exceptions like failures to read a training data yaml file. This isn't helpful as we know that the issue is the syntax in the yaml file and a stack trace won't help.
  • Many exceptions reported to sentry are things we expect to happen (errors in the training data format, a wrong action name, ...)

Before:
Bildschirmfoto 2020-10-01 um 11 15 22

After:
Bildschirmfoto 2020-10-01 um 11 16 52

Proposed changes:

  • base class for all exceptions raised in Rasa Open Source that are "expected" (e.g. file format issues, missing action names, ... things we expect a user to get wrong. they will still bubble up, but are not going to create a stacktrace). RasaOpenSourceException
  • all RasaOpenSourceExceptions will be caught if thrown as part of a CLI command and stack traces will not be printed by default, instead we use print_error
  • changed NameError exception raised when an action can not be found to be a ValueError instead
  • raise InvalidYAMLFileException instead of YAMLError (from ruamel) when failing to load a yaml file
  • made InvalidDomain a RasaOpenSourceException
  • introduced MissingDependencyException as an exception raised if packages need to be installed
  • added line numbers to output of YAML validation to the command line as paths on their own are hard to navigate:
InvalidYamlFileError: Failed to validate 'data/nlu.yml'. Please make sure the file is correct and all mandatory parameters are specified. Please take a look at the errors found during validation:
  in data/nlu.yml:2:
      Key 'ZZZZ' was not defined. Path: '/nlu/0'
  • properly exit on ctrl+c / ctrl+d in rasa shell nlu command
  • RasaOpenSourceExceptions are ignored by sentry

TODO

  • InvalidRule is very user-unfriendly and vague, but out of this PRs scope

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@tmbo tmbo requested a review from m-vdb October 1, 2020 09:14
@tmbo tmbo changed the title Improve exception reporting Improve exception reporting for CLI commands Oct 1, 2020
Copy link
Collaborator

@m-vdb m-vdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🚀 🚀 Amazing, thanks for doing these changes. I definitely think that they will improve the UX

changelog/6856.improvement.md Outdated Show resolved Hide resolved
rasa/__main__.py Show resolved Hide resolved
changelog/6856.improvement.md Outdated Show resolved Hide resolved
rasa/core/policies/rule_policy.py Show resolved Hide resolved
rasa/nlu/components.py Outdated Show resolved Hide resolved
rasa/shared/utils/io.py Outdated Show resolved Hide resolved
rasa/shared/utils/validation.py Outdated Show resolved Hide resolved
rasa/shared/utils/validation.py Outdated Show resolved Hide resolved
rasa/shared/utils/validation.py Outdated Show resolved Hide resolved
rasa/telemetry.py Show resolved Hide resolved
@tmbo
Copy link
Member Author

tmbo commented Oct 1, 2020

thanks a lot for the quick review super helpful to get this merged in time ⭐

Copy link
Collaborator

@m-vdb m-vdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 💯 I think it's mergeable once the tests go green 🟢

@tmbo tmbo merged commit ae8eee3 into master Oct 2, 2020
@tmbo tmbo deleted the exception-improvements branch October 2, 2020 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants