jakartaee / rest

Jakarta RESTful Web Services
Other
353 stars 114 forks source link

Clarification: MUST Client throw Subclass of WebApplicationException #1089

Closed mkarg closed 2 years ago

mkarg commented 2 years ago

The specification is unclear about the following question:

If Client receives an HTTP error code, then it MUST throw WebApplicationException or a subclass of that.

But does that imply that a Client MUST throw the subclass or is a runtime allows to simply throw literally the WebApplicationException class ALWAYS instead?

Due to that inaccuracy it is impossible to use try-catch for subclasses in the calling code, but always a second check is needed in case it is NOT the subclass thrown but WebApplicationException itself, so you end up with lots of switch-case.

spericas commented 2 years ago

I don't understand the part "or is a runtime allows to simply throw literally the WebApplicationException class ALWAYS instead". Could you please elaborate and/or provide an example?

mkarg commented 2 years ago

Example: The server sends 500 Server Fault. What does the JAX-RS specification mandate that a compatible JAX-RS Client does?

(a) The JAX-RS Client MUST throw InternalServerErrorException. Due to that, a client application can safely rely on simply catch (final InternalServerErrorException e) { ... } to handle 500 Server Fault.

(b) The JAX-RS Client MAY throw either InternalServerErrorException, or ServerErrorException(500) or WebApplicationException(500). Due to that, a client application cannot rely just on InternalServerErrorException but in fact the only SAFE way to handle 500 Server Fault is try (WebApplicationException e) { if (e.getResponse().getStatus() == INTERNAL_SERVER_ERROR) { ... } }.

As you can see, client application programmers need a clear answer on this question to know if they MAY do (a) or if they MUST do (b).

spericas commented 2 years ago

@mkarg Understood, yes. This needs to be clarified. It should really throw the most specific exception available for the error code received. Maybe a new section in Chapter 5 clarifying this would help.

mkarg commented 2 years ago

Everybody else +1 for me doing that proposed change? Any vetos?

sdaschner commented 2 years ago

I'd +1 Santiago's view on throwing the most specific exception available that matches the response code. So a) in your case.

mkarg commented 2 years ago

I'd +1 Santiago's view on throwing the most specific exception available that matches the response code. So a) in your case.

Just to make clear: This thread is about how the spec was meant to be understood, not what we would love it to be in future. ;-)

NicoNes commented 2 years ago

Everybody else +1 for me doing that proposed change? Any vetos?

+1, clarification would be great. thanks

spericas commented 2 years ago

@mkarg It doesn't seem the spec is very clear about this, as you found out. If we want to include a clarification in 3.1, we should create a PR right away. I plan to kick off the release shortly (finally).

mkarg commented 2 years ago

@spericas I will provide the PR tomorrow, but as it is a spec change de-facto, we cannot fast-track it.

spericas commented 2 years ago

@mkarg That's a good point. I don't think we should wait 2 weeks for this. We can target this for the next release then.