Giter Site home page Giter Site logo

Comments (14)

kedod avatar kedod commented on May 23, 2024 2

Plugin will be plugin :)
https://docs.litestar.dev/latest/usage/plugins.html

Part of the code can look like this:

def my_rfc9457_handler(request: Request, exception: Exception) -> RFC9457Response: ...

class RFC9457Plugin(InitPluginProtocol):
    def on_app_init(self, app_config: AppConfig) -> AppConfig:
        app_config.exception_handlers[HTTPException] = my_rfc9457_handler
        return app_config

app = Litestar(plugins=[RFC9457Plugin()])

from litestar.

guacs avatar guacs commented on May 23, 2024 1

Maybe the community could help produce a "default" RFC9457 handler for pydantic's ValidationError and some common ValueError, TypeError, LookupError, etc.

I like this idea. Though within litestar, I think we should only catch any litestar specific exceptions rather than ValueError etc.

from litestar.

JacobCoffee avatar JacobCoffee commented on May 23, 2024
Discord Message

guacs — Yesterday at 9:00 PM
Thanks for this!

I think this shouldn't be difficult to implement. It's just the matter of deciding how the API would look like. We could create a plugin which would inject an exception handler that would then convert all the errors into the response following the spec. Or maybe take in a config value rfc9457 that will determine the default exception handler. I like the second approach of taking in a config value, because then users would be able to control it at all the layers so if they only want these problem details for a single controller or route, they could do that.

NOTE: I only skimmed through the RFC, but 7807 has been obsoleted by 9457.

from litestar.

skewty avatar skewty commented on May 23, 2024

NOTE: I only skimmed through the RFC, but 7807 has been obsoleted by 9457.

Oops, I had both open as tabs in browser and used the wrong one when I wrote the issue up. RFC 9457 should obviously be the target goal.

take in a config value rfc9457 that will determine the default exception handler

@guacs Are you thinking something like a mapping?

RFC9457Handler: TypeAlias = Callable[[Request, Exception], RFC9457Response]

def my_rfc9457_handler(request: Request, exception: Exception) -> RFC9457Response: ...

rfc9457_config: dict[type[Controller], RFC9457Handler] = {
    # any controller missing from the list would not have RFC9457
    # error support -- the default handler would be used instead
    MyController: my_rfc9457_handler,
}

Maybe the community could help produce a "default" RFC9457 handler for pydantic's ValidationError and some common ValueError, TypeError, LookupError, etc.

It then becomes trivial to handle some exceptions as custom and return whatever the default handler would produce for others.

What I haven't figured out or put much thought into yet is the URIs being referenced. Should a default RFC9457Controller be created that can be used for the "type" field of the response? It would work in conjunction with the default handler mentioned above.

To me, if the default handler and controller were to be created a plugin in another git repository would be best. Would the plugin just be middleware at that point?

from litestar.

guacs avatar guacs commented on May 23, 2024

Plugin will be plugin :) https://docs.litestar.dev/latest/usage/plugins.html

Part of the code can look like this:

def my_rfc9457_handler(request: Request, exception: Exception) -> RFC9457Response: ...

class RFC9457Plugin(InitPluginProtocol):
    def on_app_init(self, app_config: AppConfig) -> AppConfig:
        app_config.exception_handlers[HTTPException] = my_rfc9457_handler
        return app_config

app = Litestar(plugins=[RFC9457Plugin()])

This is exactly what I had in mind.

Though there are some questions like how to set up type URL that's part of the spec. My immediate thought is that the plugin takes in a default value and uses that unless the caught exception is a new JsonProblemError (a better name will be needed) in which case it would have all the required fields and would then use that.

Another question is whether this plugin should be included in the main repo or whether this should be implemented as a separate library. Considering that this is an official RFC and an effort towards standardization, I'm kind of leaning towards including it the main litestar repo.

from litestar.

bunny-therapist avatar bunny-therapist commented on May 23, 2024

I would suggest ProblemDetailsException and ProblemDetailsPlugin since the RFC has already been updated once and the intention seems to be to support the latest version of it (?). It is also a much simpler and more descriptive name than "RFC 9457".

I recently added support to Litestar for the "+json" media type suffix partially because I wanted to make use of problem details (so now "application/problem+json" gets json encoded automatically - I think problem details may even be used as an example in the docs).

I would expect this feature to work by:

  1. Providing a new exception type, e.g. ProblemDetailsException, that can be raised, resulting in a problem details response.
  2. Optionally (if so configured thru some flag to the plugin) converting HTTPException to ProblemDetails responses instead of the usual litestar error response. This would also apply to e.g. validation errors and similar built in things. This way ProblemDetails becomes the default type of error responses from the application. The ProblemDetailsException can be used to provide more direct control of the problem details fields than whatever the conversion from HTTPException to ProblemDetails would be doing.

I think it is totally reasonable to convert the usual HTTPException to a problem details response because the problem details fields are optional, and there is some overlap (I think both have a "detail" field for example).

from litestar.

guacs avatar guacs commented on May 23, 2024

I would suggest ProblemDetailsException and ProblemDetailsPlugin since the RFC has already been updated once and the intention seems to be to support the latest version of it (?). It is also a much simpler and more descriptive name than "RFC 9457".

I recently added support to Litestar for the "+json" media type suffix partially because I wanted to make use of problem details (so now "application/problem+json" gets json encoded automatically - I think problem details may even be used as an example in the docs).

I would expect this feature to work by:

  1. Providing a new exception type, e.g. ProblemDetailsException, that can be raised, resulting in a problem details response.
  2. Optionally (if so configured thru some flag to the plugin) converting HTTPException to ProblemDetails responses instead of the usual litestar error response. This would also apply to e.g. validation errors and similar built in things. This way ProblemDetails becomes the default type of error responses from the application. The ProblemDetailsException can be used to provide more direct control of the problem details fields than whatever the conversion from HTTPException to ProblemDetails would be doing.

I think it is totally reasonable to convert the usual HTTPException to a problem details response because the problem details fields are optional, and there is some overlap (I think both have a "detail" field for example).

I agree with all your points. I agree that the names you suggested are better since it makes it easier to understand than specifying some RFC which could at some point be superceded by another RFC.

from litestar.

guacs avatar guacs commented on May 23, 2024

I read through the RFC and here are some initial thoughts/doubts I had regarding the implementation.

  • Should we convert all the HTTPExceptions into problem type definitions? Quoting the spec:

    "Likewise, truly generic problems -- i.e., conditions that might apply to any resource on the Web -- are usually better expressed as plain status codes. For example, a "write access disallowed" problem is probably unnecessary, since a 403 Forbidden status code in response to a PUT request is self-explanatory."

    Or we could just consider all of them as about:blank problem type as defined in section 4.2.1.

  • How can we do a generic conversion from ValidationError into a problem type since the problem type would vary based on the resource the client is trying to access? Maybe we can use the opt dictionary to get different details we'll need for the problem detail? This would allow for customizing the problem detail per route.

  • Should we add some kind of support for building the absolute URI for type (using route_reverse)?

  • Maintain support for the problem types defined in HTTP problem types registry? If we are doing this, I think we can do this at a later point once the basic APIs for problem types are created. To be fair, I don't know exactly what "maintain support for problem types defined in the HTTP problem types registry" means, but it could be specialized exceptions for those specific problem types.

  • We should also add support for the OpenAPI schema generation. Should this be generated for all routes? Or simply the routes that may return a problem type reponse? If the latter, then how would we be able to figure out which routes may return a problem type response? Refer Appendix A for the schema we should be generating.

from litestar.

bunny-therapist avatar bunny-therapist commented on May 23, 2024

I haven't had time to read the full RFC, so I can't address all of that right now. I assumed that whenever litestar returns an error code with some json data, that could be replaced by the plugin with problem details (ValidationError give something with detail(s) if memory serves me right) and any fields that we can't figure out can be omitted. I would have to look into the stuff about the problem type and the resource being accessed.

As for the comment about OpenAPI schema generation - I considered suggesting that, because I know that this fastapi plugin does that. However, I felt that this was a separate issue. Litestar currently does not add every possible error response for each endpoint into the Open API schema, so I am not sure it makes sense that error responses should be put there just because of their media type.

I would consider "add possible error responses to Open API schema" a separate issue from this. However, there may be something I am missing, of course.

from litestar.

bunny-therapist avatar bunny-therapist commented on May 23, 2024

Ok, read it now. I think it would be fine if to either use "about:blank" as type, or to even go so far as to define some predefined types in litestar for e.g. validation errors and such things that are already handled inside litestar itself.

from litestar.

skewty avatar skewty commented on May 23, 2024

Ok, read it now. I think it would be fine if to either use "about:blank" as type, or to even go so far as to define some predefined types in litestar for e.g. validation errors and such things that are already handled inside litestar itself.

This was my vision as well. Especially since we can, for example, use pydantic models as endpoint function parameter types for POST which can automatically generate 400 series status codes before even going into the function where the user can raise the appropriate exception type.

from litestar.

guacs avatar guacs commented on May 23, 2024

Ok, read it now. I think it would be fine if to either use "about:blank" as type, or to even go so far as to define some predefined types in litestar for e.g. validation errors and such things that are already handled inside litestar itself.

How would the predefined types work? From what I understood, they should be (or it's preferred) for them to be resolvable URIs that's specific to your application. Or have I misunderstood the spec?

from litestar.

bunny-therapist avatar bunny-therapist commented on May 23, 2024

Ok, read it now. I think it would be fine if to either use "about:blank" as type, or to even go so far as to define some predefined types in litestar for e.g. validation errors and such things that are already handled inside litestar itself.

How would the predefined types work? From what I understood, they should be (or it's preferred) for them to be resolvable URIs that's specific to your application. Or have I misunderstood the spec?

For example something like [app netloc]/problems/validation for ValidationError raised by litestar.

This is an example from the RFC in the same style:

 {
    "type": "https://example.com/probs/out-of-credit",
    "title": "You do not have enough credit.",
    "detail": "Your current balance is 30, but that costs 50.",
    "instance": "/account/12345/msgs/abc",
    "balance": 30,
    "accounts": ["/account/12345",
                 "/account/67890"]
   }

from litestar.

guacs avatar guacs commented on May 23, 2024

@bunny-therapist, @skewty I have a draft PR up with a very minimal implementation. If you have any feedback, that'd be great!

from litestar.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.