Skip to content

Commit

Permalink
Improve exception handling when clients disconnect.
Browse files Browse the repository at this point in the history
Issue: SPR-14538
  • Loading branch information
violetagg authored and rstoyanchev committed Sep 16, 2016
1 parent c4b9b92 commit 80ff5ae
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public final void onNext(Publisher<DataBuffer> publisher) {

@Override
public final void onError(Throwable t) {
if (logger.isErrorEnabled()) {
logger.error(this.state + " onError: " + t, t);
if (logger.isTraceEnabled()) {
logger.trace(this.state + " onError: " + t);
}
this.state.get().onError(this, t);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ public final void onNext(DataBuffer dataBuffer) {

@Override
public final void onError(Throwable t) {
if (logger.isErrorEnabled()) {
logger.error(this.state + " onError: " + t, t);
if (logger.isTraceEnabled()) {
logger.trace(this.state + " onError: " + t);
}
this.state.get().onError(this, t);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import reactor.core.publisher.Mono;
import reactor.ipc.netty.http.HttpChannel;

import org.apache.commons.logging.LogFactory;
import org.springframework.core.io.buffer.NettyDataBufferFactory;
import org.springframework.util.Assert;

import io.netty.handler.codec.http.HttpResponseStatus;

/**
* Adapt {@link HttpHandler} to the Reactor Netty channel handling function.
*
Expand All @@ -46,7 +49,12 @@ public Mono<Void> apply(HttpChannel channel) {
NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(channel.delegate().alloc());
ReactorServerHttpRequest adaptedRequest = new ReactorServerHttpRequest(channel, bufferFactory);
ReactorServerHttpResponse adaptedResponse = new ReactorServerHttpResponse(channel, bufferFactory);
return this.httpHandler.handle(adaptedRequest, adaptedResponse);
return this.httpHandler.handle(adaptedRequest, adaptedResponse)
.otherwise(ex -> {
LogFactory.getLog(ReactorHttpHandlerAdapter.class).error("Could not complete request", ex);
channel.status(HttpResponseStatus.INTERNAL_SERVER_ERROR);
return Mono.empty();
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
package org.springframework.http.server.reactive;

import io.netty.buffer.ByteBuf;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.reactivex.netty.protocol.http.server.HttpServerRequest;
import io.reactivex.netty.protocol.http.server.HttpServerResponse;
import io.reactivex.netty.protocol.http.server.RequestHandler;

import org.apache.commons.logging.LogFactory;
import org.reactivestreams.Publisher;
import reactor.adapter.RxJava1Adapter;
import reactor.core.publisher.Mono;
import rx.Observable;

import org.springframework.core.io.buffer.NettyDataBufferFactory;
Expand Down Expand Up @@ -49,7 +53,12 @@ public Observable<Void> handle(HttpServerRequest<ByteBuf> request, HttpServerRes
NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(response.unsafeNettyChannel().alloc());
RxNettyServerHttpRequest adaptedRequest = new RxNettyServerHttpRequest(request, bufferFactory);
RxNettyServerHttpResponse adaptedResponse = new RxNettyServerHttpResponse(response, bufferFactory);
Publisher<Void> result = this.httpHandler.handle(adaptedRequest, adaptedResponse);
Publisher<Void> result = this.httpHandler.handle(adaptedRequest, adaptedResponse)
.otherwise(ex -> {
LogFactory.getLog(RxNettyHttpHandlerAdapter.class).error("Could not complete request", ex);
response.setStatus(HttpResponseStatus.INTERNAL_SERVER_ERROR);
return Mono.empty();
});
return RxJava1Adapter.publisherToObservable(result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import reactor.core.publisher.Mono;

import org.springframework.core.NestedCheckedException;
import org.springframework.http.HttpStatus;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebExceptionHandler;
Expand All @@ -37,6 +40,36 @@
* @since 5.0
*/
public class ExceptionHandlingWebHandler extends WebHandlerDecorator {
/**
* Log category to use on network IO exceptions after a client has gone away.
* <p>The Servlet API does not provide notifications when a client disconnects;
* see <a href="https://java.net/jira/browse/SERVLET_SPEC-44">SERVLET_SPEC-44</a>.
* Therefore network IO failures may occur simply because a client has gone away,
* and that can fill the logs with unnecessary stack traces.
* <p>We make a best effort to identify such network failures, on a per-server
* basis, and log them under a separate log category. A simple one-line message
* is logged at DEBUG level, while a full stack trace is shown at TRACE level.
* @see #disconnectedClientLogger
*/
private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
"org.springframework.web.server.handler.DisconnectedClient";

/**
* Separate logger to use on network IO failure after a client has gone away.
* @see #DISCONNECTED_CLIENT_LOG_CATEGORY
*/
private static final Log disconnectedClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY);

private static final Set<String> disconnectedClientExceptions;

static {
Set<String> set = new HashSet<>(3);
set.add("ClientAbortException"); // Tomcat
set.add("EOFException"); // Tomcat
set.add("EofException"); // Jetty
// java.io.IOException "Broken pipe" on WildFly, Glassfish (already covered)
disconnectedClientExceptions = Collections.unmodifiableSet(set);
}

private static Log logger = LogFactory.getLog(ExceptionHandlingWebHandler.class);

Expand Down Expand Up @@ -77,11 +110,32 @@ public Mono<Void> handle(ServerWebExchange exchange) {
}

private Mono<? extends Void> handleUnresolvedException(ServerWebExchange exchange, Throwable ex) {
if (logger.isDebugEnabled()) {
logger.debug("Could not complete request", ex);
}
logError(ex);
exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR);
return Mono.empty();
}

private void logError(Throwable t) {
@SuppressWarnings("serial")
NestedCheckedException nestedException = new NestedCheckedException("", t) {};

if ("Broken pipe".equalsIgnoreCase(nestedException.getMostSpecificCause().getMessage()) ||
disconnectedClientExceptions.contains(t.getClass().getSimpleName())) {

if (disconnectedClientLogger.isTraceEnabled()) {
disconnectedClientLogger.trace("Looks like the client has gone away", t);
}
else if (disconnectedClientLogger.isDebugEnabled()) {
disconnectedClientLogger.debug("Looks like the client has gone away: " +
nestedException.getMessage() + " (For full stack trace, set the '" +
DISCONNECTED_CLIENT_LOG_CATEGORY + "' log category to TRACE level)");
}
}
else {
if (logger.isDebugEnabled()) {
logger.debug("Could not complete request", t);
}
}
}

}

0 comments on commit 80ff5ae

Please sign in to comment.