From 726944146b97d6f2e483edd96a5eea57ea6f6fb0 Mon Sep 17 00:00:00 2001
From: James Yuzawa <jtyuzawa@gmail.com>
Date: Fri, 23 Oct 2020 08:29:37 -0400
Subject: [PATCH] Use named exceptions in ChannelPool implementations (#10721)

Motivation:

I was collecting stats for failed promises with a FixedChannelPool and I was bucketing by stats using cause.getSimpleName(). After #9152 was released, the introduction of the anonymous classes make getSimpleName() return "" causing confusion.

Modification:

Use named classes in the ChannelPool implementations. I made them private, but I can change that if you think otherwise.

Result:

The SimpleChannelPool fails the promises with a ChannelPoolFullException. The FixedChannelPool fails the promises with an AcquireTimeoutException. Also AcquireTimeoutException is more specific than just a plain TimeoutException, which is also useful for troubleshooting. If you want different class names, please advise.
---
 .../netty/channel/pool/FixedChannelPool.java  | 23 +++++++++++--------
 .../netty/channel/pool/SimpleChannelPool.java | 20 +++++++++++-----
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java b/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
index 153c2bc197..ae8340205e 100644
--- a/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
+++ b/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java
@@ -194,15 +194,7 @@ public class FixedChannelPool extends SimpleChannelPool {
                     @Override
                     public void onTimeout(AcquireTask task) {
                         // Fail the promise as we timed out.
-                        task.promise.setFailure(new TimeoutException(
-                                "Acquire operation took longer then configured maximum time") {
-
-                            // Suppress a warning since the method doesn't need synchronization
-                            @Override
-                            public Throwable fillInStackTrace() {   // lgtm[java/non-sync-override]
-                                return this;
-                            }
-                        });
+                        task.promise.setFailure(new AcquireTimeoutException());
                     }
                 };
                 break;
@@ -514,4 +506,17 @@ public class FixedChannelPool extends SimpleChannelPool {
 
         return GlobalEventExecutor.INSTANCE.newSucceededFuture(null);
     }
+
+    private static final class AcquireTimeoutException extends TimeoutException {
+
+        private AcquireTimeoutException() {
+            super("Acquire operation took longer then configured maximum time");
+        }
+
+        // Suppress a warning since the method doesn't need synchronization
+        @Override
+        public Throwable fillInStackTrace() {   // lgtm[java/non-sync-override]
+            return this;
+        }
+    }
 }
diff --git a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
index 050de393c6..d7c5dc9afb 100644
--- a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
+++ b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java
@@ -350,12 +350,7 @@ public class SimpleChannelPool implements ChannelPool {
             handler.channelReleased(channel);
             promise.setSuccess(null);
         } else {
-            closeAndFail(channel, new IllegalStateException("ChannelPool full") {
-                @Override
-                public Throwable fillInStackTrace() {
-                    return this;
-                }
-            }, promise);
+            closeAndFail(channel, new ChannelPoolFullException(), promise);
         }
     }
 
@@ -418,4 +413,17 @@ public class SimpleChannelPool implements ChannelPool {
             }
         });
     }
+
+    private static final class ChannelPoolFullException extends IllegalStateException {
+
+        private ChannelPoolFullException() {
+            super("ChannelPool full");
+        }
+
+        // Suppress a warning since the method doesn't need synchronization
+        @Override
+        public Throwable fillInStackTrace() {   // lgtm[java/non-sync-override]
+            return this;
+        }
+    }
 }