You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2014/11/28 17:09:37 UTC
[1/2] isis git commit: ISIS-960: Make the event bus handle exceptions
more gracefully, not swallow them.
Repository: isis
Updated Branches:
refs/heads/master 3969f34c5 -> 2aa0df30a
ISIS-960: Make the event bus handle exceptions more gracefully, not swallow them.
Project: http://git-wip-us.apache.org/repos/asf/isis/repo
Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/2d1b44d9
Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/2d1b44d9
Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/2d1b44d9
Branch: refs/heads/master
Commit: 2d1b44d9a183e7b4a8a35284560fec7e0f54b313
Parents: 3969f34
Author: Jeroen van der Wal <je...@stromboli.it>
Authored: Thu Nov 27 18:14:12 2014 +0100
Committer: Jeroen van der Wal <je...@stromboli.it>
Committed: Thu Nov 27 18:14:12 2014 +0100
----------------------------------------------------------------------
.../eventbus/EventBusServiceDefault.java | 36 ++++++++++++++++----
1 file changed, 30 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/isis/blob/2d1b44d9/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
index cd3626c..9b70ab8 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
@@ -17,13 +17,18 @@
package org.apache.isis.core.runtime.services.eventbus;
import java.util.List;
+
import javax.enterprise.context.RequestScoped;
+
import com.google.common.base.Throwables;
import com.google.common.eventbus.EventBus;
import com.google.common.eventbus.SubscriberExceptionContext;
import com.google.common.eventbus.SubscriberExceptionHandler;
+
import org.apache.isis.applib.NonRecoverableException;
import org.apache.isis.applib.RecoverableException;
+import org.apache.isis.applib.services.eventbus.AbstractInteractionEvent;
+import org.apache.isis.applib.services.eventbus.AbstractInteractionEvent.Phase;
import org.apache.isis.applib.services.eventbus.EventBusService;
import org.apache.isis.core.commons.exceptions.IsisApplicationException;
import org.apache.isis.core.metamodel.facets.Annotations;
@@ -78,14 +83,33 @@ public class EventBusServiceDefault extends EventBusService {
return new SubscriberExceptionHandler(){
@Override
public void handleException(Throwable exception, SubscriberExceptionContext context) {
- final List<Throwable> causalChain = Throwables.getCausalChain(exception);
- for (Throwable cause : causalChain) {
- if(cause instanceof RecoverableException || cause instanceof NonRecoverableException) {
- getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
- return;
+ Object event = context.getEvent();
+ if(!(event instanceof AbstractInteractionEvent)) {
+ return;
+ }
+ final AbstractInteractionEvent<?> interactionEvent = (AbstractInteractionEvent<?>) event;
+ final Phase phase = interactionEvent.getPhase();
+ switch (phase) {
+ case HIDE:
+ interactionEvent.hide();
+ break;
+ case DISABLE:
+ interactionEvent.disable(exception.getMessage()!=null?exception.getMessage(): exception.getClass().getName() + " thrown.");
+ break;
+ case VALIDATE:
+ interactionEvent.invalidate(exception.getMessage()!=null?exception.getMessage(): exception.getClass().getName() + " thrown.");
+ break;
+ case EXECUTING:
+ case EXECUTED:
+ final List<Throwable> causalChain = Throwables.getCausalChain(exception);
+ for (Throwable cause : causalChain) {
+ if(cause instanceof RecoverableException || cause instanceof NonRecoverableException) {
+ getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
+ return;
+ }
}
+ break;
}
- // otherwise simply ignore
}
};
}
[2/2] isis git commit: ISIS-960: handle exception for subscriber when
invoked no-arg action. More robust exception handler strategy.
Posted by da...@apache.org.
ISIS-960: handle exception for subscriber when invoked no-arg action. More robust exception handler strategy.
If a no-arg action is invoked which generates an exception in a subscriber, then the Wicket UI should detect this and re-render the original object with any messages generated.
In addition, the exception handler strategy previously would only abort the transcation if a NonRecoverableException or a RecoverableException was thrown. This is too narrow; should abort the transaction for any exception thrown
Project: http://git-wip-us.apache.org/repos/asf/isis/repo
Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/2aa0df30
Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/2aa0df30
Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/2aa0df30
Branch: refs/heads/master
Commit: 2aa0df30a968d6cc03b029dad700e994b23da124
Parents: 2d1b44d
Author: Dan Haywood <da...@haywood-associates.co.uk>
Authored: Fri Nov 28 15:59:12 2014 +0000
Committer: Dan Haywood <da...@haywood-associates.co.uk>
Committed: Fri Nov 28 16:03:18 2014 +0000
----------------------------------------------------------------------
.../ui/components/actions/ActionPanel.java | 20 ++++++++---
.../eventbus/EventBusServiceDefault.java | 34 ++++++++++--------
.../integration/tests/ToDoItemIntegTest.java | 37 +++++++++-----------
3 files changed, 52 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/isis/blob/2aa0df30/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
----------------------------------------------------------------------
diff --git a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
index e2f85ca..b2959ff 100644
--- a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
+++ b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
@@ -20,15 +20,12 @@
package org.apache.isis.viewer.wicket.ui.components.actions;
import java.util.List;
-
import com.google.common.base.Throwables;
-
import org.apache.wicket.ajax.AjaxRequestTarget;
import org.apache.wicket.markup.html.WebMarkupContainer;
import org.apache.wicket.markup.html.basic.Label;
import org.apache.wicket.markup.html.form.Form;
import org.apache.wicket.model.Model;
-
import org.apache.isis.applib.RecoverableException;
import org.apache.isis.applib.services.command.Command;
import org.apache.isis.applib.services.command.Command.Executor;
@@ -104,7 +101,22 @@ public class ActionPanel extends PanelAbstract<ActionModel> implements ActionExe
if (actionModel.hasParameters()) {
buildGuiForParameters();
} else {
- executeActionAndProcessResults(null, null);
+
+ boolean succeeded = executeActionAndProcessResults(null, null);
+ if(succeeded) {
+ // nothing to do
+ } else {
+
+ // render the target entity again
+ //
+ // (One way this can occur is if an event subscriber has a defect and throws an exception; in which case
+ // the EventBus' exception handler will automatically veto. This results in a growl message rather than
+ // an error page, but is probably 'good enough').
+ final ObjectAdapter targetAdapter = actionModel.getTargetAdapter();
+
+ ActionResultResponse resultResponse = ActionResultResponseType.OBJECT.interpretResult(this.getActionModel(), targetAdapter, null);
+ resultResponse.getHandlingStrategy().handleResults(this, resultResponse);
+ }
}
}
http://git-wip-us.apache.org/repos/asf/isis/blob/2aa0df30/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
index 9b70ab8..1b3c826 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/eventbus/EventBusServiceDefault.java
@@ -16,17 +16,12 @@
*/
package org.apache.isis.core.runtime.services.eventbus;
-import java.util.List;
-
import javax.enterprise.context.RequestScoped;
-
-import com.google.common.base.Throwables;
import com.google.common.eventbus.EventBus;
import com.google.common.eventbus.SubscriberExceptionContext;
import com.google.common.eventbus.SubscriberExceptionHandler;
-
-import org.apache.isis.applib.NonRecoverableException;
-import org.apache.isis.applib.RecoverableException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.apache.isis.applib.services.eventbus.AbstractInteractionEvent;
import org.apache.isis.applib.services.eventbus.AbstractInteractionEvent.Phase;
import org.apache.isis.applib.services.eventbus.EventBusService;
@@ -44,6 +39,8 @@ import org.apache.isis.core.runtime.system.transaction.IsisTransactionManager;
@Deprecated
public class EventBusServiceDefault extends EventBusService {
+ private static final Logger LOG = LoggerFactory.getLogger(EventBusServiceDefault.class);
+
/**
* {@inheritDoc}
*
@@ -85,35 +82,44 @@ public class EventBusServiceDefault extends EventBusService {
public void handleException(Throwable exception, SubscriberExceptionContext context) {
Object event = context.getEvent();
if(!(event instanceof AbstractInteractionEvent)) {
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("Ignoring exception '%s' (%s), not a subclass of AbstractInteractionEvent", exception.getMessage(), exception.getClass().getName());
+ }
return;
}
final AbstractInteractionEvent<?> interactionEvent = (AbstractInteractionEvent<?>) event;
final Phase phase = interactionEvent.getPhase();
switch (phase) {
case HIDE:
+ LOG.warn("Exception '%s' (%s) thrown during HIDE phase, to be safe will veto (hide) the interaction event");
interactionEvent.hide();
break;
case DISABLE:
+ LOG.warn("Exception '%s' (%s) thrown during DISABLE phase, to be safe will veto (disable) the interaction event");
interactionEvent.disable(exception.getMessage()!=null?exception.getMessage(): exception.getClass().getName() + " thrown.");
break;
case VALIDATE:
+ LOG.warn("Exception '%s' (%s) thrown during VALIDATE phase, to be safe will veto (invalidate) the interaction event");
interactionEvent.invalidate(exception.getMessage()!=null?exception.getMessage(): exception.getClass().getName() + " thrown.");
break;
case EXECUTING:
+ LOG.warn("Exception '%s' (%s) thrown during EXECUTING phase, to be safe will abort the transaction");
+ abortTransaction(exception);
+ break;
case EXECUTED:
- final List<Throwable> causalChain = Throwables.getCausalChain(exception);
- for (Throwable cause : causalChain) {
- if(cause instanceof RecoverableException || cause instanceof NonRecoverableException) {
- getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
- return;
- }
- }
+ LOG.warn("Exception '%s' (%s) thrown during EXECUTED phase, to be safe will abort the transaction");
+ abortTransaction(exception);
break;
}
}
};
}
+ private void abortTransaction(Throwable exception) {
+ getTransactionManager().getTransaction().setAbortCause(new IsisApplicationException(exception));
+ return;
+ }
+
protected IsisTransactionManager getTransactionManager() {
return IsisContext.getTransactionManager();
}
http://git-wip-us.apache.org/repos/asf/isis/blob/2aa0df30/example/application/todoapp/integtests/src/test/java/integration/tests/ToDoItemIntegTest.java
----------------------------------------------------------------------
diff --git a/example/application/todoapp/integtests/src/test/java/integration/tests/ToDoItemIntegTest.java b/example/application/todoapp/integtests/src/test/java/integration/tests/ToDoItemIntegTest.java
index 8e961da..7b289d2 100644
--- a/example/application/todoapp/integtests/src/test/java/integration/tests/ToDoItemIntegTest.java
+++ b/example/application/todoapp/integtests/src/test/java/integration/tests/ToDoItemIntegTest.java
@@ -58,7 +58,6 @@ import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
public class ToDoItemIntegTest extends AbstractToDoIntegTest {
@@ -266,17 +265,16 @@ public class ToDoItemIntegTest extends AbstractToDoIntegTest {
}
@Test
- public void subscriberThrowingOtherExceptionIsIgnored() throws Exception {
+ public void subscriberVetoesEventWithAnyOtherException() throws Exception {
// given
toDoItemSubscriptions.subscriberBehaviour(ToDoItemSubscriptions.Behaviour.AnyExecuteVetoWithOtherException);
+ // then
+ expectedExceptions.expect(RuntimeException.class);
+
// when
toDoItem.completed();
-
- // then
- // (no expectedExceptions setup, expect to continue)
- assertTrue(true);
}
}
@@ -569,17 +567,16 @@ public class ToDoItemIntegTest extends AbstractToDoIntegTest {
}
@Test
- public void subscriberThrowingOtherExceptionIsIgnored() throws Exception {
+ public void subscriberVetoesEventWithAnyOtherException() throws Exception {
// given
toDoItemSubscriptions.subscriberBehaviour(ToDoItemSubscriptions.Behaviour.AnyExecuteVetoWithOtherException);
+ // then
+ expectedExceptions.expect(RuntimeException.class);
+
// when
toDoItem.add(otherToDoItem);
-
- // then
- // (no expectedExceptions setup, expect to continue)
- assertTrue(true);
}
}
public static class Remove extends ToDoItemIntegTest {
@@ -667,17 +664,16 @@ public class ToDoItemIntegTest extends AbstractToDoIntegTest {
}
@Test
- public void subscriberThrowingOtherExceptionIsIgnored() throws Exception {
+ public void subscriberVetoesEventWithAnyOtherException() throws Exception {
// given
toDoItemSubscriptions.subscriberBehaviour(ToDoItemSubscriptions.Behaviour.AnyExecuteVetoWithOtherException);
+ // then
+ expectedExceptions.expect(RuntimeException.class);
+
// when
toDoItem.remove(otherToDoItem);
-
- // then
- // (no expectedExceptions setup, expect to continue)
- assertTrue(true);
}
}
}
@@ -908,17 +904,16 @@ public class ToDoItemIntegTest extends AbstractToDoIntegTest {
@Test
- public void subscriberThrowingOtherExceptionIsIgnored() throws Exception {
+ public void subscriberVetoesEventWithAnyOtherException() throws Exception {
// given
toDoItemSubscriptions.subscriberBehaviour(ToDoItemSubscriptions.Behaviour.AnyExecuteVetoWithOtherException);
+ // then
+ expectedExceptions.expect(RuntimeException.class);
+
// when
toDoItem.setDescription("Buy bread and butter");
-
- // then
- // (no expectedExceptions setup, expect to continue)
- assertTrue(true);
}