From 5e35d1cd3b7e3146b551603d5937ba22654588ec Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 16 Oct 2015 13:25:29 +1100 Subject: fixed onerror tests --- .../java/org/eclipse/jetty/server/HttpChannel.java | 7 ++- .../org/eclipse/jetty/server/HttpChannelState.java | 66 +++++++++++++++++---- .../java/org/eclipse/jetty/server/HttpOutput.java | 15 +++++ .../eclipse/jetty/server/handler/ErrorHandler.java | 67 ++++++++++------------ .../eclipse/jetty/servlet/AsyncServletTest.java | 67 ++++++++++++---------- .../src/test/resources/jetty-logging.properties | 3 +- .../jetty/continuation/ContinuationsTest.java | 2 +- 7 files changed, 145 insertions(+), 82 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 258788dcd9..8f423e463f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -384,8 +384,11 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor } } catch (Throwable failure) - { - handleException(failure); + { + if ("org.eclipse.jetty.continuation.ContinuationThrowable".equals(failure.getClass().getName())) + LOG.ignore(failure); + else + handleException(failure); } finally { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index f3a0854a01..2c16602ec4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -165,12 +165,18 @@ public class HttpChannelState { try(Locker.Lock lock= _locker.lock()) { - return String.format("%s@%x{s=%s a=%s i=%b r=%s w=%b}",getClass().getSimpleName(),hashCode(),_state,_async,_initial, - _asyncReadPossible?(_asyncReadUnready?"PU":"P!U"):(_asyncReadUnready?"!PU":"!P!U"), - _asyncWrite); + return toStringLocked(); } } + public String toStringLocked() + { + return String.format("%s@%x{s=%s a=%s i=%b r=%s w=%b}",getClass().getSimpleName(),hashCode(),_state,_async,_initial, + _asyncReadPossible?(_asyncReadUnready?"PU":"P!U"):(_asyncReadUnready?"!PU":"!P!U"), + _asyncWrite); + } + + private String getStatusStringLocked() { return String.format("s=%s i=%b a=%s",_state,_initial,_async); @@ -189,10 +195,11 @@ public class HttpChannelState */ protected Action handling() { - if(DEBUG) - LOG.debug("{} handling {}",this,_state); try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("handling {}",toStringLocked()); + switch(_state) { case IDLE: @@ -267,6 +274,9 @@ public class HttpChannelState try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("startAsync {}",toStringLocked()); + if (_state!=State.DISPATCHED || _async!=null) throw new IllegalStateException(this.getStatusStringLocked()); @@ -321,11 +331,11 @@ public class HttpChannelState AsyncContextEvent schedule_event=null; boolean read_interested=false; - if(DEBUG) - LOG.debug("{} unhandle {}",this,_state); - try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("unhandle {}",toStringLocked()); + switch(_state) { case COMPLETING: @@ -427,6 +437,9 @@ public class HttpChannelState AsyncContextEvent event; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("dispatch {} -> {}",toStringLocked(),path); + boolean started=false; event=_event; switch(_async) @@ -478,6 +491,9 @@ public class HttpChannelState AsyncContextEvent event; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onTimeout {}",toStringLocked()); + if (_async!=Async.STARTED) return; _async=Async.EXPIRING; @@ -486,9 +502,6 @@ public class HttpChannelState } - if (LOG.isDebugEnabled()) - LOG.debug("Async timeout {}",this); - final AtomicReference error=new AtomicReference(); if (listeners!=null) { @@ -570,11 +583,15 @@ public class HttpChannelState public void complete() { + // just like resume, except don't set _dispatched=true; boolean handle=false; AsyncContextEvent event; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("complete {}",toStringLocked()); + boolean started=false; event=_event; @@ -610,6 +627,9 @@ public class HttpChannelState { try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("error complete {}",toStringLocked()); + _async=Async.COMPLETE; _event.setDispatchContext(null); _event.setDispatchPath(null); @@ -642,6 +662,9 @@ public class HttpChannelState try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onError {} {}",toStringLocked(),failure); + // Set error on request. if(_event!=null) { @@ -758,6 +781,9 @@ public class HttpChannelState try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onComplete {}",toStringLocked()); + switch(_state) { case COMPLETING: @@ -811,6 +837,9 @@ public class HttpChannelState cancelTimeout(); try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("recycle {}",toStringLocked()); + switch(_state) { case DISPATCHED: @@ -837,6 +866,9 @@ public class HttpChannelState cancelTimeout(); try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("upgrade {}",toStringLocked()); + switch(_state) { case IDLE: @@ -1035,6 +1067,9 @@ public class HttpChannelState boolean interested=false; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onReadUnready {}",toStringLocked()); + // We were already unready, this is not a state change, so do nothing if (!_asyncReadUnready) { @@ -1061,6 +1096,9 @@ public class HttpChannelState boolean woken=false; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onReadPossible {}",toStringLocked()); + _asyncReadPossible=true; if (_state==State.ASYNC_WAIT && _asyncReadUnready) { @@ -1083,6 +1121,9 @@ public class HttpChannelState boolean woken=false; try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onReadReady {}",toStringLocked()); + _asyncReadUnready=true; _asyncReadPossible=true; if (_state==State.ASYNC_WAIT) @@ -1108,6 +1149,9 @@ public class HttpChannelState try(Locker.Lock lock= _locker.lock()) { + if(DEBUG) + LOG.debug("onWritePossible {}",toStringLocked()); + _asyncWrite=true; if (_state==State.ASYNC_WAIT) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index ca35951889..1f437b42aa 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -286,6 +286,20 @@ public class HttpOutput extends ServletOutputStream implements Runnable return _state.get()==OutputState.CLOSED; } + public boolean isAsync() + { + switch(_state.get()) + { + case ASYNC: + case READY: + case PENDING: + case UNREADY: + return true; + default: + return false; + } + } + @Override public void flush() throws IOException { @@ -1252,4 +1266,5 @@ public class HttpOutput extends ServletOutputStream implements Runnable super.onCompleteFailure(x); } } + } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 530d97bb87..97c29dac20 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.server.AsyncContextEvent; import org.eclipse.jetty.server.Dispatcher; +import org.eclipse.jetty.server.HttpOutput; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; @@ -56,7 +57,6 @@ import org.eclipse.jetty.util.log.Logger; */ public class ErrorHandler extends AbstractHandler { - public static final String ERROR_PAGE = "org.eclipse.jetty.server.error_page"; private static final Logger LOG = Log.getLogger(ErrorHandler.class); private boolean _showStacks = true; @@ -86,53 +86,44 @@ public class ErrorHandler extends AbstractHandler if (error_page != null && context != null) { - String old_error_page = (String)request.getAttribute(ERROR_PAGE); - if (old_error_page == null || !old_error_page.equals(error_page)) + Dispatcher dispatcher = (Dispatcher)context.getRequestDispatcher(error_page); + if (dispatcher != null) { - request.setAttribute(ERROR_PAGE, error_page); - Dispatcher dispatcher = (Dispatcher)context.getRequestDispatcher(error_page); - if (dispatcher != null) + try { - try - { - dispatcher.error(request, response); - complete(request); - return; - } - catch (ServletException x) - { - throw new IOException(x); - } + dispatcher.error(request, response); + return; } - else + catch (ServletException x) { - LOG.warn("Could not dispatch to error page: {}", error_page); - // Fall through to provide the default error page. + throw new IOException(x); } } + else + { + LOG.warn("Could not dispatch to error page: {}", error_page); + // Fall through to provide the default error page. + } } } baseRequest.setHandled(true); - response.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.asString()); - String cacheHeader = getCacheControl(); - if (cacheHeader != null) - response.setHeader(HttpHeader.CACHE_CONTROL.asString(), cacheHeader); - ByteArrayISO8859Writer writer = new ByteArrayISO8859Writer(4096); - String reason = (response instanceof Response) ? ((Response)response).getReason() : null; - handleErrorPage(request, writer, response.getStatus(), reason); - writer.flush(); - response.setContentLength(writer.size()); - writer.writeTo(response.getOutputStream()); - writer.destroy(); - - complete(request); - } - - private void complete(HttpServletRequest request) - { - if (request.isAsyncStarted()) - request.getAsyncContext().complete(); + + HttpOutput out = baseRequest.getResponse().getHttpOutput(); + if (!out.isAsync()) + { + response.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.asString()); + String cacheHeader = getCacheControl(); + if (cacheHeader != null) + response.setHeader(HttpHeader.CACHE_CONTROL.asString(), cacheHeader); + ByteArrayISO8859Writer writer = new ByteArrayISO8859Writer(4096); + String reason = (response instanceof Response) ? ((Response)response).getReason() : null; + handleErrorPage(request, writer, response.getStatus(), reason); + writer.flush(); + response.setContentLength(writer.size()); + writer.writeTo(response.getOutputStream()); + writer.destroy(); + } } /* ------------------------------------------------------------ */ diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java index 8dd4dea302..e07acc9d91 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletTest.java @@ -78,6 +78,7 @@ public class AsyncServletTest protected Server _server = new Server(); protected ServletHandler _servletHandler; + protected ErrorPageErrorHandler _errorHandler; protected ServerConnector _connector; protected List _log; protected int _expectedLogs; @@ -85,6 +86,12 @@ public class AsyncServletTest protected static List __history=new CopyOnWriteArrayList<>(); protected static CountDownLatch __latch; + static void historyAdd(String item) + { + // System.err.println(Thread.currentThread()+" history: "+item); + __history.add(item); + } + @Before public void setUp() throws Exception { @@ -104,9 +111,9 @@ public class AsyncServletTest logHandler.setHandler(context); context.addEventListener(new DebugListener()); - ErrorPageErrorHandler errorHandler = new ErrorPageErrorHandler(); - context.setErrorHandler(errorHandler); - errorHandler.addErrorPage(300,599,"/error/custom"); + _errorHandler = new ErrorPageErrorHandler(); + context.setErrorHandler(_errorHandler); + _errorHandler.addErrorPage(300,599,"/error/custom"); _servletHandler=context.getServletHandler(); @@ -466,21 +473,23 @@ public class AsyncServletTest public void testStartTimeoutStart() throws Exception { _expectedCode="500 "; + _errorHandler.addErrorPage(500,"/path/error"); + String response=process("start=10&start2=10",null); assertThat(__history,contains( "REQUEST /ctx/path/info", "initial", "start", "onTimeout", - "ERROR /ctx/error/custom", + "ERROR /ctx/path/error", "!initial", "onStartAsync", "start", "onTimeout", - "ERROR /ctx/error/custom", + "ERROR /ctx/path/error", "!initial", "onComplete")); - assertContains("ERROR DISPATCH: /ctx/error/custom",response); + assertContains("ERROR DISPATCH: /ctx/path/error",response); } @Test @@ -679,9 +688,9 @@ public class AsyncServletTest @Override public void doGet(final HttpServletRequest request, final HttpServletResponse response) throws ServletException, IOException { - __history.add("FWD "+request.getDispatcherType()+" "+request.getRequestURI()); + historyAdd("FWD "+request.getDispatcherType()+" "+request.getRequestURI()); if (request instanceof ServletRequestWrapper || response instanceof ServletResponseWrapper) - __history.add("wrapped"+((request instanceof ServletRequestWrapper)?" REQ":"")+((response instanceof ServletResponseWrapper)?" RSP":"")); + historyAdd("wrapped"+((request instanceof ServletRequestWrapper)?" REQ":"")+((response instanceof ServletResponseWrapper)?" RSP":"")); request.getServletContext().getRequestDispatcher("/path1").forward(request,response); } } @@ -706,9 +715,9 @@ public class AsyncServletTest } // System.err.println(request.getDispatcherType()+" "+request.getRequestURI()); - __history.add(request.getDispatcherType()+" "+request.getRequestURI()); + historyAdd(request.getDispatcherType()+" "+request.getRequestURI()); if (request instanceof ServletRequestWrapper || response instanceof ServletResponseWrapper) - __history.add("wrapped"+((request instanceof ServletRequestWrapper)?" REQ":"")+((response instanceof ServletResponseWrapper)?" RSP":"")); + historyAdd("wrapped"+((request instanceof ServletRequestWrapper)?" REQ":"")+((response instanceof ServletResponseWrapper)?" RSP":"")); boolean wrap="true".equals(request.getParameter("wrap")); int read_before=0; @@ -742,7 +751,7 @@ public class AsyncServletTest if (request.getAttribute("State")==null) { request.setAttribute("State",new Integer(1)); - __history.add("initial"); + historyAdd("initial"); if (read_before>0) { byte[] buf=new byte[read_before]; @@ -770,7 +779,7 @@ public class AsyncServletTest while(b!=-1) if((b=in.read())>=0) c++; - __history.add("async-read="+c); + historyAdd("async-read="+c); } catch(Exception e) { @@ -786,7 +795,7 @@ public class AsyncServletTest if (start_for>0) async.setTimeout(start_for); async.addListener(__listener); - __history.add("start"); + historyAdd("start"); if ("1".equals(request.getParameter("throw"))) throw new QuietServletException(new Exception("test throw in async 1")); @@ -802,7 +811,7 @@ public class AsyncServletTest { response.setStatus(200); response.getOutputStream().println("COMPLETED\n"); - __history.add("complete"); + historyAdd("complete"); async.complete(); } catch(Exception e) @@ -820,7 +829,7 @@ public class AsyncServletTest { response.setStatus(200); response.getOutputStream().println("COMPLETED\n"); - __history.add("complete"); + historyAdd("complete"); async.complete(); } else if (dispatch_after>0) @@ -830,7 +839,7 @@ public class AsyncServletTest @Override public void run() { - __history.add("dispatch"); + historyAdd("dispatch"); if (path!=null) { int q=path.indexOf('?'); @@ -850,7 +859,7 @@ public class AsyncServletTest } else if (dispatch_after==0) { - __history.add("dispatch"); + historyAdd("dispatch"); if (path!=null) async.dispatch(path); else @@ -879,7 +888,7 @@ public class AsyncServletTest } else { - __history.add("!initial"); + historyAdd("!initial"); if (start2_for>=0 && request.getAttribute("2nd")==null) { @@ -891,7 +900,7 @@ public class AsyncServletTest { async.setTimeout(start2_for); } - __history.add("start"); + historyAdd("start"); if ("2".equals(request.getParameter("throw"))) throw new QuietServletException(new Exception("test throw in async 2")); @@ -907,7 +916,7 @@ public class AsyncServletTest { response.setStatus(200); response.getOutputStream().println("COMPLETED\n"); - __history.add("complete"); + historyAdd("complete"); async.complete(); } catch(Exception e) @@ -925,7 +934,7 @@ public class AsyncServletTest { response.setStatus(200); response.getOutputStream().println("COMPLETED\n"); - __history.add("complete"); + historyAdd("complete"); async.complete(); } else if (dispatch2_after>0) @@ -935,7 +944,7 @@ public class AsyncServletTest @Override public void run() { - __history.add("dispatch"); + historyAdd("dispatch"); async.dispatch(); } }; @@ -946,7 +955,7 @@ public class AsyncServletTest } else if (dispatch2_after==0) { - __history.add("dispatch"); + historyAdd("dispatch"); async.dispatch(); } } @@ -969,11 +978,11 @@ public class AsyncServletTest @Override public void onTimeout(AsyncEvent event) throws IOException { - __history.add("onTimeout"); + historyAdd("onTimeout"); String action=event.getSuppliedRequest().getParameter("timeout"); if (action!=null) { - __history.add(action); + historyAdd(action); switch(action) { @@ -995,17 +1004,17 @@ public class AsyncServletTest @Override public void onStartAsync(AsyncEvent event) throws IOException { - __history.add("onStartAsync"); + historyAdd("onStartAsync"); } @Override public void onError(AsyncEvent event) throws IOException { - __history.add("onError"); + historyAdd("onError"); String action=event.getSuppliedRequest().getParameter("error"); if (action!=null) { - __history.add(action); + historyAdd(action); switch(action) { @@ -1024,7 +1033,7 @@ public class AsyncServletTest @Override public void onComplete(AsyncEvent event) throws IOException { - __history.add("onComplete"); + historyAdd("onComplete"); __latch.countDown(); } }; diff --git a/jetty-servlet/src/test/resources/jetty-logging.properties b/jetty-servlet/src/test/resources/jetty-logging.properties index ed4316ee83..37f092141f 100644 --- a/jetty-servlet/src/test/resources/jetty-logging.properties +++ b/jetty-servlet/src/test/resources/jetty-logging.properties @@ -4,4 +4,5 @@ org.eclipse.jetty.LEVEL=INFO #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.servlet.LEVEL=DEBUG #org.eclipse.jetty.io.ChannelEndPoint.LEVEL=DEBUG -#org.eclipse.jetty.server.DebugListener.LEVEL=DEBUG \ No newline at end of file +#org.eclipse.jetty.server.DebugListener.LEVEL=DEBUG +#org.eclipse.jetty.server.HttpChannelState.LEVEL=DEBUG \ No newline at end of file diff --git a/tests/test-continuation/src/test/java/org/eclipse/jetty/continuation/ContinuationsTest.java b/tests/test-continuation/src/test/java/org/eclipse/jetty/continuation/ContinuationsTest.java index 77070ad4cc..f4591eb223 100644 --- a/tests/test-continuation/src/test/java/org/eclipse/jetty/continuation/ContinuationsTest.java +++ b/tests/test-continuation/src/test/java/org/eclipse/jetty/continuation/ContinuationsTest.java @@ -62,7 +62,7 @@ public class ContinuationsTest @Override public boolean add(String e) { - System.err.printf("add(%s)%n",e); + // System.err.printf("add(%s)%n",e); return super.add(e); } }; -- cgit v1.2.3