479903 - improve async onError handling

Work in progress towards clean build
This commit is contained in:
Greg Wilkins 2016-02-17 14:13:21 +01:00
parent 92c339e669
commit dd4a72ce76
9 changed files with 132 additions and 25 deletions

View File

@ -470,7 +470,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
}
else
{
LOG.info(_request.getRequestURI(), failure);
LOG.warn(_request.getRequestURI(), failure);
}
try

View File

@ -518,7 +518,8 @@ public class HttpChannelState
}
catch(Throwable x)
{
LOG.debug("Exception while invoking listener " + listener,x);
LOG.warn(x+" while invoking onTimeout listener " + listener);
LOG.debug(x);
if (error.get()==null)
error.set(x);
else
@ -721,7 +722,8 @@ public class HttpChannelState
}
catch (Throwable x)
{
LOG.info("Exception while invoking listener " + listener,x);
LOG.warn(x+" while invoking onError listener " + listener);
LOG.debug(x);
}
}
}
@ -815,7 +817,8 @@ public class HttpChannelState
}
catch(Exception e)
{
LOG.warn("Exception while invoking listener " + listener,e);
LOG.warn(e+" while invoking onComplete listener " + listener);
LOG.debug(e);
}
}
}

View File

@ -564,12 +564,15 @@ public class Response implements HttpServletResponse
{
ContextHandler.Context context = request.getContext();
ContextHandler contextHandler = context == null ? _channel.getState().getContextHandler() : context.getContextHandler();
ErrorHandler error_handler = ErrorHandler.getErrorHandler(_channel.getServer(), contextHandler);
request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, code);
request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message);
request.setAttribute(RequestDispatcher.ERROR_REQUEST_URI, request.getRequestURI());
request.setAttribute(RequestDispatcher.ERROR_SERVLET_NAME, request.getServletName());
error_handler.handle(null, request, request, this);
ErrorHandler error_handler = ErrorHandler.getErrorHandler(_channel.getServer(), contextHandler);
if (error_handler!=null)
error_handler.handle(null, request, request, this);
else
_out.close();
}
}

View File

@ -46,6 +46,7 @@ import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.handler.HandlerWrapper;
import org.eclipse.jetty.server.handler.StatisticsHandler;
import org.eclipse.jetty.util.Attributes;
@ -86,6 +87,7 @@ public class Server extends HandlerWrapper implements Attributes
private boolean _stopAtShutdown;
private boolean _dumpAfterStart=false;
private boolean _dumpBeforeStop=false;
private ErrorHandler _errorHandler;
private RequestLog _requestLog;
private final Locker _dateLocker = new Locker();
@ -142,6 +144,12 @@ public class Server extends HandlerWrapper implements Attributes
return _requestLog;
}
/* ------------------------------------------------------------ */
public ErrorHandler getErrorHandler()
{
return _errorHandler;
}
/* ------------------------------------------------------------ */
public void setRequestLog(RequestLog requestLog)
{
@ -149,6 +157,17 @@ public class Server extends HandlerWrapper implements Attributes
_requestLog = requestLog;
}
/* ------------------------------------------------------------ */
public void setErrorHandler(ErrorHandler errorHandler)
{
if (errorHandler instanceof ErrorHandler.ErrorPageMapper)
throw new IllegalArgumentException("ErrorPageMapper is applicable only to ContextHandler");
updateBean(_errorHandler,errorHandler);
_errorHandler=errorHandler;
if (errorHandler!=null)
errorHandler.setServer(this);
}
/* ------------------------------------------------------------ */
@ManagedAttribute("version of this server")
public static String getVersion()
@ -162,7 +181,6 @@ public class Server extends HandlerWrapper implements Attributes
return _stopAtShutdown;
}
/* ------------------------------------------------------------ */
/**
* Set a graceful stop time.
@ -330,6 +348,14 @@ public class Server extends HandlerWrapper implements Attributes
@Override
protected void doStart() throws Exception
{
// Create an error handler if there is none
if (_errorHandler==null)
_errorHandler=getBean(ErrorHandler.class);
if (_errorHandler==null)
setErrorHandler(new ErrorHandler());
if (_errorHandler instanceof ErrorHandler.ErrorPageMapper)
LOG.warn("ErrorPageMapper not supported for Server level Error Handling");
//If the Server should be stopped when the jvm exits, register
//with the shutdown handler thread.
if (getStopAtShutdown())

View File

@ -48,7 +48,7 @@ import org.eclipse.jetty.util.log.Logger;
/* ------------------------------------------------------------ */
/** Handler for Error pages
* An ErrorHandler is registered with {@link ContextHandler#setErrorHandler(ErrorHandler)} or
* {@link org.eclipse.jetty.server.Server#addBean(Object)}.
* {@link Server#setErrorHandler(ErrorHandler).
* It is called by the HttpResponse.sendError method to write a error page via {@link #handle(String, Request, HttpServletRequest, HttpServletResponse)}
* or via {@link #badMessageError(int, String, HttpFields)} for bad requests for which a dispatch cannot be done.
*
@ -62,6 +62,11 @@ public class ErrorHandler extends AbstractHandler
boolean _showMessageInTitle=true;
String _cacheControl="must-revalidate,no-cache,no-store";
/* ------------------------------------------------------------ */
public ErrorHandler()
{
}
/* ------------------------------------------------------------ */
/*
* @see org.eclipse.jetty.server.server.Handler#handle(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, int)
@ -79,22 +84,35 @@ public class ErrorHandler extends AbstractHandler
if (this instanceof ErrorPageMapper)
{
String error_page=((ErrorPageMapper)this).getErrorPage(request);
if (error_page!=null && request.getServletContext()!=null)
if (error_page!=null)
{
String old_error_page=(String)request.getAttribute(ERROR_PAGE);
if (old_error_page==null || !old_error_page.equals(error_page))
ServletContext servlet_context = request.getServletContext();
if (servlet_context==null)
servlet_context=ContextHandler.getCurrentContext();
if (servlet_context==null)
{
LOG.warn("No ServletContext for error page {}",error_page);
}
else if (old_error_page!=null && old_error_page.equals(error_page))
{
LOG.warn("Error page loop {}",error_page);
}
else
{
request.setAttribute(ERROR_PAGE, error_page);
Dispatcher dispatcher = (Dispatcher) request.getServletContext().getRequestDispatcher(error_page);
Dispatcher dispatcher = (Dispatcher) servlet_context.getRequestDispatcher(error_page);
try
{
if (LOG.isDebugEnabled())
LOG.debug("error page dispatch {}->{}",error_page,dispatcher);
if(dispatcher!=null)
{
dispatcher.error(request, response);
return;
}
LOG.warn("No error page "+error_page);
LOG.warn("No error page found "+error_page);
}
catch (ServletException e)
{
@ -102,7 +120,9 @@ public class ErrorHandler extends AbstractHandler
return;
}
}
} else {
}
else
{
if (LOG.isDebugEnabled())
{
LOG.debug("No Error Page mapping for request({} {}) (using default)",request.getMethod(),request.getRequestURI());
@ -110,12 +130,15 @@ public class ErrorHandler extends AbstractHandler
}
}
baseRequest.setHandled(true);
response.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.asString());
if (_cacheControl!=null)
response.setHeader(HttpHeader.CACHE_CONTROL.asString(), _cacheControl);
ByteArrayISO8859Writer writer= new ByteArrayISO8859Writer(4096);
String reason=(response instanceof Response)?((Response)response).getReason():null;
if (LOG.isDebugEnabled())
LOG.debug("default error page {} {}",request,response);
handleErrorPage(request, writer, response.getStatus(), reason);
writer.flush();
response.setContentLength(writer.size());

View File

@ -55,6 +55,7 @@ import javax.servlet.http.Part;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.MultiPartInputStreamParser;
import org.eclipse.jetty.util.Utf8Appendable;

View File

@ -53,6 +53,7 @@ import org.eclipse.jetty.io.ByteArrayEndPoint;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.session.HashSessionIdManager;
import org.eclipse.jetty.server.session.HashSessionManager;
import org.eclipse.jetty.server.session.Session;
@ -438,6 +439,37 @@ public class ResponseTest
assertEquals("Super Nanny", response.getReason());
assertEquals("must-revalidate,no-cache,no-store", response.getHeader(HttpHeader.CACHE_CONTROL.asString()));
}
@Test
public void testStatusCodesNoErrorHandler() throws Exception
{
_server.removeBean(_server.getBean(ErrorHandler.class));
Response response = newResponse();
response.sendError(404);
assertEquals(404, response.getStatus());
assertEquals("Not Found", response.getReason());
response = newResponse();
response.sendError(500, "Database Error");
assertEquals(500, response.getStatus());
assertEquals("Database Error", response.getReason());
assertThat(response.getHeader(HttpHeader.CACHE_CONTROL.asString()),Matchers.nullValue());
response = newResponse();
response.setStatus(200);
assertEquals(200, response.getStatus());
assertEquals(null, response.getReason());
response = newResponse();
response.sendError(406, "Super Nanny");
assertEquals(406, response.getStatus());
assertEquals("Super Nanny", response.getReason());
assertThat(response.getHeader(HttpHeader.CACHE_CONTROL.asString()),Matchers.nullValue());
}
@Test
public void testWriteRuntimeIOException() throws Exception

View File

@ -19,6 +19,7 @@
package org.eclipse.jetty.servlet;
import java.io.IOException;
import java.io.Writer;
import java.util.concurrent.TimeUnit;
import javax.servlet.AsyncContext;
@ -31,8 +32,11 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.QuietServletException;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.junit.After;
import org.junit.Test;
@ -82,6 +86,8 @@ public class AsyncListenerTest
response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500);
ServletOutputStream output = response.getOutputStream();
output.println(event.getThrowable().getClass().getName());
if (event.getThrowable().getCause()!=null)
output.println(event.getThrowable().getCause().getClass().getName());
output.println("COMPLETE");
event.getAsyncContext().complete();
});
@ -151,10 +157,17 @@ public class AsyncListenerTest
});
// Add a custom error page.
ErrorPageErrorHandler errorHandler = new ErrorPageErrorHandler();
errorHandler.setServer(server);
errorHandler.addErrorPage(HttpStatus.BAD_GATEWAY_502, "/error");
server.addManaged(errorHandler);
ErrorHandler errorHandler = new ErrorHandler()
{
@Override
protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException
{
writer.write("CUSTOM\n");
super.writeErrorPageMessage(request,writer,code,message,uri);
}
};
server.setErrorHandler(errorHandler);
String httpResponse = connector.getResponses("" +
"GET /ctx/path HTTP/1.1\r\n" +
@ -184,7 +197,7 @@ public class AsyncListenerTest
consumer.accept(event);
}
});
throw new TestRuntimeException();
throw new QuietServletException(new TestRuntimeException());
}
}), "/path/*");
context.addServlet(new ServletHolder(new HttpServlet()
@ -297,10 +310,18 @@ public class AsyncListenerTest
});
// Add a custom error page.
ErrorPageErrorHandler errorHandler = new ErrorPageErrorHandler();
ErrorHandler errorHandler = new ErrorHandler()
{
@Override
protected void writeErrorPageMessage(HttpServletRequest request, Writer writer, int code, String message, String uri) throws IOException
{
writer.write("CUSTOM\n");
super.writeErrorPageMessage(request,writer,code,message,uri);
}
};
errorHandler.setServer(server);
errorHandler.addErrorPage(HttpStatus.BAD_GATEWAY_502, "/error");
server.addManaged(errorHandler);
server.setErrorHandler(errorHandler);
String httpResponse = connector.getResponses("" +
"GET / HTTP/1.1\r\n" +

View File

@ -486,10 +486,8 @@ public class AsyncServletTest
"onStartAsync",
"start",
"onTimeout",
"ERROR /ctx/path/error",
"!initial",
"onComplete"));
assertContains("ERROR DISPATCH: /ctx/path/error",response);
"onComplete")); // Error Page Loop!
assertContains("HTTP ERROR 500",response);
}
@Test