Iterate on Sentry filtering

changelog/7292.improvement.md

@@ -0,0 +1,12 @@
+Improve error handling and Sentry tracking:
+- Raise `MarkdownException` when training data in Markdown format cannot be read.
+- Raise `InvalidEntityFormatException` error instead of `json.JSONDecodeError` when entity format is in valid
+  in training data.
+- Gracefully handle empty sections in endpoint config files.
+- Introduce `ConnectionException` error and raise it when `TrackerStore` and `EventBroker`
+  cannot connect to 3rd party services, instead of raising exceptions from 3rd party libraries.
+- Improve `rasa.shared.utils.common.class_from_module_path` function by making sure it always returns a class.
+  The function currently raises a deprecation warning if it detects an anomaly.
+- Ignore `MemoryError` and `asyncio.CancelledError` in Sentry.
+- `rasa.shared.utils.validation.validate_training_data` now raises a `SchemaValidationError` when validation fails
+  (this error inherits `jsonschema.ValidationError`, ensuring backwards compatibility).

data/test_endpoints/example_endpoints.yml

@@ -23,3 +23,4 @@ tracker_store:
   #db: rasa
   #user: username
   #password: password
+empty:

rasa/core/brokers/broker.py

@@ -3,8 +3,12 @@
 from asyncio import AbstractEventLoop
 from typing import Any, Dict, Text, Optional, Union
 
+import aio_pika.exceptions
+import sqlalchemy.exc
+
 import rasa.shared.utils.common
 import rasa.shared.utils.io
+from rasa.shared.exceptions import ConnectionException
 from rasa.utils.endpoints import EndpointConfig
 
 logger = logging.getLogger(__name__)
@@ -22,7 +26,18 @@ async def create(
         if isinstance(obj, EventBroker):
             return obj
 
-        return await _create_from_endpoint_config(obj, loop)
+        import aio_pika.exceptions
+        import sqlalchemy.exc
+
+        connection_errors = (
+            sqlalchemy.exc.OperationalError,
+            aio_pika.exceptions.AMQPConnectionError,
+        )
+
+        try:
+            return await _create_from_endpoint_config(obj, loop)
+        except connection_errors as error:
+            raise ConnectionException("Cannot connect to event broker.") from error
 
     @classmethod
     async def from_endpoint_config(

rasa/core/tracker_store.py

@@ -21,7 +21,6 @@
 )
 
 from boto3.dynamodb.conditions import Key
-from botocore.exceptions import ClientError
 
 import rasa.core.utils as core_utils
 import rasa.shared.utils.cli
@@ -42,6 +41,7 @@
     DialogueStateTracker,
     EventVerbosity,
 )
+from rasa.shared.exceptions import ConnectionException
 from rasa.shared.nlu.constants import INTENT_NAME_KEY
 from rasa.utils.endpoints import EndpointConfig
 import sqlalchemy as sa
@@ -65,7 +65,7 @@
 
 
 class TrackerStore:
-    """Class to hold all of the TrackerStore classes"""
+    """Represents common behavior and interface for all `TrackerStore`s."""
 
     def __init__(
         self,
@@ -115,7 +115,20 @@ def create(
         if isinstance(obj, TrackerStore):
             return obj
 
-        return _create_from_endpoint_config(obj, domain, event_broker)
+        from botocore.exceptions import BotoCoreError
+        import pymongo.errors
+        import sqlalchemy.exc
+
+        connection_errors = (
+            BotoCoreError,
+            pymongo.errors.ConnectionFailure,
+            sqlalchemy.exc.OperationalError,
+        )
+
+        try:
+            return _create_from_endpoint_config(obj, domain, event_broker)
+        except connection_errors as error:
+            raise ConnectionException("Cannot connect to tracker store.") from error
 
     def get_or_create_tracker(
         self,
@@ -407,7 +420,7 @@ def __init__(
     def get_or_create_table(
         self, table_name: Text
     ) -> "boto3.resources.factory.dynamodb.Table":
-        """Returns table or creates one if the table name is not in the table list"""
+        """Returns table or creates one if the table name is not in the table list."""
         import boto3
 
         dynamo = boto3.resource("dynamodb", region_name=self.region)
@@ -442,7 +455,9 @@ def get_or_create_table(
         return table
 
     def save(self, tracker):
-        """Saves the current conversation state"""
+        """Saves the current conversation state."""
+        from botocore.exceptions import ClientError
+
         if self.event_broker:
             self.stream_events(tracker)
         serialized = self.serialise_tracker(tracker)
@@ -475,7 +490,7 @@ def _retrieve_latest_session_date(self, sender_id: Text) -> Optional[int]:
         return dialogues[0].get("session_date")
 
     def serialise_tracker(self, tracker: "DialogueStateTracker") -> Dict:
-        """Serializes the tracker, returns object with decimal types"""
+        """Serializes the tracker, returns object with decimal types."""
         d = tracker.as_dialogue().as_dict()
         d.update(
             {"sender_id": tracker.sender_id,}
@@ -505,16 +520,15 @@ def retrieve(self, sender_id: Text) -> Optional[DialogueStateTracker]:
         )
 
     def keys(self) -> Iterable[Text]:
-        """Returns sender_ids of the DynamoTrackerStore"""
+        """Returns sender_ids of the DynamoTrackerStore."""
         return [
             i["sender_id"]
             for i in self.db.scan(ProjectionExpression="sender_id")["Items"]
         ]
 
 
 class MongoTrackerStore(TrackerStore):
-    """
-    Stores conversation history in Mongo
+    """Stores conversation history in Mongo.
 
     Property methods:
         conversations: returns the current conversation
@@ -552,11 +566,11 @@ def __init__(
 
     @property
     def conversations(self):
-        """Returns the current conversation"""
+        """Returns the current conversation."""
         return self.db[self.collection]
 
     def _ensure_indices(self):
-        """Create an index on the sender_id"""
+        """Create an index on the sender_id."""
         self.conversations.create_index("sender_id")
 
     @staticmethod
@@ -569,7 +583,7 @@ def _current_tracker_state_without_events(tracker: DialogueStateTracker) -> Dict
         return state
 
     def save(self, tracker, timeout=None):
-        """Saves the current conversation state"""
+        """Saves the current conversation state."""
         if self.event_broker:
             self.stream_events(tracker)
 
@@ -684,7 +698,7 @@ def retrieve_full_tracker(
         )
 
     def keys(self) -> Iterable[Text]:
-        """Returns sender_ids of the Mongo Tracker Store"""
+        """Returns sender_ids of the Mongo Tracker Store."""
         return [c["sender_id"] for c in self.conversations.find()]
 
 

rasa/shared/exceptions.py

@@ -1,4 +1,5 @@
 from typing import Optional, Text
+import jsonschema
 
 
 class RasaException(Exception):
@@ -17,6 +18,10 @@ class InvalidParameterException(RasaException, ValueError):
     """Raised when an invalid parameter is used."""
 
 
+class MarkdownException(RasaException, ValueError):
+    """Raised if there is an error reading Markdown."""
+
+
 class YamlException(RasaException):
     """Raised if there is an error reading yaml."""
 
@@ -77,3 +82,19 @@ class InvalidConfigException(ValueError, RasaException):
 
 class UnsupportedFeatureException(RasaCoreException):
     """Raised if a requested feature is not supported."""
+
+
+class SchemaValidationError(RasaException, jsonschema.ValidationError):
+    """Raised if schema validation via `jsonschema` failed."""
+
+
+class InvalidEntityFormatException(RasaException):
+    """Raised if the format of an entity is invalid."""
+
+
+class ConnectionException(RasaException):
+    """Raised when a connection to a 3rd party service fails.
+
+    It's used by our broker and tracker store classes, when
+    they can't connect to services like postgres, dynamoDB, mongo.
+    """

rasa/shared/nlu/training_data/entities_parser.py

@@ -4,14 +4,15 @@
 
 import rasa.shared.nlu.training_data.util
 from rasa.shared.constants import DOCS_URL_TRAINING_DATA_NLU
+from rasa.shared.exceptions import InvalidEntityFormatException
 from rasa.shared.nlu.constants import (
     ENTITY_ATTRIBUTE_VALUE,
     ENTITY_ATTRIBUTE_TYPE,
     ENTITY_ATTRIBUTE_GROUP,
     ENTITY_ATTRIBUTE_ROLE,
 )
 from rasa.shared.nlu.training_data.message import Message
-import rasa.shared.utils.io
+
 
 GROUP_ENTITY_VALUE = "value"
 GROUP_ENTITY_TYPE = "entity"
@@ -127,8 +128,8 @@ def get_validated_dict(json_str: Text) -> Dict[Text, Text]:
         json_str: The entity dict as string without "{}".
 
     Raises:
-        ValidationError if validation of entity dict fails.
-        JSONDecodeError if provided entity dict is not valid json.
+        SchemaValidationError if validation of parsed entity fails.
+        InvalidEntityFormatException if provided entity is not valid json.
 
     Returns:
         Deserialized and validated `json_str`.
@@ -141,12 +142,10 @@ def get_validated_dict(json_str: Text) -> Dict[Text, Text]:
     try:
         data = json.loads(f"{{{json_str}}}")
     except JSONDecodeError as e:
-        rasa.shared.utils.io.raise_warning(
+        raise InvalidEntityFormatException(
             f"Incorrect training data format ('{{{json_str}}}'). Make sure your "
-            f"data is valid.",
-            docs=DOCS_URL_TRAINING_DATA_NLU,
-        )
-        raise e
+            f"data is valid. More info at {DOCS_URL_TRAINING_DATA_NLU}"
+        ) from e
 
     validation_utils.validate_training_data(data, schema.entity_dict_schema())
 

rasa/shared/nlu/training_data/formats/markdown.py

@@ -9,6 +9,7 @@
     LEGACY_DOCS_BASE_URL,
     DOCS_URL_MIGRATION_GUIDE_MD_DEPRECATION,
 )
+from rasa.shared.exceptions import MarkdownException
 from rasa.shared.nlu.constants import TEXT
 from rasa.shared.nlu.training_data.formats.readerwriter import (
     TrainingDataReader,
@@ -135,47 +136,10 @@ def _parse_item(self, line: Text) -> None:
                     self.current_title, item, self.lookup_tables
                 )
 
-    @staticmethod
-    def _get_validated_dict(json_str: Text) -> Dict[Text, Text]:
-        """Converts the provided json_str to a valid dict containing the entity
-        attributes.
-
-        Users can specify entity roles, synonyms, groups for an entity in a dict, e.g.
-        [LA]{"entity": "city", "role": "to", "value": "Los Angeles"}
-
-        Args:
-            json_str: the entity dict as string without "{}"
-
-        Raises:
-            ValidationError if validation of entity dict fails.
-            JSONDecodeError if provided entity dict is not valid json.
-
-        Returns:
-            a proper python dict
-        """
-        import json
-        import rasa.shared.utils.validation as validation_utils
-        import rasa.shared.nlu.training_data.schemas.data_schema as schema
-
-        # add {} as they are not part of the regex
-        try:
-            data = json.loads(f"{{{json_str}}}")
-        except JSONDecodeError as e:
-            rasa.shared.utils.io.raise_warning(
-                f"Incorrect training data format ('{{{json_str}}}'), make sure your "
-                f"data is valid. For more information about the format visit "
-                f"{LEGACY_DOCS_BASE_URL}/nlu/training-data-format/."
-            )
-            raise e
-
-        validation_utils.validate_training_data(data, schema.entity_dict_schema())
-
-        return data
-
     def _set_current_section(self, section: Text, title: Text) -> None:
         """Update parsing mode."""
         if section not in AVAILABLE_SECTIONS:
-            raise ValueError(
+            raise MarkdownException(
                 "Found markdown section '{}' which is not "
                 "in the allowed sections '{}'."
                 "".format(section, "', '".join(AVAILABLE_SECTIONS))

rasa/shared/utils/common.py

@@ -5,32 +5,60 @@
 import logging
 from typing import Text, Dict, Optional, Any, List, Callable, Collection
 
+import rasa.shared.utils.io
+from rasa.shared.constants import NEXT_MAJOR_VERSION_FOR_DEPRECATIONS
+
+
 logger = logging.getLogger(__name__)
 
 
 def class_from_module_path(
-    module_path: Text, lookup_path: Optional[Text] = None
+    module_path: Text, lookup_path: Optional[Text] = None, ensure_class: bool = True
 ) -> Any:
     """Given the module name and path of a class, tries to retrieve the class.
 
-    The loaded class can be used to instantiate new objects."""
-    # load the module, will raise ImportError if module cannot be loaded
+    The loaded class can be used to instantiate new objects.
+
+    Args:
+        module_path: either an absolute path to a Python class,
+                     or the name of the class in the local / global scope.
+        lookup_path: a path where to load the class from, if it cannot
+                     be found in the local / global scope.
+        ensure_class: a boolean that will ensure that the loaded Python class
+                      is actually class if `True`. When acticated, a deprecation
+                      warning is raised when the loaded object is not a class.
+
+    Returns:
+        a Python class
+
+    Raises:
+        ImportError, in case the Python class cannot be found.
+    """
+    klass = None
     if "." in module_path:
         module_name, _, class_name = module_path.rpartition(".")
         m = importlib.import_module(module_name)
-        # get the class, will raise AttributeError if class cannot be found
-        return getattr(m, class_name)
+        klass = getattr(m, class_name, None)
     else:
-        module = globals().get(module_path, locals().get(module_path))
-        if module is not None:
-            return module
+        klass = globals().get(module_path, locals().get(module_path))
 
-        if lookup_path:
+        if klass is None and lookup_path:
             # last resort: try to import the class from the lookup path
             m = importlib.import_module(lookup_path)
-            return getattr(m, module_path)
-        else:
-            raise ImportError(f"Cannot retrieve class from path {module_path}.")
+            klass = getattr(m, module_path, None)
+
+    if klass is None:
+        raise ImportError(f"Cannot retrieve class from path {module_path}.")
+
+    if ensure_class and not inspect.isclass(klass):
+        rasa.shared.utils.io.raise_deprecation_warning(
+            f"`class_from_module_path()` is expected to return a class, "
+            f"but {module_path} is not one. "
+            f"This warning will be converted "
+            f"into an exception in {NEXT_MAJOR_VERSION_FOR_DEPRECATIONS}."
+        )
+
+    return klass
 
 
 def all_subclasses(cls: Any) -> List[Any]: