Do not produce an empty response when ServiceCodec raises an exception #200
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| hostname, path, mappedPath, req.content(), req, promise); | ||
| } catch (Exception e) { | ||
| logger.warn("{} Unexpected exception from a decoder:", ctx.channel(), e); | ||
| respond(ctx, reqSeq, req, HttpResponseStatus.BAD_REQUEST); |
There was a problem hiding this comment.
How about appending e as 5th parameter?
Motivation: When a ServiceCodec.decodeRequest() raises an exception, HttpServerHandler will close the connection immediately without sending anything. Instead, the server should send a '400 Bad Request' response. Modifications: - HttpServerHandler now catches an exception raised by ServiceCodec.decodeRequest() and responds with a '400 Bad Request' response. - ThriftServiceCodec.decodeRequest() now catches an exception raised by TProtocol.readMessageBegin() and returns a failed DecoderResult instead of raising an exception. Result: - Armeria server sends a 400 Bad Request even if ServiceCodec.decodeRequest() raises an exception. - ThriftServiceCodec.decodeRequest() deals with a bad request better.
trustin
force-pushed
the
fix_empty_response_on_decoder_exception
branch
from
9a1af81 to
974ed33
Compare
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
When a ServiceCodec.decodeRequest() raises an exception,
HttpServerHandler will close the connection immediately without sending
anything. Instead, the server should send a '400 Bad Request' response.
Modifications:
ServiceCodec.decodeRequest() and responds with a '400 Bad Request'
response.
TProtocol.readMessageBegin() and returns a failed DecoderResult
instead of raising an exception.
Result:
ServiceCodec.decodeRequest() raises an exception.