Skip to content

Commit

Permalink
Merge pull request #7292 from RasaHQ/fix-minor-issues-filter-sentry
Browse files Browse the repository at this point in the history
Iterate on Sentry filtering
  • Loading branch information
rasabot authored Jan 14, 2021
2 parents 5347373 + 7432444 commit 395f768
Show file tree
Hide file tree
Showing 18 changed files with 304 additions and 97 deletions.
12 changes: 12 additions & 0 deletions changelog/7292.improvement.md
Original file line number Diff line number Diff line change
@@ -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).
1 change: 1 addition & 0 deletions data/test_endpoints/example_endpoints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ tracker_store:
#db: rasa
#user: username
#password: password
empty:
12 changes: 11 additions & 1 deletion rasa/core/brokers/broker.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

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__)
Expand All @@ -22,7 +23,16 @@ async def create(
if isinstance(obj, EventBroker):
return obj

return await _create_from_endpoint_config(obj, loop)
import aio_pika.exceptions
import sqlalchemy.exc

try:
return await _create_from_endpoint_config(obj, loop)
except (
sqlalchemy.exc.OperationalError,
aio_pika.exceptions.AMQPConnectionError,
) as error:
raise ConnectionException("Cannot connect to event broker.") from error

@classmethod
async def from_endpoint_config(
Expand Down
38 changes: 25 additions & 13 deletions rasa/core/tracker_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -115,7 +115,18 @@ 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

try:
return _create_from_endpoint_config(obj, domain, event_broker)
except (
BotoCoreError,
pymongo.errors.ConnectionFailure,
sqlalchemy.exc.OperationalError,
) as error:
raise ConnectionException("Cannot connect to tracker store.") from error

def get_or_create_tracker(
self,
Expand Down Expand Up @@ -407,7 +418,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)
Expand Down Expand Up @@ -442,7 +453,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)
Expand Down Expand Up @@ -475,7 +488,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,}
Expand Down Expand Up @@ -505,16 +518,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
Expand Down Expand Up @@ -552,11 +564,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
Expand All @@ -569,7 +581,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)

Expand Down Expand Up @@ -684,7 +696,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()]


Expand Down
36 changes: 35 additions & 1 deletion rasa/shared/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import json
from typing import Optional, Text

import jsonschema


class RasaException(Exception):
"""Base exception class for all errors raised by Rasa Open Source."""
"""Base exception class for all errors raised by Rasa Open Source.
These exceptions results from invalid use cases and will be reported
to the users, but will be ignored in telemetry.
"""


class RasaCoreException(RasaException):
Expand All @@ -17,6 +24,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."""

Expand Down Expand Up @@ -77,3 +88,26 @@ 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, json.JSONDecodeError):
"""Raised if the format of an entity is invalid."""

@classmethod
def create_from(
cls, other: json.JSONDecodeError, msg: Text
) -> "InvalidEntityFormatException":
"""Create an instance of `InvalidEntityFormatException` from a `JSONDecodeError`."""
return cls(msg, other.doc, other.pos)


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.
"""
18 changes: 9 additions & 9 deletions rasa/shared/nlu/training_data/entities_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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`.
Expand All @@ -141,12 +142,11 @@ 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(
f"Incorrect training data format ('{{{json_str}}}'). Make sure your "
f"data is valid.",
docs=DOCS_URL_TRAINING_DATA_NLU,
)
raise e
raise InvalidEntityFormatException.create_from(
e,
f"Incorrect training data format ('{{{json_str}}}'). "
f"More info at {DOCS_URL_TRAINING_DATA_NLU}",
) from e

validation_utils.validate_training_data(data, schema.entity_dict_schema())

Expand Down
40 changes: 2 additions & 38 deletions rasa/shared/nlu/training_data/formats/markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
Expand Down
52 changes: 37 additions & 15 deletions rasa/shared/utils/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
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__)


Expand All @@ -13,24 +17,42 @@ def class_from_module_path(
) -> 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.
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)
else:
module = globals().get(module_path, locals().get(module_path))
if module is not None:
return module

if 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, class_name, None)
elif lookup_path:
# try to import the class from the lookup path
m = importlib.import_module(lookup_path)
klass = getattr(m, module_path, None)

if klass is None:
raise ImportError(f"Cannot retrieve class from path {module_path}.")

if 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]:
Expand Down
Loading

0 comments on commit 395f768

Please sign in to comment.