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
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
tmbo marked this conversation as resolved.
Show resolved Hide resolved

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 FormValidation, 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}. "
tmbo marked this conversation as resolved.
Show resolved Hide resolved
)


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 @@ -868,11 +868,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