Let CombinedChannelDuplexHandler correctly handle exceptionCaught. Re… #4702
Conversation
…lated to [#4528] Motivation: ChannelInboundHandler and ChannelOutboundHandler both can implement exceptionCaught(...) method and so we need to dispatch to both of them. Modifications: - Correctly first dispatch exceptionCaught to the ChannelInboundHandler but also make sure the next handler it will be dispatched to will be the ChannelOutboundHandler - Add removeInboundHandler() and removeOutboundHandler() which allows to remove one of the combined handlers - Let *Codec extends it and not ChannelHandlerAppender Result: Correctly handle events and also have same behavior as in 4.0
|
@trustin @slandelle PTAL... I think this is a better fix . @trustin I would even propose we remove |
|
Would this work when an inboundHandler or an outboundHandler also implements ChannelOutboundHandler or ChannelInboundHandler respectively? |
|
@trustin I think I could make it work yes... I just wanted to have a "quick fix" first. Once this is merged I could also try to make it work for the case you just mentioned. WDYT ? |
|
@trustin so at the moment it just support that it is either inbound or outbound handler (just as it was before). |
|
@normanmaurer tested against AHC test suite, all green! |
|
@normanmaurer Could you add some sanity checks then? i.e. rejecting an inbound handler that implements |
|
@normanmaurer LGTM sans the missing sanity checks. |
|
@trustin huh... we already check this: Also what you think about my other comment about removing ChannelHandlerAppender for now ? |
|
@normanmaurer Ugh, you're right. :-) LGTM then! and yeah, let's remove |
|
@normanmaurer - lgtm! I have no objection for removing |
…lated to [#4528]
Motivation:
ChannelInboundHandler and ChannelOutboundHandler both can implement exceptionCaught(...) method and so we need to dispatch to both of them.
Modifications:
Result:
Correctly handle events and also have same behavior as in 4.0