Skip to content

Commit

Permalink
log the error when Netty failed to print out response
Browse files Browse the repository at this point in the history 
in our case, it was `IllegalStateException: unexpected message type: DefaultHttpResponse` in case of "304 Not Modified" and "404 Not Found" responses.
  • Loading branch information
@asolntsev
asolntsev committed Jan 15, 2023
1 parent bcbbdb3 commit c0596f2
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 84 deletions.
71 changes: 30 additions & 41 deletions netty3/src/play/server/netty3/PlayHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ public void messageReceived(final ChannelHandlerContext ctx, MessageEvent messag

} catch (IllegalArgumentException ex) {
logger.warn("Exception on request. serving 400 back", ex);
serve400(ex, ctx);
serve400(ex, ctx, nettyRequest);
} catch (Exception ex) {
logger.warn("Exception on request. serving 500 back", ex);
serve500(ex, ctx);
serve500(ex, ctx, nettyRequest);
}
}

Expand Down Expand Up @@ -203,7 +203,7 @@ public boolean init() {
Router.instance.routeOnlyStatic(request);
super.init();
} catch (NotFound nf) {
serve404(nf, ctx, request);
serve404(nf, ctx, request, nettyRequest);
logger.trace("init: end false");
return false;
} catch (RenderStatic rs) {
Expand Down Expand Up @@ -251,7 +251,7 @@ public void run() {
InvocationContext.current.remove();
}
} catch (Exception e) {
serve500(e, ctx);
serve500(e, ctx, nettyRequest);
}
logger.trace("run: end");
}
Expand Down Expand Up @@ -385,18 +385,13 @@ protected static void writeResponse(ChannelHandlerContext ctx, Response response
setContentLength(nettyResponse, response.out.size());
}

ChannelFuture f = null;
if (ctx.getChannel().isOpen()) {
f = ctx.getChannel().write(nettyResponse);
ChannelFuture f = ctx.getChannel().write(nettyResponse);
addListeners(f, nettyRequest, nettyResponse, keepAlive);
} else {
logger.debug("Try to write on a closed channel[keepAlive:{}]: Remote host may have closed the connection", keepAlive);
}

// Decide whether to close the connection or not.
if (f != null && !keepAlive) {
// Close the connection when the whole content is written out.
f.addListener(ChannelFutureListener.CLOSE);
}
logger.trace("writeResponse: end");
}

Expand Down Expand Up @@ -439,12 +434,7 @@ public void copyResponse(ChannelHandlerContext ctx, Request request, Response re

// Write the initial line and the header.
ChannelFuture writeFuture = ch.write(nettyResponse);

if (!keepAlive) {
// Close the connection when the whole content is
// written out.
writeFuture.addListener(ChannelFutureListener.CLOSE);
}
addListeners(writeFuture, nettyRequest, nettyResponse, keepAlive);
} else {
fileService.serve(file, nettyRequest, nettyResponse, ctx, request, response, ctx.getChannel());
}
Expand All @@ -455,19 +445,15 @@ public void copyResponse(ChannelHandlerContext ctx, Request request, Response re
} else {
is.close();
}
if (!keepAlive) {
writeFuture.addListener(ChannelFutureListener.CLOSE);
}
addListeners(writeFuture, nettyRequest, nettyResponse, keepAlive);
} else if (stream != null) {
ChannelFuture writeFuture = ctx.getChannel().write(nettyResponse);
if (!nettyRequest.getMethod().equals(HttpMethod.HEAD) && !nettyResponse.getStatus().equals(HttpResponseStatus.NOT_MODIFIED)) {
writeFuture = ctx.getChannel().write(stream);
} else {
stream.close();
}
if (!keepAlive) {
writeFuture.addListener(ChannelFutureListener.CLOSE);
}
addListeners(writeFuture, nettyRequest, nettyResponse, keepAlive);
} else {
writeResponse(ctx, response, nettyResponse, nettyRequest);
}
Expand Down Expand Up @@ -629,15 +615,15 @@ public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e) {
}
}

private void serve400(Exception e, ChannelHandlerContext ctx) {
private void serve400(Exception e, ChannelHandlerContext ctx, HttpRequest nettyRequest) {
logger.trace("serve400: begin");
HttpResponse nettyResponse = createHttpResponse(HttpResponseStatus.BAD_REQUEST);
nettyResponse.headers().set(CONTENT_TYPE, "text/plain");
printResponse(ctx, nettyResponse, e.getMessage() + '\n');
printResponse(ctx, nettyRequest, nettyResponse, e.getMessage() + '\n');
logger.trace("serve400: end");
}

private void serve404(NotFound e, ChannelHandlerContext ctx, Request request) {
private void serve404(NotFound e, ChannelHandlerContext ctx, Request request, HttpRequest nettyRequest) {
logger.trace("serve404: begin");
String format = defaultString(request.format, "txt");
String contentType = MimeTypes.getContentType("404." + format, "text/plain");
Expand All @@ -646,7 +632,7 @@ private void serve404(NotFound e, ChannelHandlerContext ctx, Request request) {
nettyResponse.headers().set(CONTENT_TYPE, contentType);

String errorHtml = TemplateLoader.load("errors/404." + format).render(getBindingForErrors(request, e, false));
printResponse(ctx, nettyResponse, errorHtml);
printResponse(ctx, nettyRequest, nettyResponse, errorHtml);
logger.trace("serve404: end");
}

Expand All @@ -668,17 +654,17 @@ private static Map<String, Object> getBindingForErrors(Request request, Exceptio
return binding;
}

private static void printResponse(ChannelHandlerContext ctx, HttpResponse nettyResponse, String errorHtml) {
private static void printResponse(ChannelHandlerContext ctx, HttpRequest nettyRequest, HttpResponse nettyResponse, String errorHtml) {
byte[] bytes = errorHtml.getBytes(Response.current().encoding);
ChannelBuffer buf = ChannelBuffers.copiedBuffer(bytes);
setContentLength(nettyResponse, bytes.length);
nettyResponse.setContent(buf);
ChannelFuture writeFuture = ctx.getChannel().write(nettyResponse);
writeFuture.addListener(ChannelFutureListener.CLOSE);
addListeners(writeFuture, nettyRequest, nettyResponse, false);
}

// TO DO: add request and response as parameter
public void serve500(Exception e, ChannelHandlerContext ctx) {
public void serve500(Exception e, ChannelHandlerContext ctx, HttpRequest nettyRequest) {
logger.trace("serve500: begin");

HttpResponse nettyResponse = createHttpResponse(HttpResponseStatus.INTERNAL_SERVER_ERROR);
Expand Down Expand Up @@ -731,7 +717,7 @@ public void serve500(Exception e, ChannelHandlerContext ctx) {
setContentLength(nettyResponse, bytes.length);
nettyResponse.setContent(buf);
ChannelFuture writeFuture = ctx.getChannel().write(nettyResponse);
writeFuture.addListener(ChannelFutureListener.CLOSE);
addListeners(writeFuture, nettyRequest, nettyResponse, false);
logger.error("Internal Server Error (500) for request {} {}", request.method, request.url, e);
} catch (Throwable ex) {
logger.error("Internal Server Error (500) for request {} {}", request.method, request.url, e);
Expand All @@ -741,7 +727,7 @@ public void serve500(Exception e, ChannelHandlerContext ctx) {
setContentLength(nettyResponse, bytes.length);
nettyResponse.setContent(buf);
ChannelFuture writeFuture = ctx.getChannel().write(nettyResponse);
writeFuture.addListener(ChannelFutureListener.CLOSE);
addListeners(writeFuture, nettyRequest, nettyResponse, false);
}
} catch (Throwable exxx) {
try {
Expand All @@ -750,7 +736,7 @@ public void serve500(Exception e, ChannelHandlerContext ctx) {
setContentLength(nettyResponse, bytes.length);
nettyResponse.setContent(buf);
ChannelFuture writeFuture = ctx.getChannel().write(nettyResponse);
writeFuture.addListener(ChannelFutureListener.CLOSE);
addListeners(writeFuture, nettyRequest, nettyResponse, false);
} catch (Exception fex) {
logger.error("(encoding ?)", fex);
}
Expand Down Expand Up @@ -780,21 +766,16 @@ public void serveStatic(RenderStatic renderStatic, ChannelHandlerContext ctx, Re
}
}
if ((file == null || !file.exists())) {
serve404(new NotFound("The file " + renderStatic.file + " does not exist"), ctx, request);
serve404(new NotFound("The file " + renderStatic.file + " does not exist"), ctx, request, nettyRequest);
} else {
File localFile = file.getRealFile();
boolean keepAlive = isKeepAlive(nettyRequest);
nettyResponse = addEtag(nettyRequest, nettyResponse, localFile);

if (nettyResponse.getStatus().equals(HttpResponseStatus.NOT_MODIFIED)) {
Channel ch = e.getChannel();

// Write the initial line and the header.
ChannelFuture writeFuture = ch.write(nettyResponse);
if (!keepAlive) {
// Write the content.
writeFuture.addListener(ChannelFutureListener.CLOSE);
}
addListeners(writeFuture, nettyRequest, nettyResponse, keepAlive);
} else {
fileService.serve(localFile, nettyRequest, nettyResponse, ctx, request, response, e.getChannel());
}
Expand All @@ -809,7 +790,7 @@ public void serveStatic(RenderStatic renderStatic, ChannelHandlerContext ctx, Re
setContentLength(nettyResponse, bytes.length);
errorResponse.setContent(buf);
ChannelFuture future = ctx.getChannel().write(errorResponse);
future.addListener(ChannelFutureListener.CLOSE);
addListeners(future, nettyRequest, nettyResponse, false);
} catch (Exception ex) {
logger.error("serveStatic for request {} {}", request.method, request.url, ex);
}
Expand Down Expand Up @@ -975,4 +956,12 @@ public void closeChunked(Response playResponse) {
throw new UnexpectedException(e);
}
}

private static void addListeners(ChannelFuture future, HttpRequest request, HttpResponse response, boolean keepAlive) {
future.addListener(new ResponseRenderingListener(request, response));
if (!keepAlive) {
// Close the connection when the whole content is written out
future.addListener(ChannelFutureListener.CLOSE);
}
}
}
32 changes: 32 additions & 0 deletions netty3/src/play/server/netty3/ResponseRenderingListener.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package play.server.netty3;

import org.jboss.netty.channel.ChannelFuture;
import org.jboss.netty.channel.ChannelFutureListener;
import org.jboss.netty.handler.codec.http.HttpRequest;
import org.jboss.netty.handler.codec.http.HttpResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

class ResponseRenderingListener implements ChannelFutureListener {
private static final Logger logger = LoggerFactory.getLogger(ResponseRenderingListener.class);
private final HttpRequest request;
private final HttpResponse response;

ResponseRenderingListener(HttpRequest nettyRequest, HttpResponse response) {
request = nettyRequest;
this.response = response;
}

@Override
public void operationComplete(ChannelFuture future) {
if (future.isSuccess()) {
logger.trace("served {} {} (status: {})",
request.getMethod(), request.getUri(), response.getStatus());
}
else {
logger.error("failed to serve {} {} (status: {})",
request.getMethod(), request.getUri(), response.getStatus(), future.getCause());
}
}

}
Loading

0 comments on commit c0596f2

Please sign in to comment.