andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove ChannelHandler.exceptionCaught(...) as it should only exist in… (#8822)

Browse Source 
Motivation:

ChannelHandler.exceptionCaught(...) was marked as @deprecated as it should only exist in inbound handlers.

Modifications:

Remove ChannelHandler.exceptionCaught(...) and adjust code / tests.

Result:

Fixes https://github.com/netty/netty/issues/8527
This commit is contained in:
Norman Maurer 2019-01-31 20:29:17 +01:00 committed by GitHub
parent 9725cae033
commit e3846c54f6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 32 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
View File

@ -20,7 +20,6 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerAdapter;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelInitializer;
@ -43,7 +42,6 @@ import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpObject;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http2.Http2TestUtil.Http2Runnable;
import io.netty.util.AsciiString;
import io.netty.util.CharsetUtil;
import io.netty.util.concurrent.Future;
@ -667,7 +665,7 @@ public class InboundHttp2ToHttpAdapterTest {
clientDelegator = new HttpResponseDelegator(clientListener, clientLatch, clientLatch2);
p.addLast(clientDelegator);
p.addLast(new ChannelHandlerAdapter() {
p.addLast(new ChannelInboundHandlerAdapter() {
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
Http2Exception e = getEmbeddedHttp2Exception(cause);

5
codec/src/main/java/io/netty/handler/codec/DatagramPacketEncoder.java
View File

@ -136,11 +136,6 @@ public class DatagramPacketEncoder<M> extends MessageToMessageEncoder<AddressedE
encoder.handlerRemoved(ctx);
}
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
encoder.exceptionCaught(ctx, cause);
}
@Override
public boolean isSharable() {
return encoder.isSharable();

8
microbench/src/main/java/io/netty/microbench/channel/EmbeddedChannelHandlerContext.java
View File

@ -19,6 +19,7 @@ import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandler;
import io.netty.channel.ChannelPipeline;
import io.netty.channel.ChannelProgressivePromise;
import io.netty.channel.ChannelPromise;
@ -107,12 +108,15 @@ public abstract class EmbeddedChannelHandlerContext implements ChannelHandlerCon
@Override
public final ChannelHandlerContext fireExceptionCaught(Throwable cause) {
ChannelHandler handler = handler();
try {
handler().exceptionCaught(this, cause);
if (handler instanceof ChannelInboundHandler) {
((ChannelInboundHandler) handler).exceptionCaught(this, cause);
}
} catch (Exception e) {
handleException(e);
}
return null;
return this;
}
@Override

8
transport/src/main/java/io/netty/channel/ChannelHandler.java
View File

@ -188,14 +188,6 @@ public interface ChannelHandler {
*/
void handlerRemoved(ChannelHandlerContext ctx) throws Exception;
/**
* Gets called if a {@link Throwable} was thrown.
*
* @deprecated is part of {@link ChannelInboundHandler}
*/
@Deprecated
void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception;
/**
* Indicates that the same instance of the annotated {@link ChannelHandler}
* can be added to one or more {@link ChannelPipeline}s multiple times

12
transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java
View File

@ -80,16 +80,4 @@ public abstract class ChannelHandlerAdapter implements ChannelHandler {
public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
// NOOP
}
/**
* Calls {@link ChannelHandlerContext#fireExceptionCaught(Throwable)} to forward
* to the next {@link ChannelHandler} in the {@link ChannelPipeline}.
*
* Sub-classes may override this method to change behavior.
*/
@Skip
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
ctx.fireExceptionCaught(cause);
}
}

2
transport/src/main/java/io/netty/channel/ChannelInboundHandler.java
View File

@ -69,7 +69,5 @@ public interface ChannelInboundHandler extends ChannelHandler {
/**
* Gets called if a {@link Throwable} was thrown.
*/
@Override
@SuppressWarnings("deprecation")
void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception;
}

39
transport/src/main/java/io/netty/channel/CombinedChannelDuplexHandler.java
View File

@ -19,9 +19,6 @@ import io.netty.buffer.ByteBufAllocator;
import io.netty.util.Attribute;
import io.netty.util.AttributeKey;
import io.netty.util.concurrent.EventExecutor;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
import java.net.SocketAddress;
@ -31,8 +28,6 @@ import java.net.SocketAddress;
public class CombinedChannelDuplexHandler<I extends ChannelInboundHandler, O extends ChannelOutboundHandler>
extends ChannelDuplexHandler {
private static final InternalLogger logger = InternalLoggerFactory.getInstance(CombinedChannelDuplexHandler.class);
private DelegatingChannelHandlerContext inboundCtx;
private DelegatingChannelHandlerContext outboundCtx;
private volatile boolean handlerAdded;
@ -136,35 +131,7 @@ public class CombinedChannelDuplexHandler<I extends ChannelInboundHandler, O ext
}
outboundCtx = new DelegatingChannelHandlerContext(ctx, outboundHandler);
inboundCtx = new DelegatingChannelHandlerContext(ctx, inboundHandler) {
@SuppressWarnings("deprecation")
@Override
public ChannelHandlerContext fireExceptionCaught(Throwable cause) {
if (!outboundCtx.removed) {
try {
// We directly delegate to the ChannelOutboundHandler as this may override exceptionCaught(...)
// as well
outboundHandler.exceptionCaught(outboundCtx, cause);
} catch (Throwable error) {
if (logger.isDebugEnabled()) {
logger.debug(
"An exception {}" +
"was thrown by a user handler's exceptionCaught() " +
"method while handling the following exception:",
ThrowableUtil.stackTraceToString(error), cause);
} else if (logger.isWarnEnabled()) {
logger.warn(
"An exception '{}' [enable DEBUG level for full stacktrace] " +
"was thrown by a user handler's exceptionCaught() " +
"method while handling the following exception:", error, cause);
}
}
} else {
super.fireExceptionCaught(cause);
}
return this;
}
};
inboundCtx = new DelegatingChannelHandlerContext(ctx, inboundHandler);
// The inboundCtx and outboundCtx were created and set now it's safe to call removeInboundHandler() and
// removeOutboundHandler().
@ -371,7 +338,7 @@ public class CombinedChannelDuplexHandler<I extends ChannelInboundHandler, O ext
}
}
private static class DelegatingChannelHandlerContext implements ChannelHandlerContext {
private static final class DelegatingChannelHandlerContext implements ChannelHandlerContext {
private final ChannelHandlerContext ctx;
private final ChannelHandler handler;
@ -609,7 +576,7 @@ public class CombinedChannelDuplexHandler<I extends ChannelInboundHandler, O ext
return ctx.channel().hasAttr(key);
}
final void remove() {
void remove() {
EventExecutor executor = executor();
if (executor.inEventLoop()) {
remove0();

24
transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java
View File

@ -82,7 +82,7 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap
private static final int MASK_WRITE = 1 << 16;
private static final int MASK_FLUSH = 1 << 17;
private static final int MASK_ALL_INBOUND = MASK_CHANNEL_REGISTERED |
private static final int MASK_ALL_INBOUND = MASK_EXCEPTION_CAUGHT | MASK_CHANNEL_REGISTERED |
MASK_CHANNEL_UNREGISTERED | MASK_CHANNEL_ACTIVE | MASK_CHANNEL_INACTIVE | MASK_CHANNEL_READ |
MASK_CHANNEL_READ_COMPLETE | MASK_USER_EVENT_TRIGGERED | MASK_CHANNEL_WRITABILITY_CHANGED;
private static final int MASK_ALL_OUTBOUND = MASK_BIND | MASK_CONNECT | MASK_DISCONNECT |
@ -154,15 +154,15 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap
* Calculate the {@code executionMask}.
*/
private static int mask0(Class<? extends ChannelHandler> handlerType) {
int mask = MASK_EXCEPTION_CAUGHT;
int mask = 0;
try {
if (isSkippable(handlerType, "exceptionCaught", ChannelHandlerContext.class, Throwable.class)) {
mask &= ~MASK_EXCEPTION_CAUGHT;
}
if (ChannelInboundHandler.class.isAssignableFrom(handlerType)) {
mask |= MASK_ALL_INBOUND;
if (isSkippable(handlerType, "exceptionCaught", ChannelHandlerContext.class, Throwable.class)) {
mask &= ~MASK_EXCEPTION_CAUGHT;
}
if (isSkippable(handlerType, "channelRegistered", ChannelHandlerContext.class)) {
mask &= ~MASK_CHANNEL_REGISTERED;
}
@ -375,7 +375,7 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap
void invokeExceptionCaught(final Throwable cause) {
try {
handler().exceptionCaught(this, cause);
((ChannelInboundHandler) handler()).exceptionCaught(this, cause);
} catch (Throwable error) {
if (logger.isDebugEnabled()) {
logger.debug(
@ -744,7 +744,15 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap
try {
((ChannelOutboundHandler) handler()).read(this);
} catch (Throwable t) {
invokeExceptionCaughtFromOutbound(t);
}
}
private void invokeExceptionCaughtFromOutbound(Throwable t) {
if ((executionMask & MASK_EXCEPTION_CAUGHT) != 0) {
notifyHandlerException(t);
} else {
findContextInbound(MASK_EXCEPTION_CAUGHT).notifyHandlerException(t);
}
}
@ -790,7 +798,7 @@ final class DefaultChannelHandlerContext extends DefaultAttributeMap
try {
((ChannelOutboundHandler) handler()).flush(this);
} catch (Throwable t) {
notifyHandlerException(t);
invokeExceptionCaughtFromOutbound(t);
}
}

2
transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
View File

@ -1019,7 +1019,7 @@ public class DefaultChannelPipeline implements ChannelPipeline {
/**
* Called once a {@link Throwable} hit the end of the {@link ChannelPipeline} without been handled by the user
* in {@link ChannelHandler#exceptionCaught(ChannelHandlerContext, Throwable)}.
* in {@link ChannelInboundHandler#exceptionCaught(ChannelHandlerContext, Throwable)}.
*/
protected void onUnhandledInboundException(Throwable cause) {
try {

13
transport/src/test/java/io/netty/channel/CombinedChannelDuplexHandlerTest.java
View File

@ -89,7 +89,7 @@ public class CombinedChannelDuplexHandlerTest {
}
@Test
public void testExceptionCaughtBothCombinedHandlers() {
public void testExceptionCaught() {
final Exception exception = new Exception();
final Queue<ChannelHandler> queue = new ArrayDeque<>();
@ -101,14 +101,6 @@ public class CombinedChannelDuplexHandlerTest {
ctx.fireExceptionCaught(cause);
}
};
ChannelOutboundHandler outboundHandler = new ChannelOutboundHandlerAdapter() {
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
assertSame(exception, cause);
queue.add(this);
ctx.fireExceptionCaught(cause);
}
};
ChannelInboundHandler lastHandler = new ChannelInboundHandlerAdapter() {
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
@ -118,11 +110,10 @@ public class CombinedChannelDuplexHandlerTest {
};
EmbeddedChannel channel = new EmbeddedChannel(
new CombinedChannelDuplexHandler<>(
inboundHandler, outboundHandler), lastHandler);
inboundHandler, new ChannelOutboundHandlerAdapter()), lastHandler);
channel.pipeline().fireExceptionCaught(exception);
assertFalse(channel.finish());
assertSame(inboundHandler, queue.poll());
assertSame(outboundHandler, queue.poll());
assertSame(lastHandler, queue.poll());
assertTrue(queue.isEmpty());
}

2
transport/src/test/java/io/netty/channel/DefaultChannelPipelineTest.java
View File

@ -1488,7 +1488,7 @@ public class DefaultChannelPipelineTest {
fail("handler was not one of the expected handlers");
}
private static final class CheckOrderHandler extends ChannelHandlerAdapter {
private static final class CheckOrderHandler extends ChannelInboundHandlerAdapter {
private final Queue<CheckOrderHandler> addedQueue;
private final Queue<CheckOrderHandler> removedQueue;
private final AtomicReference<Throwable> error = new AtomicReference<>();

3
transport/src/test/java/io/netty/channel/ReentrantChannelTest.java
View File

@ -258,8 +258,9 @@ public class ReentrantChannelTest extends BaseChannelTest {
throw new Exception("intentional failure");
}
}, new ChannelInboundHandlerAdapter() {
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
ctx.close();
}
});