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 spring-projects/spring-boot#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.

• Add types to represent RFC 7807 problem details and exceptions #28187
• Support "application/problem+json" as the response Content-Type #28189
• Enable access to an RFC 7807 ProblemDetail formatted error response from the client side #28190
• Allow dynamic properties in ProblemDetail #28665
• Update documentation for RFC 7807 support #28438
• Add WebFlux equivalent of ResponseEntityExceptionHandler #28439
• Allow for external customization and i8n of the "detail" of an RFC 7807 response #28814
• Add native support for ExceptionHandler and ProblemDetail #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! 💯 Thanks so much for working on this issue. 🙇 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 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.

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

[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 #27052 (comment), 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.