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

DefaultResponseErrorHandler does not handle status values outside of HttpStatus enum [SPR-17461] #21993

Closed
spring-issuemaster opened this issue Nov 2, 2018 · 5 comments
Labels

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Nov 2, 2018

Liam Bryan opened SPR-17461 and commented

The DefaultResponseErrorHandler does not handle HttpStatus values not specified in the enum values.  For example; a service we call returns 299 which reports as an error in the latest spring versions (linked code).

In previous versions (checked 4.3.14 as used in current project), this wasn't reported as an error; however the thrown UnknownHttpStatusCodeException (which is caught, and converted to false for there being an error) contained the responseBody as one of its arguments, which in turn closed the InputStream or the response meaning that the body of the response could not be parsed to a ResponseEntity anywhere.

 

At present, this is being gotten around with a custom class:

public final class UnrecognisedHttpStatusCodeFriendlyResponseErrorHandler extends DefaultResponseErrorHandler {
    @Override
    public boolean hasError(final ClientHttpResponse response) throws IOException {
        try {
            final HttpStatus.Series statusSeries = HttpStatus.Series.valueOf(response.getRawStatusCode());
            return (HttpStatus.Series.CLIENT_ERROR.equals(statusSeries))
                    || (HttpStatus.Series.SERVER_ERROR.equals(statusSeries));
        } catch (final IllegalArgumentException iaEx) {
            // Series was not in 1-5...  Something's probably wrong...
            return true;
        }
    }
} 

Affects: 4.3.14, 5.1.2

Reference URL:

Issue Links:

  • #21971 RestTemplate does not throw exception for custom error codes
  • #21145 DefaultResponseErrorHandler wastes the body of a response with an unknown status
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 2, 2018

Juergen Hoeller commented

Are you possibly referring to the same problem as #21971? We intend to revisit this for 5.1.3.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 2, 2018

Liam Bryan commented

Juergen Hoeller: Ah, I did a search for DefaultErrorResponseHandler as that was where the problem lay.

Officially 'no' since that's for custom error codes; however I'm more than happy for this to be addressed there.

Are there further updates for Spring 4 planned?  If so there's a bigger issue there which I highlight here, where using the responseBody as an argument to an exception which is caught, and eventually returned as not an error means that one gets an IOException when trying to parse the response body through what appears to be normal usage of the classes.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 23, 2018

Juergen Hoeller commented

As for the problem with the response body getting swallowed, this should be fixed in 4.3.15+ (#21145). In general, those RestClientException variants which contain the response body are meant to be terminal, i.e. not caught and suppressed while proceeding to process the response normally... a rule we violated outselves in 4.3.14, sorted out as of 4.3.15.

It's not clear to me why the status code 299 would throw an exception in the latest versions. The hasError implementation would simply not consider it as an error; is this subsequently falling over its own feet elsewhere? Your custom workaround seems to solve the problem with custom error codes in #21971 but does not seem to differ in behavior in any other respect. Am I missing something?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 1, 2019

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 8, 2019

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.