RESTEasy Reactive: support json error RFC

[FroMage]

https://tools.ietf.org/html/rfc7807 appears to be super useful for most JSON/XML APIs, so we should support it.

[FroMage]

@gytis this is what @geoand talked to you about.

[gytis]

Yes, I'm happy to take this one.

[FroMage]

Excellent!

[FroMage]

https://github.com/quarkusio/quarkus-quickstarts/pull/809/files#diff-568cb5a2fe72fb07ec9f5b4b90618a6af7cc6ffa2b8061ff715476091a16d085R108 has a pretty common error handler that produces:

{
"exceptionType": exception.getClass().getName(),
"code": statusCode,
"error": exception.getMessage()
}

while the RFC is more:

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

See https://tools.ietf.org/html/rfc7807#section-3.1

Not sure how to reconcile the two.

I can't find docs on what sort of format Angular expects, if it does at all? https://angular.io/api/common/http/HttpErrorResponse#description

[FroMage]

I can't find anything in the react docs about error format either.

Do @phillip-kruger or @ia3andy have any knowledge about this?

[phillip-kruger]

Sorry I don't know, but I like the idea :)

[gytis]

Why not add exception message as a title? Exception class name, though, should probably be a custom member, because it doesn't align with any of the default object members.

[ia3andy]

I don't think Angular expects any format, it will wrap the response, that's it..

[pschyma]

@FroMage React doesn't expect anything. It's the developers choice what they use.

Btw. maybe https://github.com/zalando/problem maybe useful for implementing this.

[FroMage]

Thanks, it's good to know it exists, and perhaps we should point our users to it if they need custom problem responses.

But I don't think we need this ourselves to just turn the default exceptions into a problem JSON.

[gytis]

@FroMage, I finally have some more time to start properly working on this. I've been thinking maybe we could specify a new exception type (e.g. ProblemException extends RuntimeException) which a user could build with a builder setting various problem parameters and throw it. Then we could handle it with a JAX-RS mapper and build an appropriate response. We could similarly handle other common errors (not found, validation etc.).

[FroMage]

For extra detail, sure, but this should be the correct error body even for other errors if the clients wants JSON or if the response type is JSON.

[gytis]

Sure, we could handle any unhandled error this way. But the RFC is not limited to JSON, it also has XML response type: application/problem+xml. Or do you prefer to limit this to JSON?

[FroMage]

Oh, XML, sure why not. I forgot it existed ;)

[pazkooda]

Consider looking at just opensourced extension we developed at TietoEvry https://github.com/TietoEVRY/quarkus-resteasy-problem

You may find it useful.

Regarding integrating this into Quarkus suggestion from dev mailing list is to put this into Quarkiverse which will try to do in near future.

[FroMage]

That looks nice, but I'm not sure if this should really live outside of quarkus itself. I mean: I don't see why most users would want to opt out of having this feature when they import resteasy (classic or reactive).

[geoand]

So @FroMage you would prefer to integrate what @pazkooda and team developed this into RESTEasy Classic and/or Reactive ?

[pazkooda]

I've just made same question/concern on dev mailing list. But is not me to decide here :). In case you'd be interesting in incorporating our part (somehow) don't hesitate to contact me or @lwitkowski for details. We are open to help out with Reactive port if needed.

[FroMage]

Well, yes, it sounds like this could be a new core extension that both resteasy versions import by default, no?

[geoand]

I can see the merit, I'm not against it

[pazkooda]

Just for you info: we already evaluated XML support - see this (closed) PullReq#20. Yet we were not sure if some side effects of implementation are worth introducing it in early stage of this extension. Waiting for community feedback before introducing this to core of our code.

[gytis]

I just looked through the extension code and I agree, it looks nice. I only have a question to @FroMage and @geoand. Is it OK to include a Zalando dependency to the core extension? Or would it be better to replace the Problem classes that come from there with new implementations inside the extension?

[geoand]

I would be in favor of the latter

[FroMage]

Same.

[pazkooda]

You might then be interested in (still-in-progress) PullReq#62.

Consider providing suggestions there if you'd like it to be flavored the way it will suit you in Quakrus

[pazkooda]

master branch of our extension is now Zalando-independent, yet with optional support when lib is present on classpath.

[pazkooda]

RESTeasy Reactive support is now also present in our code, see: PullReq#67

[geoand]

@pazkooda would you be interested in contributing that module to RESTEasy Reactive?

[pazkooda]

Yes, we can take a initial look at it maybe even this week and start implementing it in the following ones. Looking forward for your support to meet project quality standards 🙂.

@lwitkowski, @Wertros: interested?

[pazkooda]

@pazkooda would you be interested in contributing that module to RESTEasy Reactive?

BTW: I assume non-Reactive part is also to be covered by same set of standardized responses on errors, correct?

[geoand]

We don't need to cover it, especially not in the beginning

[PhilippParis]

Hi! any plans for integrating the extension into resteasy-classic? or will it stay outside of quarkus?

[pazkooda]

Actually once investigated means for integration of it to Quarkus Core I've identified that:

• it looks at first glance that doing this only for Reactive will most likely make our standalone extension colliding with core,
• non-Reactive scenarios will require standalone extension to be present - we don't want to separate Reactive out of it just Core prefers to have it Reactive-only first.
• I cannot spend time at work on things that will (most likely) break our code (standalone extension)

Bearing in mind all above and fact that I've actually changed my professional project to non-Quarkus based technologies I'll most likely did not find time for this anytime soon.

[mkgl]

Hi there, I recently noticed Spring 6 / Boot 3 comes with the ProblemDetail API as built-in functionality. Should we expect Quarkus and/or RESTEasy to follow along?

Side note, I think this pairs nicely with the high-level "detail exposure policy" config, as proposed in #25264 👍 (not sure how tied are those two).

Edit: I personally wouln't mind if it's for reactive only at first, or likely ever.

[geoand]

Yes, we do plan to address this at some point

[FroMage]

Apparently, the original RFC had time to be obsoleted by https://www.rfc-editor.org/rfc/rfc9457 :)

[RobertDiebels]

Apparently, the original RFC had time to be obsoleted by https://www.rfc-editor.org/rfc/rfc9457 :)

I recently stumbled upon this RFC and was about to file an FR to support it. Is anyone working on this? I'd be happy to submit a PR just need some guidance on where to start.

[FroMage]

A PR would be great.

I'm not too sure of how exactly to provide support. This requires JSON and/or XML and those are optional modules of Quarkus REST. But it'd be nice to get this out of the box.

I suppose we can start by trying to add a ProblemDetails type (singular or plural TBD, the spec uses both at random) that maps the JSON type defined in the spec, in the quarkus/independent-projects/resteasy-reactive/common/runtime module, so that it can be used by both the client and server.

Once we can build that type, remains to be tested whether it produces the right JSON and XML representations for the jackson and jsonb extensions, in the extensions/resteasy-reactive/rest-jackson/deployment/src/test/ and extensions/resteasy-reactive/rest-jsonb/deployment/src/test/ extensions.

Same for the client extensions, we want to make sure they deserialise properly.

Once we have that, remains to be seen how we want people to interact with them, I suppose there are several ways, from the server's perspective:

• Returning a ProblemDetails type from the endpoint. This should just work OOTB, though remains to be decided if the content-type should be automatically set to application/problem+json automatically or manually by the user.
• Throwing a new subtype of WebApplicationException, perhaps named ProblemDetailsException which accepts a ProblemDetails member, for which we'd have a custom exception mapper setting the right content type.
• Turning any Exception thrown into a ProblemDetails response. Now, this might be problematic unless it is only enabled via configuration, or an annotation on an endpoint, or for clients that explicitly accept the application/problem+json mime type? It could be problematic because that would change existing application behaviour and turn existing error text responses into json. This might even be problematic for DEV mode where we generate HTML errors, not sure what to do in this case.

Another option is to opt-in to the ProblemDetails behaviour by importing a quarkus-rest-problem-details extension, but this seems like an anoying thing to require for something that could work OOTB. I'm kinda leaning towards a method/class/package annotation to enable it.

In any case, the first 2 points (defining and mapping the ProblemDetails type to JSON/XML, and custom exception type) can advance while we brainstorm about the third point.

WDYT @geoand ?

[RobertDiebels]

@FroMage If I might add my 2 cents.

Regarding the user interaction comments.

• I'm not sure about the current behavior regarding the application/json media-type. Does the current implementation set the content-type when it is not explicitly declared in a @Produces annotation (we generate our APIs from OpenAPI specs, so forgive the question)? If that is the case I would support both an explicitly declared application/problem+json and one derived from the return type so that the behavior is consistent.
• How would this work for a ValidationException? I'm assuming that there already is a mapper to deal with those, are there any quirks that could come from mapping those to a ProblemDetailsException?
• If this would be supported, should it also be possible to add custom properties to the returned ProblemDetails and how would that be achieved?

Regarding the extension comment.

I think an extension is not much of a hindrance, especially given that the JSON module is already optional. Adding an extension often is a minor effort given the benefit that they provide. Not to mention that an opt-in means that a user will be aware that the introduction of the extension might break the current behavior. Meaning that features like you mentioned above, such as returning a ProblemDetails for any Exception would be less of an issue.

[FroMage]

• I'm not sure about the current behavior regarding the application/json media-type. Does the current implementation set the content-type when it is not explicitly declared in a @Produces annotation (we generate our APIs from OpenAPI specs, so forgive the question)? If that is the case I would support both an explicitly declared application/problem+json and one derived from the return type so that the behavior is consistent.

Well, I suppose we might set the application/problem+json mime type if there is no content type set, or there is a less precise content type such as application/json. And same for XML. Now, on the topic of incompatible mime types, if the normal operation of an endpoint is to return a plain/text content, such as a String, and for some reason this throws an exception and we've chosed to turn exceptions into ProblemDetails then, yes I suppose we'll have to override the content type to set it properly so that the response is valid, regarless of OpenAPI schemas. OpenAPI users will be able to set @Produces({"text/plain", "application/problem+json"}) manually, or we could override our OpenAPI extension to do this automatically.

• How would this work for a ValidationException? I'm assuming that there already is a mapper to deal with those, are there any quirks that could come from mapping those to a ProblemDetailsException?

Good question. I don't think there's an existing mapper, since errors are plain text, or HTML. But we'd have to check and the intuitive thing is that they end up properly nested in ProblemDetails

• If this would be supported, should it also be possible to add custom properties to the returned ProblemDetails and how would that be achieved?

I suppose it could take a Map<String,Object> for custom properties, all to be serialised via Jackson or Jsonb.

I think an extension is not much of a hindrance, especially given that the JSON module is already optional. Adding an extension often is a minor effort given the benefit that they provide. Not to mention that an opt-in means that a user will be aware that the introduction of the extension might break the current behavior. Meaning that features like you mentioned above, such as returning a ProblemDetails for any Exception would be less of an issue.

Yeah, that's a valid point. I'm not convinced either way. I can see good points on both sides.

Thanks a lot for your feedback!

[RobertDiebels]

If this would be supported, should it also be possible to add custom properties to the returned ProblemDetails and how would that be achieved?

I suppose it could take a Map<String,Object> for custom properties, all to be serialised via Jackson or Jsonb.

This would be for the use-case: "Turning any Exception thrown into a ProblemDetails" right? (Since my bullet-points were intended to match up with yours ) If so, what would be the source of those additional properties? Additional static properties would be doable I assume, but anything that varies may be an issue. Or would it be reasonable to not support additional properties for this use-case?

Thanks a lot for your feedback!

Happy to help 👍 .

To emphasize again, I'd love to make a PR for this. So, once a decision has been made regarding the extension vs. OOTB discussion, please do not hesitate to tag me and I'll get on it.

[geoand]

I'm kinda leaning towards a method/class/package annotation to enable it.

I think it makes sense

[vsevel]

Yes, we do plan to address this at some point

+1 for having some support

[vsevel]

have you looked at https://github.com/TietoEVRY/quarkus-resteasy-problem ?

[FroMage]

Perhaps we don't have to implement it ourselves, and just propose to that extension to move to Quarkiverse to get even more users :)

[akil-rails]

The repo has been moved to https://github.com/quarkiverse/quarkus-resteasy-problem, but I suppose some steps are pending to be done?

[FroMage]

Oh, that's cool. Is it normal that it's using the com.tietoevry.quarkus group ID, though?

I suppose on the Quarkus REST end, we should add a point to quarkus-resteasy-problem in our documentation.