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

Add contextual exceptions #2290

Merged
merged 7 commits into from Nov 18, 2021
Merged

Add contextual exceptions #2290

merged 7 commits into from Nov 18, 2021

Conversation

ahopkins
Copy link
Member

@ahopkins ahopkins commented Oct 27, 2021

As you know, I have been looking at exception handling a lot lately (see #2162). This is an attempt to make exception handling even more consistent across an application and more developer-friendly. Yes, we have @app.exception to allow you to format exceptions. But, especially if you have lots of different types of exceptions, it often becomes overwhelming to create a set of consistent error messaging.

#2216 took a nice step forward with this:

class TeapotError(SanicException):
    status_code = 418
    message = "Sorry, I cannot brew coffee"

raise TeapotError

But this lacks two things:

  1. A dynamic and predictable message format
  2. The ability to add additional context to an error message (more on this in a moment)

With this PR, we can add some information to the exception when we raise it to help us format the error message:

class TeapotError(SanicException):
    status_code = 418

    @property
    def message(self):
        return f"Sorry {self.extra['name']}, I cannot make you coffee"

raise TeapotError(extra={"name": "Adam"})

In a somewhat similar API to logging, we have the ability to bring extra meta to the exception instance. This extra info object should be suppressed when in production mode, but we allow it to be displayed in debug mode.

image

image


Getting back to item 2 from above: The ability to add additional context to an error message

This is particularly useful when creating microservices or an API that you intend to pass error messages back in JSON format. In this use case, we want to have some context around the exception beyond just a parseable error message to return details to the client.

This PR also addes a context object that is similar to what is used in signals.

raise TeapotError(context={"foo": "bar"})

This is information that we want to always be passed in the error (when it is available). Here is what it should look like in non-debug mode (better naming on this in #2237):

{
  "description": "I'm a teapot",
  "status": 418,
  "message": "Sorry Adam, I cannot make you coffee",
  "context": {
    "foo": "bar"
  }
}

With debug now turned on:

{
  "description": "I'm a teapot",
  "status": 418,
  "message": "Sorry Adam, I cannot make you coffee",
  "context": {
    "foo": "bar"
  },
  "extra": {
    "name": "Adam",
    "more": "lines",
    "complex": {
      "one": "two"
    }
  },
  "path": "/",
  "args": {},
  "exceptions": [
    {
      "type": "TeapotError",
      "exception": "Sorry Adam, I cannot make you coffee",
      "frames": [
        {
          "file": "handle_request",
          "line": 83,
          "name": "handle_request",
          "src": ""
        },
        {
          "file": "/tmp/p.py",
          "line": 17,
          "name": "handler",
          "src": "raise TeapotError("
        }
      ]
    }
  ]
}

TODO

  • JSON context testing
  • JSON extra testing
  • HTML context testing
  • HTML extra testing
  • TEXT context testing
  • TEXT extra testing

@ahopkins ahopkins marked this pull request as ready for review November 18, 2021 07:20
@ahopkins ahopkins requested a review from a team as a code owner November 18, 2021 07:20
@ahopkins ahopkins added the needs review the PR appears ready but requires a review label Nov 18, 2021
Copy link
Member

@vltr vltr left a comment

Choose a reason for hiding this comment

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

this seems very useful 💪

@codeclimate
Copy link

codeclimate bot commented Nov 18, 2021

Code Climate has analyzed commit 1bf59ef and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (86% is the threshold).

This pull request will bring the total coverage in the repository to 86.8% (0.0% change).

View more on Code Climate.

@ahopkins ahopkins merged commit 523db19 into main Nov 18, 2021
@ahopkins ahopkins deleted the exception-context branch November 18, 2021 15:47
ChihweiLHBird pushed a commit to ChihweiLHBird/sanic that referenced this pull request Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review the PR appears ready but requires a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants