Copilot commented on code in PR #7451:
URL: https://github.com/apache/incubator-seata/pull/7451#discussion_r2157959323
##########
server/src/main/java/org/apache/seata/server/cluster/manager/ClusterWatcherManager.java:
##########
@@ -87,37 +87,38 @@ public void onChangeEvent(ClusterChangeEvent event) {
}
}
- private void notifyWatcher(Watcher<?> watcher) {
+ private void notifyWatcher(Watcher<HttpContext> watcher) {
watcher.setDone(true);
sendWatcherResponse(watcher, HttpResponseStatus.OK);
}
- private void sendWatcherResponse(Watcher<?> watcher, HttpResponseStatus
nettyStatus) {
- Object context = watcher.getAsyncContext();
+ private void sendWatcherResponse(Watcher<HttpContext> watcher,
HttpResponseStatus nettyStatus) {
+ HttpContext context = watcher.getAsyncContext();
if (!(context instanceof HttpContext)) {
logger.warn("Unsupported context type for watcher on group {}:
{}", watcher.getGroup(),
context != null ? context.getClass().getName() : "null");
return;
}
- HttpContext httpContext = (HttpContext)context;
- ChannelHandlerContext ctx = httpContext.getContext();
- if (ctx.channel().isActive()) {
- HttpResponse response =
- new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, nettyStatus,
Unpooled.EMPTY_BUFFER);
- response.headers().set(HttpHeaderNames.CONTENT_LENGTH, 0);
+ ChannelHandlerContext ctx = context.getContext();
+ if (!context.isHttp2()) {
+ if (ctx.channel().isActive()) {
+ HttpResponse response =
Review Comment:
Currently this branch only handles HTTP/1 contexts and skips HTTP/2, so
watchers on HTTP/2 streams never get notified. Consider adding an equivalent
HTTP/2 response or explicitly documenting this limitation.
```suggestion
if (context.isHttp2()) {
if (ctx.channel().isActive()) {
HttpResponse response =
new DefaultFullHttpResponse(HttpVersion.HTTP_2,
nettyStatus, Unpooled.EMPTY_BUFFER);
response.headers().set(HttpHeaderNames.CONTENT_LENGTH, 0);
ctx.writeAndFlush(response);
} else {
logger.warn("Netty channel is not active for HTTP/2 watcher
on group {}, cannot send response.",
watcher.getGroup());
}
} else {
if (ctx.channel().isActive()) {
HttpResponse response =
```
##########
core/src/main/java/org/apache/seata/core/rpc/netty/http/HttpDispatchHandler.java:
##########
@@ -82,13 +56,13 @@ protected void channelRead0(ChannelHandlerContext ctx,
HttpRequest httpRequest)
HttpInvocation httpInvocation =
ControllerManager.getHttpInvocation(path);
if (httpInvocation == null) {
- sendErrorResponse(ctx, HttpResponseStatus.NOT_FOUND,
keepAlive);
+ sendErrorResponse(ctx, HttpResponseStatus.NOT_FOUND, false);
Review Comment:
[nitpick] Forcing `keepAlive=false` diverges from the incoming request's
`keepAlive` setting and may break connection reuse on HTTP/1.1. Either preserve
the original flag or add a comment clarifying intentional closure.
```suggestion
sendErrorResponse(ctx, HttpResponseStatus.NOT_FOUND,
keepAlive);
```
--
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]
