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

Consider using RFC 7807 problem details for error responses #19525

Open
bclozel opened this issue Jan 3, 2020 · 25 comments
Open

Consider using RFC 7807 problem details for error responses #19525

bclozel opened this issue Jan 3, 2020 · 25 comments
Labels
status: noteworthy A noteworthy issue to call out in the release notes theme: web-error-handling type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Jan 3, 2020

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.

@bclozel bclozel added type: enhancement A general enhancement status: noteworthy A noteworthy issue to call out in the release notes labels Jan 3, 2020
@bclozel bclozel added this to the 2.x milestone Jan 3, 2020
@jnizet
Copy link
Contributor

jnizet commented Jan 4, 2020

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

  • allow the client to distinguish between unexpected errors, that shouldn't happen if the client is correctly implemented (such as forgetting to provide a required input), and expected ones that must be handled in a specific way by the client (such as providing a value that only the server can check as invalid, like a duplicate identifier or a multipart file with invalid data)
  • allow the client to get an error key, used to display an i18ned error message (the i18n of the error messages is done at client side, as everything else in the pages), sometimes with a map of parameter keys and values (example: { "errorKey": "error.accountBanalnceTooLow", errorParams: { "currentBalance": 100 } })

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.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jan 4, 2020
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jan 16, 2020
@t1
Copy link

t1 commented Jan 20, 2020

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!

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Jan 21, 2020
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Jan 31, 2020
@skurlow
Copy link

skurlow commented Mar 18, 2020

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

@roamingthings
Copy link

roamingthings commented Oct 24, 2020

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
Copy link
Contributor

vpavic commented Dec 11, 2020

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
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Member

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

@t1
Copy link

t1 commented Dec 12, 2020

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
Copy link
Contributor

vy commented Jun 9, 2021

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:

  • String host – The host on which this problem has occurred.
  • List<Violation> violations – The list of constraint violations.
  • Problem cause – The underlying cause, if any.
  • Map<String, Object> metadata – Collection of fields that are unknown during deserialization.

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:

  • BindException
  • ConstraintViolationException
  • HttpMediaTypeNotAcceptableException
  • HttpMediaTypeNotSupportedException
  • HttpMessageNotReadableException
  • HttpRequestMethodNotSupportedException
  • MethodArgumentNotValidException
  • MethodArgumentTypeMismatchException
  • MethodNotAllowedException
  • MissingServletRequestParameterException
  • NotAcceptableStatusException
  • ResponseStatusException
  • ServerWebInputException
  • UnsupportedMediaTypeStatusException
  • WebExchangeBindException

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
Copy link
Contributor

I've created an issue in the Spring Framework spring-projects/spring-framework#27052 for this.

@scottfrederick scottfrederick added the status: blocked An issue that's blocked on an external project change label Nov 1, 2021
@dhh1128
Copy link

dhh1128 commented Nov 2, 2021

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
Copy link

t1 commented Nov 2, 2021

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

@dhh1128
Copy link

dhh1128 commented Nov 2, 2021

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
Copy link
Contributor

breun commented Nov 3, 2021

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
Copy link

t1 commented Nov 3, 2021

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
Copy link
Contributor

sagacity commented Nov 3, 2021

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
Copy link

dhh1128 commented Nov 5, 2021

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
Copy link

Is this fixed with spring-projects/spring-framework#27052 now?

@bclozel
Copy link
Member Author

bclozel commented Dec 2, 2022

@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.

@bclozel bclozel modified the milestones: 3.x, 3.1.x Jan 19, 2023
bobeal added a commit to stellio-hub/stellio-context-broker that referenced this issue Jan 31, 2023
- ProblemDetails support in SB is not totally finished (spring-projects/spring-boot#33885, spring-projects/spring-boot#19525), stick to semi-manual implementation
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.x Apr 17, 2023
@fabiofranco85
Copy link

fabiofranco85 commented Jun 2, 2023

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
Copy link

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
Copy link
Contributor

juliojgd commented Oct 6, 2023

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
Copy link
Member Author

bclozel commented Oct 6, 2023

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

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

@juliojgd
Copy link
Contributor

juliojgd commented Oct 6, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: noteworthy A noteworthy issue to call out in the release notes theme: web-error-handling type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests