Skip to content

Commit

Permalink
Add more debug information to HTTP/2 GOAWAY frame
Browse files Browse the repository at this point in the history
Motivation:

An HTTP/2 GOAWAY frame with non-zero error code sometimes gives an empty
debugData, making it hard to determine the root cause of the error.

Modifications:

- Add the following information to debugData:
  - the exact type of Http2Exception
  - the original message of Http2Exception
  - the exact type, message and stack trace of the cause of Http2Exception
- Add Exceptions.traceText() to stringify the stack trace of an
  exception
- Fix incorrect Logger level check

Result:

We will have more information when we get an error GOAWAY frame.
  • Loading branch information
trustin committed Apr 19, 2016
1 parent 389b35b commit f0bca3d
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@

package com.linecorp.armeria.common.http;

import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;

import com.linecorp.armeria.common.util.Exceptions;

import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.handler.codec.http2.Http2ConnectionDecoder;
import io.netty.handler.codec.http2.Http2ConnectionEncoder;
import io.netty.handler.codec.http2.Http2ConnectionHandler;
import io.netty.handler.codec.http2.Http2Exception;
import io.netty.handler.codec.http2.Http2Exception.ClosedStreamCreationException;
import io.netty.handler.codec.http2.Http2Settings;
import io.netty.handler.codec.http2.Http2Stream.State;
import io.netty.handler.codec.http2.Http2StreamVisitor;
Expand All @@ -43,8 +48,10 @@ public abstract class AbstractHttpToHttp2ConnectionHandler extends HttpToHttp2Co
private boolean closing;
private boolean handlingConnectionError;

protected AbstractHttpToHttp2ConnectionHandler(Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder,
Http2Settings initialSettings, boolean validateHeaders) {
protected AbstractHttpToHttp2ConnectionHandler(
Http2ConnectionDecoder decoder, Http2ConnectionEncoder encoder,
Http2Settings initialSettings, boolean validateHeaders) {

super(decoder, encoder, initialSettings, validateHeaders);
}

Expand All @@ -59,9 +66,45 @@ protected void onConnectionError(ChannelHandlerContext ctx, Throwable cause, Htt
}

handlingConnectionError = true;

// TODO(trustin): Remove this once Http2ConnectionHandler.goAway() sends better debugData.
// See https://github.com/netty/netty/issues/5160
if (http2Ex == null) {
http2Ex = new Http2Exception(INTERNAL_ERROR, goAwayDebugData(null, cause), cause);
} else if (http2Ex instanceof ClosedStreamCreationException) {
final ClosedStreamCreationException e = (ClosedStreamCreationException) http2Ex;
http2Ex = new ClosedStreamCreationException(e.error(), goAwayDebugData(e, cause), cause);
} else {
http2Ex = new Http2Exception(
http2Ex.error(), goAwayDebugData(http2Ex, cause), cause, http2Ex.shutdownHint());
}

super.onConnectionError(ctx, cause, http2Ex);
}

private static String goAwayDebugData(Http2Exception http2Ex, Throwable cause) {
final StringBuilder buf = new StringBuilder(256);
final String type;
final String message;

if (http2Ex != null) {
type = http2Ex.getClass().getName();
message = http2Ex.getMessage();
} else {
type = null;
message = null;
}

buf.append("type: ");
buf.append(type != null ? type : "n/a");
buf.append(", message: ");
buf.append(message != null ? message : "n/a");
buf.append(", cause: ");
buf.append(cause != null ? Exceptions.traceText(cause) : "n/a");

return buf.toString();
}

@Override
public void close(ChannelHandlerContext ctx, ChannelPromise promise) throws Exception {
closing = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ ch, sentOrReceived, lastStreamId, errorStr(errorCode),
ByteBufUtil.hexDump(debugData));
}
} else {
if (logger.isInfoEnabled()) {
if (logger.isDebugEnabled()) {
logger.debug("{} {} a GOAWAY frame: lastStreamId={}, errorCode=NO_ERROR",
ch, sentOrReceived, lastStreamId);
}
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/com/linecorp/armeria/common/util/Exceptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static java.util.Objects.requireNonNull;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.channels.ClosedChannelException;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -118,5 +120,17 @@ public static <T extends Throwable> T clearTrace(T exception) {
return exception;
}

/**
* Returns the stack trace of the specified {@code exception} as a {@link String}.
*/
public static String traceText(Throwable exception) {
requireNonNull(exception, "exception");
final StringWriter out = new StringWriter(256);
final PrintWriter pout = new PrintWriter(out);
exception.printStackTrace(pout);
pout.flush();
return out.toString();
}

private Exceptions() {}
}

0 comments on commit f0bca3d

Please sign in to comment.