PROTON-881: Graceful handling of runtime exceptions thrown within handlers When a runtime (unchecked) exception is thrown from within a handler, the reactor now catches it and rethrows it as a HandlerException (which is also an unchecked exception). The HandlerException references the handler that threw the original exception and also chains the original exception as its "cause".
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/973b429c Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/973b429c Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/973b429c Branch: refs/heads/proton-j-reactor Commit: 973b429cdc12b6686033894eb7fd6a52114540fe Parents: 5ed6de4 Author: Adrian Preston <[email protected]> Authored: Mon Jun 15 22:49:43 2015 +0100 Committer: Rafael Schloming <[email protected]> Committed: Thu Jun 18 07:13:31 2015 -0400 ---------------------------------------------------------------------- .../org/apache/qpid/proton/engine/Event.java | 2 +- .../qpid/proton/engine/HandlerException.java | 39 +++ .../qpid/proton/engine/impl/EventImpl.java | 253 ++++++++++--------- .../org/apache/qpid/proton/reactor/Reactor.java | 7 +- .../qpid/proton/reactor/impl/ReactorImpl.java | 9 +- .../apache/qpid/proton/reactor/ReactorTest.java | 179 +++++++++++++ 6 files changed, 357 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java ---------------------------------------------------------------------- diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java index 743b6d0..d351eed 100644 --- a/proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java +++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java @@ -87,7 +87,7 @@ public interface Event extends Extendable Object getContext(); - void dispatch(Handler handler); + void dispatch(Handler handler) throws HandlerException; Connection getConnection(); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/main/java/org/apache/qpid/proton/engine/HandlerException.java ---------------------------------------------------------------------- diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/HandlerException.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/HandlerException.java new file mode 100644 index 0000000..ca18d7c --- /dev/null +++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/HandlerException.java @@ -0,0 +1,39 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +package org.apache.qpid.proton.engine; + + +public class HandlerException extends RuntimeException { + + private static final long serialVersionUID = 5300211824119834005L; + + private final Handler handler; + + public HandlerException(Handler handler, Throwable cause) { + super(cause); + this.handler = handler; + } + + public Handler getHandler() { + return handler; + } +} http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java ---------------------------------------------------------------------- diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java index 1210eca..4e97332 100644 --- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java +++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java @@ -26,6 +26,7 @@ import org.apache.qpid.proton.engine.Connection; import org.apache.qpid.proton.engine.Delivery; import org.apache.qpid.proton.engine.Event; import org.apache.qpid.proton.engine.Handler; +import org.apache.qpid.proton.engine.HandlerException; import org.apache.qpid.proton.engine.Link; import org.apache.qpid.proton.engine.Record; import org.apache.qpid.proton.engine.Session; @@ -81,130 +82,134 @@ class EventImpl implements Event @Override public void dispatch(Handler handler) { - switch (type) { - case CONNECTION_INIT: - handler.onConnectionInit(this); - break; - case CONNECTION_LOCAL_OPEN: - handler.onConnectionLocalOpen(this); - break; - case CONNECTION_REMOTE_OPEN: - handler.onConnectionRemoteOpen(this); - break; - case CONNECTION_LOCAL_CLOSE: - handler.onConnectionLocalClose(this); - break; - case CONNECTION_REMOTE_CLOSE: - handler.onConnectionRemoteClose(this); - break; - case CONNECTION_BOUND: - handler.onConnectionBound(this); - break; - case CONNECTION_UNBOUND: - handler.onConnectionUnbound(this); - break; - case CONNECTION_FINAL: - handler.onConnectionFinal(this); - break; - case SESSION_INIT: - handler.onSessionInit(this); - break; - case SESSION_LOCAL_OPEN: - handler.onSessionLocalOpen(this); - break; - case SESSION_REMOTE_OPEN: - handler.onSessionRemoteOpen(this); - break; - case SESSION_LOCAL_CLOSE: - handler.onSessionLocalClose(this); - break; - case SESSION_REMOTE_CLOSE: - handler.onSessionRemoteClose(this); - break; - case SESSION_FINAL: - handler.onSessionFinal(this); - break; - case LINK_INIT: - handler.onLinkInit(this); - break; - case LINK_LOCAL_OPEN: - handler.onLinkLocalOpen(this); - break; - case LINK_REMOTE_OPEN: - handler.onLinkRemoteOpen(this); - break; - case LINK_LOCAL_DETACH: - handler.onLinkLocalDetach(this); - break; - case LINK_REMOTE_DETACH: - handler.onLinkRemoteDetach(this); - break; - case LINK_LOCAL_CLOSE: - handler.onLinkLocalClose(this); - break; - case LINK_REMOTE_CLOSE: - handler.onLinkRemoteClose(this); - break; - case LINK_FLOW: - handler.onLinkFlow(this); - break; - case LINK_FINAL: - handler.onLinkFinal(this); - break; - case DELIVERY: - handler.onDelivery(this); - break; - case TRANSPORT: - handler.onTransport(this); - break; - case TRANSPORT_ERROR: - handler.onTransportError(this); - break; - case TRANSPORT_HEAD_CLOSED: - handler.onTransportHeadClosed(this); - break; - case TRANSPORT_TAIL_CLOSED: - handler.onTransportTailClosed(this); - break; - case TRANSPORT_CLOSED: - handler.onTransportClosed(this); - break; - case REACTOR_FINAL: - handler.onReactorFinal(this); - break; - case REACTOR_QUIESCED: - handler.onReactorQuiesced(this); - break; - case REACTOR_INIT: - handler.onReactorInit(this); - break; - case SELECTABLE_ERROR: - handler.onSelectableError(this); - break; - case SELECTABLE_EXPIRED: - handler.onSelectableExpired(this); - break; - case SELECTABLE_FINAL: - handler.onSelectableFinal(this); - break; - case SELECTABLE_INIT: - handler.onSelectableInit(this); - break; - case SELECTABLE_READABLE: - handler.onSelectableReadable(this); - break; - case SELECTABLE_UPDATED: - handler.onSelectableWritable(this); - break; - case SELECTABLE_WRITABLE: - handler.onSelectableWritable(this); - break; - case TIMER_TASK: - handler.onTimerTask(this); - break; - default: - handler.onUnhandled(this); - break; + try { + switch (type) { + case CONNECTION_INIT: + handler.onConnectionInit(this); + break; + case CONNECTION_LOCAL_OPEN: + handler.onConnectionLocalOpen(this); + break; + case CONNECTION_REMOTE_OPEN: + handler.onConnectionRemoteOpen(this); + break; + case CONNECTION_LOCAL_CLOSE: + handler.onConnectionLocalClose(this); + break; + case CONNECTION_REMOTE_CLOSE: + handler.onConnectionRemoteClose(this); + break; + case CONNECTION_BOUND: + handler.onConnectionBound(this); + break; + case CONNECTION_UNBOUND: + handler.onConnectionUnbound(this); + break; + case CONNECTION_FINAL: + handler.onConnectionFinal(this); + break; + case SESSION_INIT: + handler.onSessionInit(this); + break; + case SESSION_LOCAL_OPEN: + handler.onSessionLocalOpen(this); + break; + case SESSION_REMOTE_OPEN: + handler.onSessionRemoteOpen(this); + break; + case SESSION_LOCAL_CLOSE: + handler.onSessionLocalClose(this); + break; + case SESSION_REMOTE_CLOSE: + handler.onSessionRemoteClose(this); + break; + case SESSION_FINAL: + handler.onSessionFinal(this); + break; + case LINK_INIT: + handler.onLinkInit(this); + break; + case LINK_LOCAL_OPEN: + handler.onLinkLocalOpen(this); + break; + case LINK_REMOTE_OPEN: + handler.onLinkRemoteOpen(this); + break; + case LINK_LOCAL_DETACH: + handler.onLinkLocalDetach(this); + break; + case LINK_REMOTE_DETACH: + handler.onLinkRemoteDetach(this); + break; + case LINK_LOCAL_CLOSE: + handler.onLinkLocalClose(this); + break; + case LINK_REMOTE_CLOSE: + handler.onLinkRemoteClose(this); + break; + case LINK_FLOW: + handler.onLinkFlow(this); + break; + case LINK_FINAL: + handler.onLinkFinal(this); + break; + case DELIVERY: + handler.onDelivery(this); + break; + case TRANSPORT: + handler.onTransport(this); + break; + case TRANSPORT_ERROR: + handler.onTransportError(this); + break; + case TRANSPORT_HEAD_CLOSED: + handler.onTransportHeadClosed(this); + break; + case TRANSPORT_TAIL_CLOSED: + handler.onTransportTailClosed(this); + break; + case TRANSPORT_CLOSED: + handler.onTransportClosed(this); + break; + case REACTOR_FINAL: + handler.onReactorFinal(this); + break; + case REACTOR_QUIESCED: + handler.onReactorQuiesced(this); + break; + case REACTOR_INIT: + handler.onReactorInit(this); + break; + case SELECTABLE_ERROR: + handler.onSelectableError(this); + break; + case SELECTABLE_EXPIRED: + handler.onSelectableExpired(this); + break; + case SELECTABLE_FINAL: + handler.onSelectableFinal(this); + break; + case SELECTABLE_INIT: + handler.onSelectableInit(this); + break; + case SELECTABLE_READABLE: + handler.onSelectableReadable(this); + break; + case SELECTABLE_UPDATED: + handler.onSelectableWritable(this); + break; + case SELECTABLE_WRITABLE: + handler.onSelectableWritable(this); + break; + case TIMER_TASK: + handler.onTimerTask(this); + break; + default: + handler.onUnhandled(this); + break; + } + } catch(RuntimeException runtimeException) { + throw new HandlerException(handler, runtimeException); } Iterator<Handler> children = handler.children(); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java ---------------------------------------------------------------------- diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java index d6cca72..3201c5a 100644 --- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java +++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java @@ -27,6 +27,7 @@ import java.util.Set; import org.apache.qpid.proton.engine.Collector; import org.apache.qpid.proton.engine.Connection; import org.apache.qpid.proton.engine.Handler; +import org.apache.qpid.proton.engine.HandlerException; import org.apache.qpid.proton.engine.Record; import org.apache.qpid.proton.reactor.impl.ReactorImpl; @@ -72,15 +73,15 @@ public interface Reactor { public boolean quiesced(); - public boolean process(); + public boolean process() throws HandlerException; public void wakeup() throws IOException; public void start() ; - public void stop() ; + public void stop() throws HandlerException; - public void run(); + public void run() throws HandlerException; // pn_reactor_schedule from reactor.c public Task schedule(int delay, Handler handler); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java ---------------------------------------------------------------------- diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java index db20601..94d3595 100644 --- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java +++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java @@ -35,6 +35,7 @@ import org.apache.qpid.proton.engine.Event; import org.apache.qpid.proton.engine.Event.Type; import org.apache.qpid.proton.engine.Extendable; import org.apache.qpid.proton.engine.Handler; +import org.apache.qpid.proton.engine.HandlerException; import org.apache.qpid.proton.engine.Record; import org.apache.qpid.proton.engine.impl.CollectorImpl; import org.apache.qpid.proton.engine.impl.ConnectionImpl; @@ -253,7 +254,7 @@ public class ReactorImpl implements Reactor, Extendable { } @Override - public boolean process() { + public boolean process() throws HandlerException { mark(); Type previous = null; while (true) { @@ -308,9 +309,9 @@ public class ReactorImpl implements Reactor, Extendable { } @Override - public void stop() { + public void stop() throws HandlerException { collector.put(Type.REACTOR_FINAL, this); - // (Comment from C code) XXX: should consider removing this fron stop to avoid reentrance + // (Comment from C code) XXX: should consider removing this from stop to avoid reentrance process(); collector = null; } @@ -320,7 +321,7 @@ public class ReactorImpl implements Reactor, Extendable { } @Override - public void run() { + public void run() throws HandlerException { setTimeout(3141); start(); while(process()) {} http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/973b429c/proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java ---------------------------------------------------------------------- diff --git a/proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java b/proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java index c784849..61e2761 100644 --- a/proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java +++ b/proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -32,6 +33,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import junit.framework.AssertionFailedError; + import org.apache.qpid.proton.Proton; import org.apache.qpid.proton.engine.BaseHandler; import org.apache.qpid.proton.engine.Connection; @@ -39,6 +42,7 @@ import org.apache.qpid.proton.engine.Delivery; import org.apache.qpid.proton.engine.Event; import org.apache.qpid.proton.engine.Event.Type; import org.apache.qpid.proton.engine.Handler; +import org.apache.qpid.proton.engine.HandlerException; import org.apache.qpid.proton.engine.Sender; import org.apache.qpid.proton.engine.Session; import org.apache.qpid.proton.reactor.impl.AcceptorImpl; @@ -384,4 +388,179 @@ public class ReactorTest { Type.SELECTABLE_FINAL, Type.REACTOR_FINAL); taskHandler.assertEvents(Type.TIMER_TASK); } + + private class BarfException extends RuntimeException { + private static final long serialVersionUID = -5891140258375562884L; + } + + private class BarfOnSomethingHandler extends BaseHandler { + protected final BarfException exception; + + protected BarfOnSomethingHandler(BarfException exception) { + this.exception = exception; + } + } + + private class BarfOnReactorInit extends BarfOnSomethingHandler { + + protected BarfOnReactorInit(BarfException exception) { + super(exception); + } + + @Override + public void onReactorInit(Event e) { + throw exception; + } + } + + private class BarfOnReactorFinal extends BarfOnSomethingHandler { + + protected BarfOnReactorFinal(BarfException exception) { + super(exception); + } + + @Override + public void onReactorFinal(Event event) { + throw exception; + } + } + + private class BarfOnConnectionInit extends BarfOnSomethingHandler { + + protected BarfOnConnectionInit(BarfException exception) { + super(exception); + } + + @Override + public void onConnectionInit(Event e) { + throw exception; + } + } + + private class BarfOnSessionInit extends BarfOnSomethingHandler { + + protected BarfOnSessionInit(BarfException exception) { + super(exception); + } + + @Override + public void onSessionInit(Event e) { + throw exception; + } + } + + private class BarfOnLinkInit extends BarfOnSomethingHandler { + + protected BarfOnLinkInit(BarfException exception) { + super(exception); + } + + @Override + public void onLinkInit(Event e) { + throw exception; + } + } + + private class BarfOnTask extends BarfOnSomethingHandler { + + protected BarfOnTask(BarfException exception) { + super(exception); + } + + @Override + public void onTimerTask(Event e) { + throw exception; + } + } + + private void assertReactorRunBarfsOnHandler(Reactor reactor, BarfException expectedException, Handler expectedHandler) { + try { + reactor.run(); + throw new AssertionFailedError("Reactor.run() should have thrown an exception"); + } catch(HandlerException handlerException) { + assertSame("Linked exception does not match expected exception", expectedException, handlerException.getCause()); + assertSame("Handler in exception does not match expected handler", expectedHandler, handlerException.getHandler()); + } + } + + @Test + public void barfInReactorFinal() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnReactorFinal(expectedBarf); + reactor.getGlobalHandler().add(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnGlobalSet() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnReactorInit(expectedBarf); + reactor.setGlobalHandler(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnGlobalAdd() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnReactorInit(expectedBarf); + reactor.getGlobalHandler().add(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnReactorSet() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnReactorInit(expectedBarf); + reactor.setHandler(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnReactorAdd() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnReactorInit(expectedBarf); + reactor.getHandler().add(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnConnection() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnConnectionInit(expectedBarf); + reactor.connection(expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnSession() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnSessionInit(expectedBarf); + reactor.connection(expectedHandler).session(); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnLink() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnLinkInit(expectedBarf); + reactor.connection(expectedHandler).session().sender("barf"); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } + + @Test + public void barfOnSchedule() throws IOException { + BarfException expectedBarf = new BarfException(); + Handler expectedHandler = new BarfOnTask(expectedBarf); + reactor.schedule(0, expectedHandler); + assertReactorRunBarfsOnHandler(reactor, expectedBarf, expectedHandler); + reactor.free(); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
