Support for problem details based on RFC 7807

[rstoyanchev]

This has been requested a number of times but so far has been deferred to Spring Boot which supports error response details. The corresponding https://github.com/spring-projects/spring-boot/issues/19525 issue in Spring Boot has gathered quite a bit of feedback and comments.

For Spring Framework 6 we have an opportunity to revisit this topic and more generally, web error handling. Given a standard such as RFC 7807 for the format of error responses, the Spring Framework could provide more infrastructure that Boot can build on.

• [x] #28187
• [x] #28189
• [x] #28190
• [x] #28665
• [x] #28438
• [x] #28439
• [x] #28814
• [x] #29297

[hantsy]

Spring HATEOAS includes another VndErrors API, it is based on https://github.com/blongden/vnd.error.

[vy]

@rstoyanchev, I have reviewed the commits so far and they look great! :100: Thanks so much for working on this issue. :bow: Given our experiences with RFC 7807 I shared earlier, I would like to share some remarks:

Loss of information while deserializing ProblemDetail

ProblemDetail is encouraged to be extended and users (certainly, we) will extend it. Further, RFC 7807 itself is also pretty open about this. This said, the current class doesn't provide a way to capture the extra information while getting deserialized from an extended ProblemDetail class. Consider the following use case:

1. Server contains a RichProblemDetail extending ProblemDetail with a String host field. It emits this from an HTTP endpoint to the client.
2. Client receives the JSON and deserializes it to ProblemDetail – the information of host field is totally lost.

I suggest introducing a generic field (e.g., Map<String, Object> attributes) to capture the information of unknown fields.

Public ProblemDetail methods

ProblemDetail is implemented as a mutable class with many utility methods. For a class that is expected to be extended, this imposes considerable mundane work that needs to be implemented by users. Further, current model implies implementation difficulties for users embracing immutable models. Implementing ProblemDetail as an interface instead can address all these shortcomings.

[rstoyanchev]

@vy thanks for the review and comments.

For ProblemDetail extensions, my thought was that sub-classes will use Jackson's @JsonTypeInfo (or similar) like is done in Zalando for example, but I've yet to confirm this end to end, which will be the goal for #28190. I suspect we might need an interface for a "mix-in" but I'm not sure until I try. I'm also thinking on the client side there could be an option to specify the class to deserialize to.

For Map<String, Object> attributes, it adds a level of nesting with an "attributes" key, which for once reveals a framework detail, and generally leads to questions about how to customize or eliminate this. An application might then try to add Jackson's @JsonUnwrapped which brings us back to extending.

For mutability, indeed immutable would be my preferred choice, but I'm just thinking, (de-)serialization friendliness aside, that the case here is rather specific. We're talking about an exception type, or a type carried within an exception that short-circuits regular handling and follows a straight line towards being serialized to the response body. Moreover, there are already reasons to allow mutability, such as when a ResponseErrorException is extended, or handled in places like ResponseEntityExceptionHandler. It doesn't feel like there are strong reasons against this while it certainly simplifies the situation in terms of (de-)serializing and extensions.

[vy]

Thanks so much for the prompt reply @rstoyanchev. (Apologies for my late reaction; vacation + other priorities get in my way.)

For ProblemDetail extensions, my thought was that sub-classes will use Jackson's @JsonTypeInfo (or similar) like is done in Zalando for example, but I've yet to confirm this end to end, which will be the goal for #28190. I suspect we might need an interface for a "mix-in" but I'm not sure until I try. I'm also thinking on the client side there could be an option to specify the class to deserialize to.

I am not able to follow your reasoning here. Libraries will expect a ProblemDetail. Using a mix-in that doesn't extend from ProblemDetail will simply break this contract. Why do you explicitly avoid making ProblemDetail more extension-friendly using an interface but rather opt for other alternatives? What would be the shortcomings of making ProblemDetail an interface?

For Map<String, Object> attributes, it adds a level of nesting with an "attributes" key, which for once reveals a framework detail, and generally leads to questions about how to customize or eliminate this.

I see your point regarding the questions about how to customize or eliminate attributes. That said, how else can a user capture the unmapped information while deserializing?

An application might then try to add Jackson's @JsonUnwrapped which brings us back to extending.

Sorry, but I couldn't follow. Would you mind elaborating on this, please? (I know what @JsonUnwrapped is.)

For mutability, indeed immutable would be my preferred choice, but I'm just thinking, (de-)serialization friendliness aside, that the case here is rather specific. We're talking about an exception type, or a type carried within an exception that short-circuits regular handling and follows a straight line towards being serialized to the response body. Moreover, there are already reasons to allow mutability, such as when a ResponseErrorException is extended, or handled in places like ResponseEntityExceptionHandler.

Again, I am not able to follow. If ErrorResponseException (I think that is what you meant by ResponseErrorException, right?) is extended, a new ProblemDetail can be instantiated. How does making ProblemDetail mutable help here?

It doesn't feel like there are strong reasons against this while it certainly simplifies the situation in terms of (de-)serializing and extensions.

Mind sharing a more tangible example on how mutability of ProblemDetail simplifies (de)serialization and/or extension?

[rstoyanchev]

@vy my turn to apologize for not coming back to this until now.

I've created a sample project. It demonstrates use of @ControllerAdvice that extends ResponseEntityExceptionHandler to customize the output for any Spring MVC exception as well as for any ResponseErrorException. It does two things:

1. Customize the type and detail message for each exceptions. Although the "detail" for each Spring MVC exception has been carefully considered and minimized, an application might still choose to customize the error details to avoid revealing implementation details (e.g. framework used, possibly version if messages change) as well as for consistency across all application errors.
2. Re-create ProblemDetail as ExtendedProblemDetail that has an additional host field from a single place for the entire application.

On the client side, we will provide some conveniences in #28190 to be able to decode the error response body to a target class. This could be ExtendedProblemDetail if you have it on the classpath, it could be your own extension of ProblemDetail, any class that matches the expected JSON, or even a Map.

Apologies for not answering your questions directly, but I'm hoping the sample helps to provide more context, and we can continue with questions from there.

One comment/question to you. For you suggestion to add Map<String, Object> attributes, and also looking at your https://github.com/spring-projects/spring-boot/issues/19525#issuecomment-857551764 under the Boot issue, I think you are making a distinction between extra fields that are known and always added (e.g. host) vs others that are not known upfront and need to be supported through a generic map of attributes? In other words, a more dynamic mechanism to add any attribute, in addition to the ability to extend ProblemDetail in order to support well-known fields that are always added. This makes sense to me to have such a Map of attributes in ProblemDetail.

[rstoyanchev]

A couple of updates:

1. Content negotiation now supports application/problem+json, see 28189
2. Client exceptions support decoding error content, see 28190

The sample now uses WebClient and RestTemplate and decodes the error content.

[wimdeblauwe]

I am trying to better understand the scope of this issue. I had a look to the sample project. Is the intention of providing "just" a ProblemDetails class that everybody could use when writing their own exception handlers? Or is there also the intention of providing out-of-the-box exception handling that translates exceptions into ProblemDetails?

In my error-handling-spring-boot-starter I translate exceptions into JSON notation (not using the RFC7807). See https://foojay.io/today/better-error-handling-for-your-spring-boot-rest-apis/ if you want a nice overview of what the library does.

The JSON I send back as an error response is represented by the ApiErrorResponse. It also has a generic Map<String, Object> properties field which allows to add extra properties in the response.

[vy]

@rstoyanchev, thanks so much for keeping on involving us during the development. I've checked the sample project. I have shared my remarks below.

1. I still think ProblemDetail should be an interface and certainly not a mutable POJO. For instance, ExtendedProblemDetail misses withHost() to get aligned with ProblemDetail surface. (I know, ExtendedProblemDetail is just there to test extensibility. But my point rather is people will repeat that mistake and miss those conventional methods.) You modify the exception (through its ProblemDetail) in controller advices. These don't look, if I may say, good, given generally how well-thought Spring touch points are.
2. We have an extensive RFC 7807 test suite against corner cases we have encountered through the years; thrown exceptions from a controller advice should be converted to a problem detail, etc. There are also certain exceptions that can happen before the controller method is called and don't get caught by the @ExceptionHandler methods. This suite ensures that application failures always get rendered as a problem detail. We can share these suite of cases, if you wish.
3. We provide selective logging on the problem details; e.g., log every response of type /problem/bad-request at level WARN. Here the type and level are user provided configurations. Would you consider adding this feature to the Spring? If not, what would be the best interception point for us given the new ErrorResponse family.
4. RFC 7807 implementations out there in the wild also expose a throwable variant of the ProblemDetail extending from RuntimeException. What is your take on this?

One comment/question to you. For you suggestion to add Map<String, Object> attributes, and also looking at your spring-projects/spring-boot#19525 (comment) under the Boot issue, I think you are making a distinction between extra fields that are known and always added (e.g. host) vs others that are not known upfront and need to be supported through a generic map of attributes? In other words, a more dynamic mechanism to add any attribute, in addition to the ability to extend ProblemDetail in order to support well-known fields that are always added. This makes sense to me to have such a Map of attributes in ProblemDetail.

Exactly! Having a ProblemDetail#attributes field of type Map<String, Object> enables access to problem detail fields not known upfront.

[rstoyanchev]

Apologies for the delayed responses. I discovered I've had issues with receiving GitHub notifications.

@wimdeblauwe, having ProblemDetail, along with the ErrorResponse (an interface to represent a full error response, including status, headers, and a ProblemDetail body), allows us to build support in a few key areas.

• All internal web framework exceptions now implement ErrorResponse to expose the status, headers, and body to use. To enable this, an application extends (the existing) RespnoseEntityExceptionHandler, which has been updated for this, and declare it as @ControllerAdvice. This automates the handling of our own internal exceptions and also provides a central place to customize the rendering of any ErrorResponse exception.

• ProblemDetail and ErrorResponse are supported as return types from @ExceptionHandler and controller methods, similar to ResponseEntity. The "status" property of ProblemDetail is used to set the response status, and its "instance" property may be filled in to reflect the current URL path.

• When serializing ProblemDetail, the Jackson HttpMessageConverter and Encoder return application/problem+json as a first choice, ensuring it gets selected first, as long as it meets the client requested media types. Also, when rendering ProblemDetail, if we don't find a suitable media type, we fall back on application/problem+json.

• RestClientResponseException and WebClientResponseException allow decoding the body to some target type. This isn't specific to having ProblemDetail but helps on the client side to decode an error response.

To come back to your question, the goal for 6.0 is to make use of a "standard" format such as RFC 7807. This includes out-of-the-box exception handling for our own internal exceptions, via RespnoseEntityExceptionHandler, and the same can be used to customize any ErrorResponse exception or extension of ErrorResponseException.

This is a foundation to build upon. I can imagine taking it further and allowing more ways to customize error response details. For example, we could consider using message codes from properties as a way of customizing the detail message. We are certainly open to other ideas as well.

[rstoyanchev]

@vy, no worries and thanks for the continued feedback.

I wasn't expecting that ExtendedProblemDetail needs withHost(). Those are just convenience methods that a subclass may choose to not provide. I agree however that ProblemDetail should aim to be as plain as possible, and would be okay to drop the withXyz() methods in ProblemDetail based on this feedback.

For mutability, our internal web framework exceptions in the ErrorResponse hierarchy all follow the same pattern, initializing error details with default values in the constructor, and then remaining open to further customizations, either via subclasses, or in @ExceptionHandler methods, or in our internal response handling where we may fill in "instance" with the URL path.

There is clearly a need to remain open to customizations after an exception is first initialized, and before it is rendered. How did you imagine making that possible? You mention you don't like ResponseEntityExceptionHandler. This is a class that ensures that all built-in exceptions are broken down into status, headers, and body, so you can customize each individually by exception type. It's analogous to DefaultHandlerExceptionResolver but with body rendering. Again if you have alternatives in mind, please do share.

For interface, I don't see the need for different impelmentations, but rather a simple container of properties to render. To connect to your other question about a ProblemDetail variant that extends from RuntimeException, making an exception itself be a ProblemDetail means exceptions properties like stackTrace need to somehow be excluded. For this reason the two are separated intentionally. ErrorResponse is the interface that an exception can implement and that in turn contains ProblemDetail that contains only what should be rendered.

There are also certain exceptions that can happen before the controller method is called and don't get caught by the @ExceptionHandler methods. This suite ensures that application failures always get rendered as a problem detail. We can share these suite of cases, if you wish.

That would be great. I expect Spring Boot will add support for ErrorResponse and ProblemDetail, which means that any exceptions that escape should now be handled consistently between Framework and Boot. That said we might be able to proactively catch more of these exceptions if feasible. I think NoHandlerFoundException is one such exception but that can be caught with an @ControllerAdvice I think.

For logging, I'll refer back to ResponseEntityExceptionHandler as the intended central place from where to handle internal, web framework, and any ErrorResponse exception. We could consider expanding it with support for logging by ProblemDetail "type".

[rstoyanchev]

For dynamic properties, I've created #28665.

[knoobie]

I'm wondering if it would make sense to move the ProblemDetail to spring-core. Allowing to extend it with something like CompanyProblemDetail within base libraries without having to enforce the additional dependency on spring-web.

I've seen libraries often using spring-core or spring-security-core as dependency in base libraries to allow re-using of common spring interfaces like Resources or GrantedAuthority. In my opinion ProblemDetail falls in the same category.

[rstoyanchev]

As an HTTP abstraction, ProblemDetail belongs in org.springframework.http, not any lower. ProblemDetail is nothing but a simple holder of the properties and it wouldn't make sense for Spring Security to use it without the rest of the support, as outlined in https://github.com/spring-projects/spring-framework/issues/27052#issuecomment-1160657627, which in turn requires spring-web. In any case, the goal is to provide an abstraction for spring-web processing.

[rstoyanchev]

@vy and @wimdeblauwe, the work under #28814 to allow external customization, including internationalization, of the "detail" for Spring MVC / WebFlux and for any ErrorResponseException is now largely complete. I would appreciate your taking a look and any comments. Probably best under #28814.

[rstoyanchev]

All tasks are now complete.

[vy]

@rstoyanchev, this PR is in our roadmap and we will extensively review it in a week or two. I will really appreciate it if you can allow us some time.

[rstoyanchev]

@vy, I presume you mean the changes under #28814? Ahead of next week's RC2 would be much appreciated, but either way, I look forward to your feedback.

[rstoyanchev]

Linking here to a related discussion for the class name ProblemDetail in #29303.

[rstoyanchev]

For everyone following here, there is a specific corner case described in #29378.