fix: Check for null when handling exception (#17542) (#17543)

For errors in beforeClientResponse handle cases where any part to the errorHandler is null Fixes #17352 Co-authored-by: caalador <mikael.grankvist@vaadin.com>
vaadin · Sep 4, 2023 · a5325b3 · a5325b3
1 parent fb65cee
commit a5325b3
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 2 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java b/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java
@@ -35,6 +35,7 @@
 import com.vaadin.flow.internal.nodefeature.NodeFeature;
 import com.vaadin.flow.server.DefaultErrorHandler;
 import com.vaadin.flow.server.ErrorEvent;
+import com.vaadin.flow.server.ErrorHandler;
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.server.communication.UidlWriter;
 import com.vaadin.flow.shared.Registration;
@@ -395,8 +396,7 @@ public void runExecutionsBeforeClientResponse() {
                                             .isClientSideInitialized());
                             entry.getExecution().accept(context);
                         } catch (Exception e) {
-                            if (getUI().getSession().getErrorHandler()
-                                    .getClass()
+                            if (getErrorHandlerClass()
                                     .equals(DefaultErrorHandler.class)) {
                                 throw e;
                             }
@@ -407,6 +407,15 @@ public void runExecutionsBeforeClientResponse() {
         }
     }
 
+    private Class<? extends ErrorHandler> getErrorHandlerClass() {
+        UI ui = getUI();
+        VaadinSession session = ui == null ? null : ui.getSession();
+        ErrorHandler errorHandler = session == null ? null
+                : session.getErrorHandler();
+        return errorHandler == null ? DefaultErrorHandler.class
+                : errorHandler.getClass();
+    }
+
     private List<StateTree.BeforeClientResponseEntry> flushCallbacks() {
         if (!hasCallbacks()) {
             return Collections.emptyList();

diff --git a/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java b/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java
@@ -30,7 +30,9 @@
 
 import org.apache.commons.lang3.SerializationUtils;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.component.Tag;
@@ -49,12 +51,17 @@
 import com.vaadin.flow.internal.nodefeature.ElementPropertyMap;
 import com.vaadin.flow.internal.nodefeature.NodeFeature;
 import com.vaadin.flow.internal.nodefeature.PushConfigurationMap.PushConfigurationParametersMap;
+import com.vaadin.flow.server.ErrorHandler;
+import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.tests.util.TestUtil;
 
 import elemental.json.JsonObject;
 
 public class StateTreeTest {
 
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
     private StateTree tree = new UI().getInternals().getStateTree();
 
     public static class AttachableNode extends StateNode {
@@ -556,6 +563,54 @@ public void beforeClientResponse_withAttachedNodesDuringExecution() {
                 results.get(3).intValue());
     }
 
+    @Test
+    public void beforeClientResponse_failingExecutionWithNullErrorHandler_NoNPE() {
+        thrown.expect(IllegalStateException.class);
+        thrown.reportMissingExceptionWithMessage(
+                "Failure should be thrown again for no errorhandler");
+
+        StateNode rootNode = tree.getRootNode();
+        tree.beforeClientResponse(rootNode, context -> {
+            throw new IllegalStateException("Throw before client response");
+        });
+
+        Assert.assertNull(tree.getUI().getSession());
+
+        VaadinSession mockSession = Mockito.mock(VaadinSession.class);
+        Mockito.when(mockSession.getErrorHandler()).thenReturn(null);
+
+        try {
+            tree.getUI().getInternals().setSession(mockSession);
+            tree.beforeClientResponse(rootNode, context -> {
+                throw new IllegalStateException("Throw before client response");
+            });
+
+            tree.runExecutionsBeforeClientResponse();
+        } finally {
+            tree.getUI().getInternals().setSession(null);
+        }
+    }
+
+    @Test
+    public void beforeClientResponse_failingExecutionWithNullSession_NoNPE() {
+        thrown.expect(IllegalStateException.class);
+        thrown.reportMissingExceptionWithMessage(
+                "Failure should be thrown again for no errorhandler");
+
+        StateNode rootNode = tree.getRootNode();
+        tree.beforeClientResponse(rootNode, context -> {
+            throw new IllegalStateException("Throw before client response");
+        });
+
+        Assert.assertNull(tree.getUI().getSession());
+
+        tree.beforeClientResponse(rootNode, context -> {
+            throw new IllegalStateException("Throw before client response");
+        });
+
+        tree.runExecutionsBeforeClientResponse();
+    }
+
     @Test
     public void beforeClientResponse_nodeGarbageCollectedDespiteClosure()
             throws InterruptedException {