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

Ccustom mapper should not always return a 503 when the mapping fails #3609

Closed
JeffreyThijs opened this issue Jan 22, 2024 · 0 comments · Fixed by #3610
Closed

Ccustom mapper should not always return a 503 when the mapping fails #3609

JeffreyThijs opened this issue Jan 22, 2024 · 0 comments · Fixed by #3610

Comments

@JeffreyThijs
Copy link
Contributor

When using a custom http mapper the HttpBasedMessageMapping will always return a 503 code when the mapping fails (

result.fail(new ServerErrorException(HttpURLConnection.HTTP_UNAVAILABLE, httpResponseAsyncResult.cause()));
). This gives the requesting party the assumption that the service is currently not capable of handling the request but maybe it will be able to do so in the future. So when you receive a 503, you might continuously retry doing the request until the server will become ready which might never happen in case you constantly send faulty data.

Preferred behavior would be to return a more suitable error, like just using the status code of the outgoing request when it fails.

JeffreyThijs added a commit to JeffreyThijs/hono that referenced this issue Jan 22, 2024
@sophokles73 sophokles73 changed the title custom mapper should not always return a 503 when the mapping fails Ccustom mapper should not always return a 503 when the mapping fails Jan 27, 2024
sophokles73 pushed a commit that referenced this issue Jan 27, 2024
The logic for invoking a custom mapper for upstream commands has been adapted
to consider the (failure) status code returned by the mapper when generating a corresponding
ServiceInvocationException to be sent back in the reply to the downstream sender.

fixes #3609
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant