Copilot commented on code in PR #15926:
URL: https://github.com/apache/dubbo/pull/15926#discussion_r2650293859


##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyServer.java:
##########
@@ -282,4 +283,42 @@ protected io.netty.channel.Channel getBossChannel() {
     protected Map<String, Channel> getServerChannels() {
         return channels;
     }
+
+    @Override
+    public void fireChannelEvent(ChannelEvent event) {
+        Collection<Channel> channels = getChannels();
+        if (CollectionUtils.isEmpty(channels)) {
+            return;
+        }
+        for (Channel channel : channels) {
+            try {
+                if (channel.isConnected()) {
+                    fireChannelEventToChannel(channel, event);
+                }
+            } catch (Throwable e) {
+                logger.warn(
+                        TRANSPORT_FAILED_CLOSE,
+                        "",
+                        "",
+                        "Failed to fire channel event to channel: " + channel 
+ ", event: " + event,
+                        e);
+            }
+        }
+    }
+
+    /**
+     * Fire ChannelEvent to the channel.
+     * The event will be handled by protocol-specific handlers.
+     *
+     * @param channel the Dubbo channel
+     * @param event the channel event to fire
+     */
+    private void fireChannelEventToChannel(Channel channel, ChannelEvent 
event) {
+        if (channel instanceof NettyChannel) {
+            io.netty.channel.Channel nettyChannel = ((NettyChannel) 
channel).getNioChannel();
+            if (nettyChannel != null && nettyChannel.isActive()) {
+                nettyChannel.pipeline().fireUserEventTriggered(event);
+            }
+        }
+    }

Review Comment:
   The `fireChannelEventToChannel` method in `NettyServer` and 
`NettyPortUnificationServer` contains duplicate code. Consider extracting this 
common logic into a shared utility method or base class to improve 
maintainability and reduce code duplication.



##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractServer.java:
##########
@@ -198,4 +199,16 @@ public void disconnected(Channel ch) throws 
RemotingException {
         }
         super.disconnected(ch);
     }
+
+    @Override
+    public void fireChannelEvent(ChannelEvent event) {
+        // Default implementation does nothing.
+        // Subclasses can override this method to implement protocol-specific 
event handling.
+        logger.warn(
+                INTERNAL_ERROR,
+                "unknown error in remoting module",
+                "",
+                "The fireChannelEvent method is not implemented for "
+                        + getClass().getName() + ", event: " + event);
+    }

Review Comment:
   The default implementation of `fireChannelEvent` in `AbstractServer` logs a 
warning when called, but this warning may be too severe for servers that 
legitimately don't need to support channel events (like NettyHttp3Server or 
older NettyServer implementations). Consider either changing the log level to 
debug or providing a way for subclasses to opt out of the warning if they don't 
support this functionality.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to