Skip to content

Commit

Permalink
Improve exception reporting for CLI commands (#6856)
Browse files Browse the repository at this point in the history
* introduced RasaOpenSourceException

* improved exception handling

* fixed linting issues

* Update telemetry.py

* review suggestions

* fixed typing issue

* trying to fix tests

* fixed tests

* fixed failing tests
  • Loading branch information
tmbo authored Oct 2, 2020
1 parent 50629aa commit ae8eee3
Show file tree
Hide file tree
Showing 35 changed files with 357 additions and 260 deletions.
16 changes: 16 additions & 0 deletions changelog/6856.improvement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Improved exception handling within Rasa Open Source.

All exceptions that are somewhat expected (e.g. errors in file formats like
configurations or training data) will share a common base class
`RasaException`.

::warning Backwards Incompatibility
Base class for the exception raised when an action can not be found has been changed
from a `NameError` to a `ValueError`.
::

Some other exceptions have also slightly changed:
- raise `YamlSyntaxException` instead of YAMLError (from ruamel) when
failing to load a yaml file with information about the line where loading failed
- introduced `MissingDependencyException` as an exception raised if packages
need to be installed
56 changes: 33 additions & 23 deletions rasa/__main__.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,33 @@
import sys
import argparse
import logging
import os
import platform
import sys

from rasa_sdk import __version__ as rasa_sdk_version

import rasa.utils.io
from rasa import version
import rasa.telemetry
from rasa.cli import (
scaffold,
run,
train,
data,
export,
interactive,
run,
scaffold,
shell,
telemetry,
test,
train,
visualize,
data,
x,
export,
)
from rasa.cli.arguments.default_arguments import add_logging_options
from rasa.cli.utils import parse_last_positional_argument_as_model_path
from rasa.utils.common import set_log_level, set_log_and_warnings_filters
from rasa.shared.exceptions import RasaException
from rasa.shared.utils.cli import print_error
import rasa.telemetry
from rasa.utils.common import set_log_and_warnings_filters, set_log_level
import rasa.utils.io
import rasa.utils.tensorflow.environment as tf_env
from rasa_sdk import __version__ as rasa_sdk_version

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -89,8 +93,6 @@ def print_version() -> None:

def main() -> None:
# Running as standalone python application
import os
import sys

parse_last_positional_argument_as_model_path()
arg_parser = create_argument_parser()
Expand All @@ -106,17 +108,25 @@ def main() -> None:
# insert current path in syspath so custom modules are found
sys.path.insert(1, os.getcwd())

if hasattr(cmdline_arguments, "func"):
rasa.utils.io.configure_colored_logging(log_level)
set_log_and_warnings_filters()
rasa.telemetry.initialize_error_reporting()
cmdline_arguments.func(cmdline_arguments)
elif hasattr(cmdline_arguments, "version"):
print_version()
else:
# user has not provided a subcommand, let's print the help
logger.error("No command specified.")
arg_parser.print_help()
try:
if hasattr(cmdline_arguments, "func"):
rasa.utils.io.configure_colored_logging(log_level)
set_log_and_warnings_filters()
rasa.telemetry.initialize_error_reporting()
cmdline_arguments.func(cmdline_arguments)
elif hasattr(cmdline_arguments, "version"):
print_version()
else:
# user has not provided a subcommand, let's print the help
logger.error("No command specified.")
arg_parser.print_help()
sys.exit(1)
except RasaException as e:
# these are exceptions we expect to happen (e.g. invalid training data format)
# it doesn't make sense to print a stacktrace for these if we are not in
# debug mode
logger.debug("Failed to run CLI command due to an exception.", exc_info=e)
print_error(f"{e.__class__.__name__}: {e}")
sys.exit(1)


Expand Down
6 changes: 2 additions & 4 deletions rasa/cli/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,10 @@ def run_nlu_test(args: argparse.Namespace) -> None:
for file in args.config:
try:
validation_utils.validate_yaml_schema(
rasa.shared.utils.io.read_file(file),
CONFIG_SCHEMA_FILE,
show_validation_errors=False,
rasa.shared.utils.io.read_file(file), CONFIG_SCHEMA_FILE,
)
config_files.append(file)
except validation_utils.InvalidYamlFileError:
except validation_utils.YamlValidationException:
logger.debug(
f"Ignoring file '{file}' as it is not a valid config file."
)
Expand Down
3 changes: 2 additions & 1 deletion rasa/core/actions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
ACTION_BACK_NAME,
REQUESTED_SLOT,
)
from rasa.shared.exceptions import RasaException
from rasa.shared.nlu.constants import INTENT_NAME_KEY, INTENT_RANKING_KEY
from rasa.shared.core.events import (
UserUtteranceReverted,
Expand Down Expand Up @@ -661,7 +662,7 @@ def name(self) -> Text:
return self._name


class ActionExecutionRejection(Exception):
class ActionExecutionRejection(RasaException):
"""Raising this exception will allow other policies
to predict a different action"""

Expand Down
10 changes: 0 additions & 10 deletions rasa/core/evaluate.py

This file was deleted.

8 changes: 3 additions & 5 deletions rasa/core/featurizers/tracker_featurizers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import jsonpickle
import logging

from rasa.shared.exceptions import RasaException
from rasa.shared.nlu.constants import TEXT
from tqdm import tqdm
from typing import Tuple, List, Optional, Dict, Text, Union
Expand All @@ -22,18 +23,15 @@
logger = logging.getLogger(__name__)


class InvalidStory(Exception):
class InvalidStory(RasaException):
"""Exception that can be raised if story cannot be featurized."""

def __init__(self, message) -> None:
self.message = message
super(InvalidStory, self).__init__()

def __str__(self) -> Text:
# return message in error colours
return rasa.shared.utils.io.wrap_with_color(
self.message, color=rasa.shared.utils.io.bcolors.FAIL
)
return self.message


class TrackerFeaturizer:
Expand Down
17 changes: 13 additions & 4 deletions rasa/core/policies/ensemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from typing import Text, Optional, Any, List, Dict, Tuple, Set, NamedTuple, Union

import rasa.core
from rasa.shared.exceptions import RasaException
import rasa.shared.utils.common
import rasa.shared.utils.io
import rasa.utils.io
Expand Down Expand Up @@ -380,7 +381,6 @@ def from_dict(cls, policy_configuration: Dict[Text, Any]) -> List[Policy]:
parsed_policies = []

for policy in policies:
policy_name = policy.pop("name")
if policy.get("featurizer"):
featurizer_func, featurizer_config = cls.get_featurizer_from_dict(
policy
Expand All @@ -401,6 +401,7 @@ def from_dict(cls, policy_configuration: Dict[Text, Any]) -> List[Policy]:
# override policy's featurizer with real featurizer class
policy["featurizer"] = featurizer_func(**featurizer_config)

policy_name = policy.pop("name")
try:
constr_func = registry.policy_from_module_path(policy_name)
try:
Expand All @@ -424,7 +425,11 @@ def from_dict(cls, policy_configuration: Dict[Text, Any]) -> List[Policy]:
def get_featurizer_from_dict(cls, policy) -> Tuple[Any, Any]:
# policy can have only 1 featurizer
if len(policy["featurizer"]) > 1:
raise InvalidPolicyConfig("policy can have only 1 featurizer")
raise InvalidPolicyConfig(
f"Every policy can only have 1 featurizer "
f"but '{policy.get('name')}' "
f"uses {len(policy['featurizer'])} featurizers."
)
featurizer_config = policy["featurizer"][0]
featurizer_name = featurizer_config.pop("name")
featurizer_func = registry.featurizer_from_module_path(featurizer_name)
Expand All @@ -435,7 +440,11 @@ def get_featurizer_from_dict(cls, policy) -> Tuple[Any, Any]:
def get_state_featurizer_from_dict(cls, featurizer_config) -> Tuple[Any, Any]:
# featurizer can have only 1 state featurizer
if len(featurizer_config["state_featurizer"]) > 1:
raise InvalidPolicyConfig("featurizer can have only 1 state featurizer")
raise InvalidPolicyConfig(
f"Every featurizer can only have 1 state "
f"featurizer but one of the featurizers uses "
f"{len(featurizer_config['state_featurizer'])}."
)
state_featurizer_config = featurizer_config["state_featurizer"][0]
state_featurizer_name = state_featurizer_config.pop("name")
state_featurizer_func = registry.state_featurizer_from_module_path(
Expand Down Expand Up @@ -737,7 +746,7 @@ def _check_policy_for_forms_available(
)


class InvalidPolicyConfig(Exception):
class InvalidPolicyConfig(RasaException):
"""Exception that can be raised when policy config is not valid."""

pass
16 changes: 7 additions & 9 deletions rasa/core/policies/rule_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json

from rasa.shared.constants import DOCS_URL_RULES
from rasa.shared.exceptions import RasaException
import rasa.shared.utils.io
from rasa.shared.core.events import LoopInterrupted, UserUttered, ActionExecuted
from rasa.core.featurizers.tracker_featurizers import TrackerFeaturizer
Expand Down Expand Up @@ -56,20 +57,17 @@
DO_NOT_PREDICT_LOOP_ACTION = "do_not_predict_loop_action"


class InvalidRule(Exception):
class InvalidRule(RasaException):
"""Exception that can be raised when rules are not valid."""

def __init__(self, message: Text) -> None:
super().__init__()
self.message = message + (
f"\nYou can find more information about the usage of "
f"rules at {DOCS_URL_RULES}. "
)
self.message = message

def __str__(self) -> Text:
# return message in error colours
return rasa.shared.utils.io.wrap_with_color(
self.message, color=rasa.shared.utils.io.bcolors.FAIL
return self.message + (
f"\nYou can find more information about the usage of "
f"rules at {DOCS_URL_RULES}. "
)


Expand Down Expand Up @@ -370,7 +368,7 @@ def _find_contradicting_rules(
if error_messages:
error_messages = "\n".join(error_messages)
raise InvalidRule(
f"\nContradicting rules or stories found🚨\n\n{error_messages}\n"
f"\nContradicting rules or stories found 🚨\n\n{error_messages}\n"
f"Please update your stories and rules so that they don't contradict "
f"each other."
)
Expand Down
8 changes: 0 additions & 8 deletions rasa/core/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,3 @@ async def load_agent_on_start(

logger.info("Rasa server is up and running.")
return app.agent


if __name__ == "__main__":
raise RuntimeError(
"Calling `rasa.core.run` directly is no longer supported. "
"Please use `rasa run` to start a Rasa server or `rasa shell` to chat with "
"your bot on the command line."
)
8 changes: 0 additions & 8 deletions rasa/core/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -876,11 +876,3 @@ async def _evaluate_core_model(model: Text, stories_file: Text) -> int:
)
failed_stories = story_eval_store.failed_stories
return number_of_stories - len(failed_stories)


if __name__ == "__main__":
raise RuntimeError(
"Calling `rasa.core.test` directly is no longer supported. Please use "
"`rasa test` to test a combined Core and NLU model or `rasa test core` "
"to test a Core model."
)
8 changes: 0 additions & 8 deletions rasa/core/train.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,3 @@ def do_interactive_learning(
conversation_id=args.conversation_id,
server_args=args.__dict__,
)


if __name__ == "__main__":
raise RuntimeError(
"Calling `rasa.core.train` directly is no longer supported. Please use "
"`rasa train` to train a combined Core and NLU model or `rasa train core` "
"to train a Core model."
)
8 changes: 0 additions & 8 deletions rasa/core/visualize.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,3 @@ async def visualize(
import webbrowser

webbrowser.open(full_output_path)


if __name__ == "__main__":
raise RuntimeError(
"Calling `rasa.core.visualize` directly is "
"no longer supported. "
"Please use `rasa visualize` instead."
)
Loading

0 comments on commit ae8eee3

Please sign in to comment.