zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 07:57:13 +02:00
Code Issues Projects Releases Packages Wiki Activity

Refine logs in the proxy

Browse source 
[1] All logs should contain a reference to the channel so that it is easy to search for logs about a specific channel.

[2] EPP ssl handshake failure should be logged at warning. It is mostly the client that failed to complete the handshake, for example by sending bad cert, or not sending cert, or not using the correct SSL version. We should not lot it at error and spam the log.

[3] When the EPP response is not 200, we should not log at error because it means that the GAE app responded successfully. For example when datastore contention occurs, app engine responds with a non-200 status and logs at warning. The proxy should not at a higher level than app engine itself.

[4] Timeout is a non-fatal error that should be logged at warning.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=207562299
This commit is contained in:
jianglai 2018-08-06 10:02:41 -07:00
parent 4ff77fb370
commit 6810e959f9
3 changed files with 24 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

17
java/google/registry/proxy/handler/HttpsRelayServiceHandler.java
View file

@ -34,6 +34,7 @@ import io.netty.handler.codec.http.HttpVersion;
import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
import io.netty.handler.codec.http.cookie.ClientCookieEncoder;
import io.netty.handler.codec.http.cookie.Cookie;
import io.netty.handler.timeout.ReadTimeoutException;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@ -152,8 +153,9 @@ abstract class HttpsRelayServiceHandler extends ByteToMessageCodec<FullHttpRespo
throws Exception {
checkArgument(
response.status().equals(HttpResponseStatus.OK),
"Cannot relay HTTP response status \"%s\"\n%s",
"Cannot relay HTTP response status \"%s\"in channel %s:\n%s",
response.status(),
ctx.channel(),
response.content().toString(UTF_8));
saveCookies(response);
byteBuf.writeBytes(encodeFullHttpResponse(response));
@ -162,8 +164,17 @@ abstract class HttpsRelayServiceHandler extends ByteToMessageCodec<FullHttpRespo
/** Terminates connection upon inbound exception. */
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
logger.atSevere().withCause(cause).log(
"Inbound exception caught for channel %s", ctx.channel());
// ReadTimeoutException is non fatal as the client times out due to inactivity.
// IllegalArgumentException is thrown by the checkArgument in the #encode command, it just means
// that GAE returns a non-200 response and the connection should be killed. The request is still
// processed by GAE, so this is not an unexpected behavior.
if (cause instanceof ReadTimeoutException || cause instanceof IllegalArgumentException) {
logger.atWarning().withCause(cause).log(
"Inbound exception caught for channel %s", ctx.channel());
} else {
logger.atSevere().withCause(cause).log(
"Inbound exception caught for channel %s", ctx.channel());
}
ChannelFuture unusedFuture = ctx.close();
}