Repository: netty/netty
Issue: Add some warn log about unknown channel option?
Labels: defect

Should add some WARN log [at this point](https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/channel/DefaultChannelConfig.java#L109) ? like [this one](https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java#L242) ?

Issue comments:
@wangyuntao we warn in `ServerBootstrap` and `Bootstrap`. The rest is up to the user when the methods return false.
@normanmaurer , I checked the code and found that [Bootstrap](https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/bootstrap/Bootstrap.java#L272) has warn logs, but [ServerBootstrap](https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java#L144) does't.
@wangyuntao ups... let me fix this then.
Should be fixed by https://github.com/netty/netty/pull/6235
Fixed by https://github.com/netty/netty/pull/6235

Pull request review comments:
May be need rethrow or catch Exception instead of Throwable?
@fenik17 I just moved the code around that was there before tho... @Scottmitch @nmittler @trustin WDYT ?
fine for this PR. We will look at `Throwable` in a more broad sense for https://github.com/netty/netty/issues/6096
@Scottmitch  +1 :)

Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java b/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java
index d6d0e3efde7..8140ebf9d68 100644
--- a/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java
+++ b/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java
@@ -30,6 +30,7 @@
 import io.netty.util.concurrent.EventExecutor;
 import io.netty.util.concurrent.GlobalEventExecutor;
 import io.netty.util.internal.StringUtil;
+import io.netty.util.internal.logging.InternalLogger;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -436,6 +437,33 @@ final Map<AttributeKey<?>, Object> attrs() {
         return copiedMap(attrs);
     }
 
+    static void setChannelOptions(
+            Channel channel, Map<ChannelOption<?>, Object> options, InternalLogger logger) {
+        for (Map.Entry<ChannelOption<?>, Object> e: options.entrySet()) {
+            setChannelOption(channel, e.getKey(), e.getValue(), logger);
+        }
+    }
+
+    static void setChannelOptions(
+            Channel channel, Map.Entry<ChannelOption<?>, Object>[] options, InternalLogger logger) {
+        for (Map.Entry<ChannelOption<?>, Object> e: options) {
+            setChannelOption(channel, e.getKey(), e.getValue(), logger);
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private static void setChannelOption(
+            Channel channel, ChannelOption<?> option, Object value, InternalLogger logger) {
+        try {
+            if (!channel.config().setOption((ChannelOption<Object>) option, value)) {
+                logger.warn("Unknown channel option '{}' for channel '{}'", option, channel);
+            }
+        } catch (Throwable t) {
+            logger.warn(
+                    "Failed to set channel option '{}' with value '{}' for channel '{}'", option, channel, channel, t);
+        }
+    }
+
     @Override
     public String toString() {
         StringBuilder buf = new StringBuilder()
diff --git a/transport/src/main/java/io/netty/bootstrap/Bootstrap.java b/transport/src/main/java/io/netty/bootstrap/Bootstrap.java
index 7e2716ec975..17a84d46d56 100644
--- a/transport/src/main/java/io/netty/bootstrap/Bootstrap.java
+++ b/transport/src/main/java/io/netty/bootstrap/Bootstrap.java
@@ -266,15 +266,7 @@ void init(Channel channel) throws Exception {
 
         final Map<ChannelOption<?>, Object> options = options0();
         synchronized (options) {
-            for (Entry<ChannelOption<?>, Object> e: options.entrySet()) {
-                try {
-                    if (!channel.config().setOption((ChannelOption<Object>) e.getKey(), e.getValue())) {
-                        logger.warn("Unknown channel option: " + e);
-                    }
-                } catch (Throwable t) {
-                    logger.warn("Failed to set a channel option: " + channel, t);
-                }
-            }
+            setChannelOptions(channel, options, logger);
         }
 
         final Map<AttributeKey<?>, Object> attrs = attrs0();
diff --git a/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java b/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java
index c142a1144e2..98730e9426b 100644
--- a/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java
+++ b/transport/src/main/java/io/netty/bootstrap/ServerBootstrap.java
@@ -141,7 +141,7 @@ public ServerBootstrap childHandler(ChannelHandler childHandler) {
     void init(Channel channel) throws Exception {
         final Map<ChannelOption<?>, Object> options = options0();
         synchronized (options) {
-            channel.config().setOptions(options);
+            setChannelOptions(channel, options, logger);
         }
 
         final Map<AttributeKey<?>, Object> attrs = attrs0();
@@ -204,13 +204,13 @@ public ServerBootstrap validate() {
     }
 
     @SuppressWarnings("unchecked")
-    private static Entry<ChannelOption<?>, Object>[] newOptionArray(int size) {
+    private static Entry<AttributeKey<?>, Object>[] newAttrArray(int size) {
         return new Entry[size];
     }
 
     @SuppressWarnings("unchecked")
-    private static Entry<AttributeKey<?>, Object>[] newAttrArray(int size) {
-        return new Entry[size];
+    private static Map.Entry<ChannelOption<?>, Object>[] newOptionArray(int size) {
+        return new Map.Entry[size];
     }
 
     private static class ServerBootstrapAcceptor extends ChannelInboundHandlerAdapter {
@@ -237,13 +237,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
             child.pipeline().addLast(childHandler);
 
             for (Entry<ChannelOption<?>, Object> e: childOptions) {
-                try {
-                    if (!child.config().setOption((ChannelOption<Object>) e.getKey(), e.getValue())) {
-                        logger.warn("Unknown channel option: " + e);
-                    }
-                } catch (Throwable t) {
-                    logger.warn("Failed to set a channel option: " + child, t);
-                }
+                setChannelOptions(child, childOptions, logger);
             }
 
             for (Entry<AttributeKey<?>, Object> e: childAttrs) {


Test patch: