Bug: Client errors are not being preserved out to the client

[sdavids13]

I am using charon to sit in front of an Elasticsearch cluster. If someone sends a bad search request to Elasticsearch it responds with a 400 response code with the response body containing a helpful error message. I would like to preserver the response code and error message but charon is returning a 500 response code (internal server error) with an error message of "400 Bad Request".

After looking through the code the intent is there assuming you aren't attempting retries in the RequestForwarder. I turned on the charon.tracing and found that the onForwardError method was being called in LoggingTraceInterceptor which makes me believe the shouldRetry method is not working as documented (client errors should not be retried). If it did make it past that if statement it looks like it would send back the upstream error response.

[mkopylec]

Retrying on 4xx is disabled by default. Look at the test. The behaviour you are describing can occure when charon.retrying.retry-on.client-http-error: true is set.

[sdavids13]

I'm not changing any of the default values, I was looking through the code last night and everything seemed like it should work logically. I will need to step through my code to see what is going on, will report back soon.

[sdavids13]

Sorry for the late response, so I stepped through the code and found this...

The default "exception" is java.lang.Exception which ends up passing the shouldRetry every time because the Exception class is assignable to all subclasses. Since it passes that check it will go ahead and propagate the exception up instead of passing the underlying response.

To try to remedy this I tried to disable the "exception" value by setting it to an empty value: charon.retrying.retry-on.exceptions: "". I then found that in the properties configuration that doing that it ended up disabling all exception handling (even if the serverHttpError config was set to true).

So.. to finally work around this issue I configured: charon.retrying.retry-on.exceptions: org.springframework.web.client.HttpServerErrorException

[sdavids13]

@mkopylec I looked into why the test was passing and found that the default retry options have been overriden for this test:

  retrying:
    max-attempts: 3
    retry-on:
      client-http-error: false
      server-http-error: false
      exceptions: java.nio.channels.UnresolvedAddressException

Since the catch-all java.lang.Exception was removed it makes the tests pass... If you remove the one line: exceptions: java.nio.channels.UnresolvedAddressException (and rely on the default) the test fails.

[mkopylec]

Ok, I see the point now. I'll try to fix the bug.

[dilgerma]

this hit me too. By not changing the defaults, retries are enabled because the HttpStatusCode Exception is propagated and rethrown and later then handled by tomcat, which sets status 500 on the response. Please fix this, this is really annoying. Thanks!

[mkopylec]

I'm not planning to fix the issue in 3.x.x version. I'm working on 4.x.x version which will have it fixed. If you really need this to be fixed please create a PR.

[mkopylec]

The 4.0.0 version has been released and it should not contain the issue descirbed here.