spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.18k stars 40.68k forks source link

Consider using RFC 7807 problem details for error responses #19525

Open bclozel opened 4 years ago

bclozel commented 4 years ago

Right now Spring Boot is using an ad hoc format for error responses. Often, applications are configured to support JSON/XML formats and the error map is serialized with such message converters for machine clients, alternatively errors are rendered as HTML pages for browsers.

We could consider using a better defined format for Spring Boot errors, here the RFC 7807 "problem details" specification. This specification can carry error responses like the following:

   HTTP/1.1 403 Forbidden
   Content-Type: application/problem+json
   Content-Language: en

   {
    "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"]
   }

This could improve error handling in several ways.

First, we could add more contextual information to error maps, like "type", "title" and "detail" and provide application hook points to translate application exceptions to problem details (see the Zalando library for this).

Also, in cases like #19522 and spring-projects/spring-framework#23421, it could allow HTTP clients to add the specific media type "application/problem+json" to their "Accept" header - and would disambiguate about the format to use when rendering errors. Right now, "application/json" and "application/xml" are the common ones but it's hard to differentiate between application payload and error payloads.

jnizet commented 4 years ago

My two cents regarding JSON errors in general: in basically every project using Spring Boot for HTTP services, and a JavaScript framework (such as Angular or Vue), I always need to customize the JSON error sent by Spring Boot in order to

I haven't read the RFC very carefully yet, but if Spring Boot could provide a standard way to support this common (in my experience) need, it would be great. Or at least not lose the possibility to do it ourselves, and maybe document what the best strategy is to do it by respecting the standard.

t1 commented 4 years ago

I like to map exceptions to rfc-7807 compliant bodies on the server side and/or back to exceptions on the client side (note: they don't have to be the same exception, they only have to be compatible). I defined an API and wrote an implementation where you can rely on useful defaults for the fields or annotate your exceptions when you have specific requirements: https://github.com/t1/problem-details

I’ve blogged about the topic in more detail: https://blog.codecentric.de/en/2020/01/rfc-7807-problem-details-with-spring-boot-and-jax-rs/

I also created a PR for Microprofile Sandbox, as I think it should be a common standard.

I would be glad to contribute the Spring Boot implementation.

In order to make the client side feel more organic, maybe we would need to scan for all exceptions?

Any feedback (except for silence ;-) is welcome!

skurlow commented 4 years ago

Yes agreed. A spring implementation of rfc 7807 would be great!

roamingthings commented 4 years ago

Currently I'm looking into error handling in Spring Boot applications and found RFC 7807 to be very interesting. I like that by default it allows me to give meaningful and helpful Problem responses while hiding implementation details like stacktraces or class names for advanced security.

So I would also welcome this in Spring (Boot).

@t1 thank you for the detailed and practical article

vpavic commented 3 years ago

Spring HATEOAS provides basic support for RFC-7807 since 1.1, see spring-projects/spring-hateoas#1057 and the relevant chapter of reference manual.

Now, this might not mean too much for Spring (Boot) users that don't use Spring HATEOAS, but since RFC-7807 is IMO a generally very useful spec, I wonder whether this basic support could be moved to Spring Framework proper? Because it is something that has a clear use even without the involvement of Spring HATEOAS and/or Spring Boot. Both could then leverage the Framework support and build their own extended support on top of it. WDYT @bclozel and @gregturn?

Also, seeing this was twice marked as for: team-attention, could Spring Boot team share outcome of those discussions?

philwebb commented 3 years ago

@vpavic I don't think there really was an outcome from the team discussions. We probably flagged it to discuss how easy it would be to get it into a release, but ultimately decided to prioritize other issues.

gregturn commented 3 years ago

Naturally I’d favor this. At least the domain types from Spring HATEOAS could be moved, giving universal access to them.

But we’d need agreement between Framework and Boot on this approach.

philwebb commented 3 years ago

Also timing will be a bit tricky given that we're not expecting a Framework 5.4 release.

t1 commented 3 years ago

Building the response Problem object by hand looks like a lot of boiler plate code to me. I think throwing a specific business exception and have that automatically mapped to a reasonable problem detail by default (with some annotations for specialization) is much easier, esp. if this also works on the client side! (for more details take a look at my blog mentioned above).

vy commented 3 years ago

At bol.com, the biggest online retailer in the Netherlands and Belgium, RFC 7807 is our bread and butter. Judging from the amount of work Zalando has also put into this (problem and problem-spring-web), I think we are not alone in this effort. This said, there is a significant amount of work that we needed to implement to make Problems work with Spring. Here I want to share some highlights from our iceberg.

Model

In our custom Problem model, we enhance the one in RFC 7807 with the following fields:

Problem is an interface. Its default implementation is DefaultProblem accessible via Problem.builder(). DefaultProblem extends from AbstractThrowableProblem, which extends from RuntimeException and implements Problem.

There are, IIRC, at least 3 Jackson bugs one need to work around to make serialization work using annotations. For instance, if cause extends from Throwable, given Throwable is final, quite some mixin magic is needed to avoid serializing stack traces and such. Hence, the necessary (de)serialization plumbing is far from trivial.

Models are published in a standalone artifact with only Jackson dependency.

Integration

We encourage our programmers to always throw standard exceptions (e.g., RuntimeException) in their business logic. If need arises, they are only advised to catch exceptions at the controller level and map them to Problems manually. Put another way, we only advise explicit use of Problems to communicate a failure if current mappers fall short of addressing the need; else, standard exceptions are the way to go.

Mapping exceptions to Problems

Our plethora of Spring sauce does its best to catch thrown exceptions and create a meaningful Problem out of that. We have custom mappers for a variety of Spring exceptions:

While this works 99% of the cases, it indeed is a moving target. Yet, it works pretty good so far for us.

Logging exceptions

This is the curse and bliss of our implementation. We support logging of Problems. That is, whenever a REST controller responds with a Problem, we log it. The logging behaviour can be configured to set the logging level and filter on type. For instance, "log Problems of type /problems/unhandled-exception at ERROR level".

Our users really much depend on this feature and love it! Yet, it is practically impossible to cover every case. There are a couple of bug reports with really weird combinations (e.g., "exceptions thrown from exception handlers in a Web MVC controller fails to log the response Problem") we still need to figure out how to tackle. This issue is simply solved via introducing a bean extending from HandlerResultHandler and access to the response model via HandlerResult#getReturnValue(). Though this only works for the WebFlux backend. Web MVC doesn't have such a global interceptor where one can access to the response model. WebFlux's HandlerResultHandler equivalent in Web MVC is HandlerInterceptor and that only exposes the raw I/O stream. Long story short, intercepting Problems via a global handler that works with both WebFlux and Web MVC is not possible at this stage, to the best of our knowledge.

Conclusion

We are totally unhappy with the myriad of enhancements we need to add to make Problems work. Worse, occassionally we stumble upon corner cases that doesn't work. We will be extremely happy to see this feature being shipped by Spring itself. I also need to note that we can afford company time to contribute to this effort.

/cc @lkleeven @mzeijen @breun @sagacity

rstoyanchev commented 3 years ago

I've created an issue in the Spring Framework https://github.com/spring-projects/spring-framework/issues/27052 for this.

dhh1128 commented 3 years ago

This comment from a developer at Reddit, about why Reddit is not supporting the RFC, is another data point: https://www.reddit.com/r/api/comments/hze1i2/do_you_folks_use_rfc_7807_problem_details_in_http/

t1 commented 3 years ago

@dhh1128: The post has been deleted!?! What are the arguments?

dhh1128 commented 3 years ago

It's not the post; it's the response to the post (which still remains), that's interesting. The argument is that a standard error reporting mechanism that is rich rather than terse encourages over-disclosure in errors, which is a security risk.

breun commented 3 years ago

I don't find the security angle a very compelling argument against supporting RFC 7807 Problems. I've been using Problem JSON for a couple of years now in a micro services architecture with many hundreds of REST API applications, and in my experience developers that come across Problem JSON mostly start producing errors that are way clearer and easier to understand for users. Over-disclosure can happen through any format, and a plain text stack trace is generally way worse than a Problem JSON object. I don't feel that we shouldn't have nice things just because they could contain information that shouldn't be exposed. Not having a nice standard for errors isn't a way to get more secure applications. Things like training, awareness, testing, etc. are what will get you more secure applications.

t1 commented 3 years ago

It's an old dispute going forth and back: should we prevent good things just because they can be misused? Don't get me wrong: this dispute is difficult and there is no simple answer. But in this case, I stand clearly on one side, because it's a very good thing and the risk is not seriously big. And best of all, we can even implement it in a way that doesn't reveal anything by default, so developers have to actively decide what they want to be exposed... and they do that every day with non-exceptional code — I think they can handle it.

sagacity commented 3 years ago

Additionally, there is already a precedent in the Spring Boot white label error page which can can be configured to show stack trace information.

dhh1128 commented 3 years ago

Just to be clear, i was posting the argument because I thought it was worth analyzing. It deserves a response. But that doesn't mean I agree with it. I think the benefits of a general mechanism outweigh the drawbacks.

StefanLobbenmeier commented 1 year ago

Is this fixed with https://github.com/spring-projects/spring-framework/issues/27052 now?

bclozel commented 1 year ago

@StefanLobbenmeier not completely, as Spring Boot doesn't produce this format by default and Spring Boot error handling is not based on that. The Spring Framework feature can be enabled with a property but is still opt in for now.

fabiofranco85 commented 1 year ago

using latest SB 3.1 with webflux, I can successfully use problem details when raising ResponseStatusException and can successfully handle other exceptions with @ExceptionHandler. But if I make a rest request that doesn't match any of my controllers the server responds with the old format and the only way I found to fix this was by adding spring.web.resources.add-mappings=false in my application.properties file. I tried to understand this but couldn't find any useful explanations in the docs, internet and even spent several hours digging into spring's source code but I still cannot figure out why this is happening and how to "change" this outcome without having to disable the resources mappings. any ideas?

update: interesting thing... I managed to fix this by adding spring.webflux.static-path-pattern=/resources/** to my application.properties - so now if I make a request to a non-existing resource it returns the correct ProblemDetails response... even if I request a non-existing resource to /resources/** it comes back correctly. So I'm wondering if that's a bug 🤔

danielrohe commented 1 year ago

The problem of the different handling for Spring Framework using ControllerAdvice and Spring Boot using ErrorAttributes. Did someone consider adding a new implementation of the ErrorAttributes interface and ErrorView class to Spring Boot that handles the exceptions and converts them into a Problem JSON structure considering possible existing ProblemDetail objects?

juliojgd commented 1 year ago

As RFC 7807 has been obsoleted by RFC 9457:

@rstoyanchev @bclozel are there any plans to support the format defined in the new RFC 9457?

I couldn't find any reference to it either in Spring Framework repository nor in this Spring Boot one.

Thanks in advance

bclozel commented 1 year ago

I don't see much difference here. Am I missing something?

https://datatracker.ietf.org/doc/html/rfc9457#appendix-D

juliojgd commented 1 year ago

I didn't see much difference either, maybe the references in documentation/code to RFC 7807 should be updated in order to reference the new RFC 9457? To highlight the support of the new non-obsoleted RFC.

hbudde commented 1 month ago

Any updates on this topic?

philwebb commented 1 month ago

@hbudde I'm afraid not. Rest assured, we'll update the issue with news if we have any.