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

Document whether code can rely on specific subclasses of HttpClientErrorException/HttpServerErrorException being thrown from DefaultResponseErrorHandler #25067

Closed
mjustin opened this issue May 13, 2020 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@mjustin
Copy link

mjustin commented May 13, 2020

From perusing the code, I discovered that DefaultResponseErrorHandler will throw subclasses of HttpClientErrorException/HttpServerErrorException, created by HttpClientErrorException.create()/HttpServerErrorException.create(). For instance, if the request returned a 404 error, a HttpClientErrorException.NotFound exception will be thrown.

That said, I don't see any documentation indicating that the subclass is what will be thrown, rather than a the superclass HttpClientErrorException. I assume that it is intended that I can rely on this subclass being thrown, so I can write code like the following:

try {
    return restTemplate.getForObject(url, MyType.class);
} catch (HttpClientErrorException.NotFound e) {
    // Do something specific to 404
} catch (HttpClientErrorException e) {
    // Handle other client errors
}

instead of the clunkier:

try {
    return restTemplate.getForObject(url, MyType.class);
} catch (HttpClientErrorException e) {
    if (HttpStatus.NOT_FOUND.equals(e.getStatusCode())) {
        // Do something specific to 404
    } else {
        // Handle other client errors
    }
}

It would be useful if it were documented that a consumer of the RestTemplate/DefaultResponseErrorHandler can rely on these specific subclasses being thrown. This documentation could live in either or both the DefaultResponseErrorHandler Javadocs or the general Spring documentation.

The closest the current documentation comes to this is in the method Javadocs for DefaultResponseErrorHandler.handleError(response, statusCode). But all that states is that it will throw HttpClientErrorException or HttpServerErrorException, not that it will throw the appropriate subclass if it exists. It has a "See also" section to the code that creates the subclasses, but that doesn't tell the user that it will be calling that, merely that the given code is related in some way.

The default implementation throws an HttpClientErrorException if the status code is CLIENT_ERROR, an HttpServerErrorException if it is SERVER_ERROR, or an UnknownHttpStatusCodeException in other cases.

See Also:
HttpClientErrorException.create(org.springframework.http.HttpStatus, java.lang.String, org.springframework.http.HttpHeaders, byte[], java.nio.charset.Charset), HttpServerErrorException.create(org.springframework.http.HttpStatus, java.lang.String, org.springframework.http.HttpHeaders, byte[], java.nio.charset.Charset)

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 13, 2020
@rstoyanchev rstoyanchev self-assigned this May 22, 2020
@rstoyanchev rstoyanchev added this to the 5.2.7 milestone May 22, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 22, 2020
@rstoyanchev
Copy link
Contributor

This was done in #1956 but indeed no pointers were left in the docs. Thanks for the reminder.

FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

3 participants