You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by gg...@apache.org on 2026/10/16 17:10:49 UTC

(storm) branch master updated: Make strict deserialization mode fatal for IOException-wrapped failures (#9159)

This is an automated email from the ASF dual-hosted git repository.

GGraziadei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git


The following commit(s) were added to refs/heads/master by this push:
     new 69f7bfe14 Make strict deserialization mode fatal for IOException-wrapped failures (#9159)
69f7bfe14 is described below

commit 69f7bfe14a976ee763875831b4b2f8c31e3181b2
Author: Richard Zowalla <13...@users.noreply.github.com>
AuthorDate: Fri Oct 16 19:10:42 2026 +0200

    Make strict deserialization mode fatal for IOException-wrapped failures (#9159)
    
    * Make strict deserialization mode fatal for IOException-wrapped failures
    
    With topology.tuple.deserialization.strict.enable set, a decode failure is
    rethrown from DeserializingConnectionCallback.recv and reaches
    StormServerHandler.exceptionCaught, which swallows anything with an
    IOException in its cause chain. Failures from a corrupt or oversized
    compressed frame wrap an IOException, so they closed the connection and
    dropped the batch instead of terminating the worker.
    
    Strict-mode failures are now wrapped in FatalDeserializationException, and
    StormServerHandler does not apply the IOException exemption to it. Genuine
    connection errors keep the existing handling.
    
    Fixes #9157
    
    * Handle strict deserialization failures directly in exceptionCaught
    
    FatalDeserializationException is now checked at the top of
    StormServerHandler.exceptionCaught instead of going through an empty
    allowed-exception set and the catch (Error) block.
    
    The worker is terminated from a separate thread. Calling exit() on the
    netty event loop thread blocks forever, because the worker shutdown hook
    closes the netty server and waits for that same event loop to finish.
    
    The tests now call exceptionCaught itself, with the terminator injected.
    
    * Address the review comments, moving reusable helper methods into Utils
    
    ---------
    
    Co-authored-by: Gianluca Graziadei <gr...@gmail.com>
---
 docs/Metrics.md                                    |   2 +-
 docs/Serialization.md                              |   2 +-
 storm-client/src/jvm/org/apache/storm/Config.java  |   6 +-
 .../messaging/DeserializingConnectionCallback.java |  10 +-
 ...ver.java => FatalDeserializationException.java} |  17 ++--
 .../org/apache/storm/messaging/netty/IServer.java  |   2 +
 .../netty/NettyUncaughtExceptionHandler.java       |   6 +-
 .../org/apache/storm/messaging/netty/Server.java   |   6 ++
 .../storm/messaging/netty/StormServerHandler.java  |  34 ++++++-
 .../src/jvm/org/apache/storm/utils/Utils.java      |  18 ++++
 .../DeserializingConnectionCallbackTest.java       |  44 ++++++++-
 .../messaging/netty/StormServerHandlerTest.java    | 104 +++++++++++++++++++++
 12 files changed, 228 insertions(+), 23 deletions(-)

diff --git a/docs/Metrics.md b/docs/Metrics.md
index 9a3a9fc83..433f9aae3 100644
--- a/docs/Metrics.md
+++ b/docs/Metrics.md
@@ -302,7 +302,7 @@ Be aware that the `__system` bolt is an actual bolt so regular bolt metrics desc
 
 `dequeuedMessages` is a throwback to older code where there was an internal queue between the server and the bolts/spouts.  That is no longer the case and the value can be ignored.
 `enqueued` is a map between the address of the remote worker and the number of tuples that were sent from it to this worker.
-`deserializationFailures` is the number of incoming messages that failed to deserialize and were dropped. When `topology.tuple.deserialization.strict.enable` is set, deserialization failures are not dropped or counted; they propagate instead. Non-IO failures terminate the worker, while failures caused by an `IOException`, such as a corrupt compressed frame, close the connection and leave the worker running.
+`deserializationFailures` is the number of incoming messages that failed to deserialize and were dropped. When `topology.tuple.deserialization.strict.enable` is set, deserialization failures are not dropped or counted; they terminate the worker instead.
 
 ##### Send (Netty Client)
 
diff --git a/docs/Serialization.md b/docs/Serialization.md
index 4d3841ad8..714c234f3 100644
--- a/docs/Serialization.md
+++ b/docs/Serialization.md
@@ -131,7 +131,7 @@ Be aware that the topology-wide form enables compression for *every* remote-boun
 | `topology.tuple.compression.threshold` | `1460` | Minimum serialized tuple size, in bytes, before compression is attempted. Tuples at or below this size are sent uncompressed. The default matches the typical Ethernet TCP MSS, so payloads that already fit in a single network frame are never compressed. |
 | `storm.compression.zstd.level` | `3` | Zstd compression level. Supported range is 1–19; levels 20–22 (ultra mode) are prohibited because of their memory requirements. |
 | `topology.tuple.compression.max.decompressed.bytes` | `10485760` (10 MB) | Upper bound on the decompressed size of a single tuple. Decompression that would exceed this limit fails, guarding against malicious or corrupt payloads. |
-| `topology.tuple.deserialization.strict.enable` | `false` | Makes tuple deserialization failures fatal: an incoming message that fails to decode kills the receiving worker instead of being dropped and counted. This restores the pre-3.1.0 behavior and is mainly useful for debugging, because a persistent bad frame will put the worker in a restart loop. A failure rooted in an `IOException`, such as a corrupt or oversized compressed frame, is an exception: the receiving worker closes that c [...]
+| `topology.tuple.deserialization.strict.enable` | `false` | Makes tuple deserialization failures fatal: an incoming message that fails to decode kills the receiving worker instead of being dropped and counted. This restores the pre-3.1.0 behavior and is mainly useful for debugging, because a persistent bad frame will put the worker in a restart loop. This includes failures rooted in an `IOException`, such as a corrupt or oversized compressed frame. |
 
 #### How decompression works
 
diff --git a/storm-client/src/jvm/org/apache/storm/Config.java b/storm-client/src/jvm/org/apache/storm/Config.java
index 9c275f82c..9fec88235 100644
--- a/storm-client/src/jvm/org/apache/storm/Config.java
+++ b/storm-client/src/jvm/org/apache/storm/Config.java
@@ -1653,10 +1653,8 @@ public class Config extends HashMap<String, Object> {
      * By default a message that fails to decode on the receiving worker is dropped and counted, and the worker keeps
      * running. When set to {@code true}, any deserialization failure propagates and the worker exits, restoring the
      * pre-3.1.0 behavior. Be aware that a single corrupt frame from a peer then kills the worker, and the supervisor
-     * restarts it into the same failure, so a persistent bad frame results in a restart loop. Failures that wrap an
-     * {@code IOException}, such as a broken or oversized compressed frame, do not reach the exit path: the messaging
-     * layer treats them like a connection error, drops the batch being read, and leaves the worker running while the
-     * peer reconnects.
+     * restarts it into the same failure, so a persistent bad frame results in a restart loop. This includes failures
+     * that wrap an {@code IOException}, such as a broken or oversized compressed frame.
      * Default: {@code false}.
      */
     @IsBoolean
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java b/storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java
index cee8776a1..3967ccc6d 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java
@@ -105,10 +105,16 @@ public class DeserializingConnectionCallback implements IConnectionCallback, IMe
         ArrayList<AddressedTuple> ret = new ArrayList<>(batch.size());
         for (TaskMessage message : batch) {
             Tuple tuple;
+            byte[] payload = message.message();
             try {
-                tuple = des.deserialize(message.message());
+                tuple = des.deserialize(payload);
             } catch (Exception e) {
-                if (strictMode || !isToleratedDeserializationFailure(e)) {
+                if (strictMode) {
+                    int size = payload == null ? -1 : payload.length;
+                    throw new FatalDeserializationException("Failed to deserialize a message of " + size
+                                                            + " bytes destined for task " + message.task(), e);
+                }
+                if (!isToleratedDeserializationFailure(e)) {
                     throw e;
                 }
                 deserializationFailures.incrementAndGet();
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java b/storm-client/src/jvm/org/apache/storm/messaging/FatalDeserializationException.java
similarity index 59%
copy from storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java
copy to storm-client/src/jvm/org/apache/storm/messaging/FatalDeserializationException.java
index 4b095d680..ad6dae573 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/FatalDeserializationException.java
@@ -10,12 +10,15 @@
  * and limitations under the License.
  */
 
-package org.apache.storm.messaging.netty;
+package org.apache.storm.messaging;
 
-import org.apache.storm.shade.io.netty.channel.Channel;
-
-public interface IServer {
-    void channelActive(Channel c);
-
-    void received(Object message, String remote, Channel channel) throws InterruptedException;
+/**
+ * Wraps a tuple deserialization failure under {@link org.apache.storm.Config#TOPOLOGY_TUPLE_DESERIALIZATION_STRICT_ENABLE}.
+ * The messaging layer treats it as fatal even when the cause chain contains an exception it would otherwise tolerate,
+ * such as the {@code IOException} of a corrupt compressed frame.
+ */
+public class FatalDeserializationException extends RuntimeException {
+    public FatalDeserializationException(String message, Throwable cause) {
+        super(message, cause);
+    }
 }
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java b/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java
index 4b095d680..5f34365f7 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/netty/IServer.java
@@ -18,4 +18,6 @@ public interface IServer {
     void channelActive(Channel c);
 
     void received(Object message, String remote, Channel channel) throws InterruptedException;
+
+    default void terminating() {}
 }
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/netty/NettyUncaughtExceptionHandler.java b/storm-client/src/jvm/org/apache/storm/messaging/netty/NettyUncaughtExceptionHandler.java
index e7bd779dd..a2d8998a7 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/NettyUncaughtExceptionHandler.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/netty/NettyUncaughtExceptionHandler.java
@@ -22,7 +22,7 @@ public class NettyUncaughtExceptionHandler implements Thread.UncaughtExceptionHa
     @Override
     public void uncaughtException(Thread t, Throwable e) {
         try {
-            LOG.error("Uncaught exception in netty " + e.getCause());
+            LOG.error("Uncaught exception in netty", e);
         } catch (Throwable err) {
             // Doing nothing (probably due to an oom issue) and hoping Utils.handleUncaughtException will handle it
         }
@@ -30,9 +30,9 @@ public class NettyUncaughtExceptionHandler implements Thread.UncaughtExceptionHa
         try {
             Utils.handleUncaughtException(e);
         } catch (Throwable throwable) {
-            LOG.error("Exception thrown while handling uncaught exception " + throwable.getCause());
+            LOG.error("Exception thrown while handling uncaught exception", throwable);
         }
         LOG.info("Received error in netty thread.. terminating server...");
-        Runtime.getRuntime().exit(1);
+        Utils.exitProcessFromNewThread(1, "Uncaught exception in a netty thread");
     }
 }
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/netty/Server.java b/storm-client/src/jvm/org/apache/storm/messaging/netty/Server.java
index 3d54cab0d..c1a790d68 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/Server.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/netty/Server.java
@@ -162,6 +162,7 @@ class Server extends ConnectionWithStatus implements IStatefulObject, ISaslServe
      */
     @Override
     public void close() {
+        closing = true;
         allChannels.close().awaitUninterruptibly();
         workerEventLoopGroup.shutdownGracefully().awaitUninterruptibly();
         bossEventLoopGroup.shutdownGracefully().awaitUninterruptibly();
@@ -270,6 +271,11 @@ class Server extends ConnectionWithStatus implements IStatefulObject, ISaslServe
         allChannels.add(c);
     }
 
+    @Override
+    public void terminating() {
+        closing = true;
+    }
+
     @Override
     public void received(Object message, String remote, Channel channel) throws InterruptedException {
         List<TaskMessage> msgs;
diff --git a/storm-client/src/jvm/org/apache/storm/messaging/netty/StormServerHandler.java b/storm-client/src/jvm/org/apache/storm/messaging/netty/StormServerHandler.java
index 0088395d1..3c8202711 100644
--- a/storm-client/src/jvm/org/apache/storm/messaging/netty/StormServerHandler.java
+++ b/storm-client/src/jvm/org/apache/storm/messaging/netty/StormServerHandler.java
@@ -16,6 +16,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
+import org.apache.storm.messaging.FatalDeserializationException;
 import org.apache.storm.shade.io.netty.channel.Channel;
 import org.apache.storm.shade.io.netty.channel.ChannelHandlerContext;
 import org.apache.storm.shade.io.netty.channel.ChannelInboundHandlerAdapter;
@@ -27,9 +28,16 @@ public class StormServerHandler extends ChannelInboundHandlerAdapter {
     private static final Logger LOG = LoggerFactory.getLogger(StormServerHandler.class);
     private static final Set<Class<?>> ALLOWED_EXCEPTIONS = new HashSet<>(Arrays.asList(new Class<?>[]{ IOException.class }));
     private final IServer server;
+    private final Runnable terminator;
 
     public StormServerHandler(IServer server) {
+        this(server, StormServerHandler::terminateWorker);
+    }
+
+    // Package-private for testing.
+    StormServerHandler(IServer server, Runnable terminator) {
         this.server = server;
+        this.terminator = terminator;
     }
 
     @Override
@@ -53,6 +61,12 @@ public class StormServerHandler extends ChannelInboundHandlerAdapter {
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+        if (Utils.exceptionCauseIsInstanceOf(FatalDeserializationException.class, cause)) {
+            // Strict deserialization mode: fatal even if the failure wraps an IOException.
+            terminate(ctx, cause, "Strict deserialization failure from {}, terminating worker");
+            ctx.close();
+            return;
+        }
         try {
             LOG.error("server errors in handling the request from {}", ctx.channel().remoteAddress().toString(), cause);
         } catch (Throwable err) {
@@ -62,9 +76,25 @@ public class StormServerHandler extends ChannelInboundHandlerAdapter {
             Utils.handleUncaughtException(cause, ALLOWED_EXCEPTIONS, false);
             ctx.close();
         } catch (Error error) {
-            LOG.info("Received error in netty thread.. terminating server...");
-            Runtime.getRuntime().exit(1);
+            terminate(ctx, cause, "Received error in netty thread from {}, terminating server...");
+        }
+
+    }
+
+    // Stops the server delivering further batches (the event loops run until the exit completes), then exits.
+    // Logging comes last and is guarded: a failure to log (e.g. an OOM) must not skip the termination.
+    private void terminate(ChannelHandlerContext ctx, Throwable cause, String message) {
+        server.terminating();
+        terminator.run();
+        try {
+            LOG.error(message, ctx.channel().remoteAddress(), cause);
+        } catch (Throwable err) {
+            // nop
         }
+    }
 
+    // Not on this thread: the worker shutdown hook closes the netty server, which waits for this event loop.
+    private static void terminateWorker() {
+        Utils.exitProcessFromNewThread(1, "Fatal error in the netty server");
     }
 }
diff --git a/storm-client/src/jvm/org/apache/storm/utils/Utils.java b/storm-client/src/jvm/org/apache/storm/utils/Utils.java
index feebf0fab..cd0ffdeb4 100644
--- a/storm-client/src/jvm/org/apache/storm/utils/Utils.java
+++ b/storm-client/src/jvm/org/apache/storm/utils/Utils.java
@@ -67,6 +67,7 @@ import java.util.Stack;
 import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.jar.JarFile;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -121,6 +122,7 @@ import org.slf4j.LoggerFactory;
 
 public class Utils {
     public static final Logger LOG = LoggerFactory.getLogger(Utils.class);
+    private static final AtomicBoolean EXIT_REQUESTED = new AtomicBoolean();
     public static final String DEFAULT_STREAM_ID = "default";
     private static final Set<Class<?>> defaultAllowedExceptions = Collections.emptySet();
     private static final List<String> LOCALHOST_ADDRESSES = Lists.newArrayList("localhost", "127.0.0.1", "0:0:0:0:0:0:0:1");
@@ -528,6 +530,22 @@ public class Utils {
         Runtime.getRuntime().exit(val);
     }
 
+    /**
+     * Exits the process like {@link #exitProcess(int, String)}, but from a new thread. Needed when the caller runs on
+     * a thread that a shutdown hook waits for, such as a Netty event loop: the worker shutdown hook closes the Netty
+     * server, which awaits its event loops, while {@code Runtime.exit()} blocks the calling thread until the hooks
+     * complete. Idempotent: once an exit is in progress, further calls return immediately instead of starting another
+     * thread that would block in {@code Runtime.exit()} forever.
+     */
+    public static void exitProcessFromNewThread(int val, String msg) {
+        if (!EXIT_REQUESTED.compareAndSet(false, true)) {
+            return;
+        }
+        Thread exitThread = new Thread(() -> exitProcess(val, msg), "process-exit");
+        exitThread.setDaemon(false);
+        exitThread.start();
+    }
+
     public static String uuid() {
         return UUID.randomUUID().toString();
     }
diff --git a/storm-client/test/jvm/org/apache/storm/messaging/DeserializingConnectionCallbackTest.java b/storm-client/test/jvm/org/apache/storm/messaging/DeserializingConnectionCallbackTest.java
index de8bf62b2..fbc5d6061 100644
--- a/storm-client/test/jvm/org/apache/storm/messaging/DeserializingConnectionCallbackTest.java
+++ b/storm-client/test/jvm/org/apache/storm/messaging/DeserializingConnectionCallbackTest.java
@@ -170,12 +170,29 @@ public class DeserializingConnectionCallbackTest {
         WorkerState.ILocalTransferCallback transfer = mock(WorkerState.ILocalTransferCallback.class);
         DeserializingConnectionCallback callback = new DeserializingConnectionCallback(conf, context, transfer);
 
-        assertThrows(KryoException.class, () -> callback.recv(Collections.singletonList(taskMessage(truncated))));
+        FatalDeserializationException thrown = assertThrows(FatalDeserializationException.class,
+                                                            () -> callback.recv(Collections.singletonList(taskMessage(truncated))));
+        assertTrue(thrown.getCause() instanceof KryoException, "expected a KryoException cause but was: " + thrown.getCause());
 
         verify(transfer, never()).transfer(any());
         assertEquals(0L, callback.getAndResetDeserializationFailures());
     }
 
+    @Test
+    public void testStrictModeReportsNullPayloadAsFatal() {
+        Map<String, Object> conf = baseConf();
+        conf.put(Config.TOPOLOGY_TUPLE_DESERIALIZATION_STRICT_ENABLE, true);
+
+        WorkerState.ILocalTransferCallback transfer = mock(WorkerState.ILocalTransferCallback.class);
+        DeserializingConnectionCallback callback = new DeserializingConnectionCallback(conf, context, transfer);
+
+        FatalDeserializationException thrown = assertThrows(FatalDeserializationException.class,
+                                                            () -> callback.recv(Collections.singletonList(taskMessage(null))));
+        assertTrue(thrown.getMessage().contains("-1 bytes"), "unexpected message: " + thrown.getMessage());
+
+        verify(transfer, never()).transfer(any());
+    }
+
     @Test
     public void testStrictModeMakesUnknownTaskFailureFatal() {
         Map<String, Object> conf = baseConf();
@@ -184,8 +201,29 @@ public class DeserializingConnectionCallbackTest {
         WorkerState.ILocalTransferCallback transfer = mock(WorkerState.ILocalTransferCallback.class);
         DeserializingConnectionCallback callback = new DeserializingConnectionCallback(conf, context, transfer);
 
-        assertThrows(TupleDeserializationException.class,
-                     () -> callback.recv(Collections.singletonList(taskMessage(unknownSourceTaskTuple()))));
+        FatalDeserializationException thrown = assertThrows(FatalDeserializationException.class,
+            () -> callback.recv(Collections.singletonList(taskMessage(unknownSourceTaskTuple()))));
+        assertTrue(thrown.getCause() instanceof TupleDeserializationException,
+                   "expected a TupleDeserializationException cause but was: " + thrown.getCause());
+
+        verify(transfer, never()).transfer(any());
+        assertEquals(0L, callback.getAndResetDeserializationFailures());
+    }
+
+    @Test
+    public void testStrictModeMakesIoExceptionFailureFatal() {
+        Map<String, Object> conf = baseConf();
+        conf.put(Config.TOPOLOGY_TUPLE_DESERIALIZATION_STRICT_ENABLE, true);
+        conf.put(Config.TOPOLOGY_TUPLE_COMPRESSION_ENABLE, true);
+        byte[] fakeZstd = {(byte) 0x28, (byte) 0xB5, (byte) 0x2F, (byte) 0xFD, 0x00, 0x01, 0x02, 0x03};
+
+        WorkerState.ILocalTransferCallback transfer = mock(WorkerState.ILocalTransferCallback.class);
+        DeserializingConnectionCallback callback = new DeserializingConnectionCallback(conf, context, transfer);
+
+        FatalDeserializationException thrown = assertThrows(FatalDeserializationException.class,
+            () -> callback.recv(Collections.singletonList(taskMessage(fakeZstd))));
+        assertTrue(Utils.exceptionCauseIsInstanceOf(IOException.class, thrown),
+                   "expected an IOException in the cause chain but was: " + thrown);
 
         verify(transfer, never()).transfer(any());
         assertEquals(0L, callback.getAndResetDeserializationFailures());
diff --git a/storm-client/test/jvm/org/apache/storm/messaging/netty/StormServerHandlerTest.java b/storm-client/test/jvm/org/apache/storm/messaging/netty/StormServerHandlerTest.java
new file mode 100644
index 000000000..bf8d281bb
--- /dev/null
+++ b/storm-client/test/jvm/org/apache/storm/messaging/netty/StormServerHandlerTest.java
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The ASF licenses this file to you under the Apache License, Version
+ * 2.0 (the "License"); you may not use this file except in compliance with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
+ * and limitations under the License.
+ */
+
+package org.apache.storm.messaging.netty;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import org.apache.storm.messaging.FatalDeserializationException;
+import org.apache.storm.shade.io.netty.channel.Channel;
+import org.apache.storm.shade.io.netty.channel.ChannelHandlerContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class StormServerHandlerTest {
+
+    private Runnable terminator;
+    private IServer server;
+    private Channel channel;
+    private ChannelHandlerContext ctx;
+    private StormServerHandler handler;
+
+    @BeforeEach
+    public void setUp() {
+        terminator = mock(Runnable.class);
+        server = mock(IServer.class);
+        channel = mock(Channel.class);
+        when(channel.remoteAddress()).thenReturn(new InetSocketAddress("localhost", 6700));
+        ctx = mock(ChannelHandlerContext.class);
+        when(ctx.channel()).thenReturn(channel);
+        handler = new StormServerHandler(server, terminator);
+    }
+
+    @Test
+    public void testIoExceptionClosesConnection() {
+        handler.exceptionCaught(ctx, new RuntimeException("Failed to decompress", new IOException("corrupt frame")));
+
+        verify(ctx).close();
+        verify(terminator, never()).run();
+        verify(server, never()).terminating();
+    }
+
+    @Test
+    public void testSocketIoExceptionClosesConnection() {
+        // what netty itself reports for a connection error: a bare IOException
+        handler.exceptionCaught(ctx, new IOException("Connection reset by peer"));
+
+        verify(ctx).close();
+        verify(terminator, never()).run();
+    }
+
+    @Test
+    public void testFatalDeserializationWrappingIoExceptionTerminates() {
+        handler.exceptionCaught(ctx, new FatalDeserializationException("strict",
+            new RuntimeException("Failed to decompress", new IOException("corrupt frame"))));
+
+        verify(server).terminating();
+        verify(terminator).run();
+        verify(ctx).close();
+    }
+
+    @Test
+    public void testWrappedFatalDeserializationTerminates() {
+        handler.exceptionCaught(ctx, new RuntimeException("rethrown",
+            new FatalDeserializationException("strict", new IOException("corrupt frame"))));
+
+        verify(server).terminating();
+        verify(terminator).run();
+        verify(ctx).close();
+    }
+
+    @Test
+    public void testFatalDeserializationTerminatesEvenIfLoggingFails() {
+        when(channel.remoteAddress()).thenThrow(new OutOfMemoryError("no heap left to log"));
+
+        handler.exceptionCaught(ctx, new FatalDeserializationException("strict", new IOException("corrupt frame")));
+
+        verify(server).terminating();
+        verify(terminator).run();
+        verify(ctx).close();
+    }
+
+    @Test
+    public void testOtherFailureTerminates() {
+        handler.exceptionCaught(ctx, new IllegalStateException("unexpected"));
+
+        verify(server).terminating();
+        verify(terminator).run();
+        verify(ctx, never()).close();
+    }
+}