Fixed swallower access. Now error handler logs exceptions. (#12703)

Change-Id: If8fe00e10c7ec56cbd8753ff88d4816613a340f2

6 files changed, 330 insertions, 2 deletions

diff --git a/server/src/com/vaadin/server/ErrorHandlingRunnable.java b/server/src/com/vaadin/server/ErrorHandlingRunnable.java
new file mode 100644
index 0000000000..8ae6ce3d5d
--- /dev/null
+++ b/server/src/com/vaadin/server/ErrorHandlingRunnable.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed 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 com.vaadin.server;
+
+/**
+ * Defines the interface to handle exceptions thrown during the execution of a
+ * FutureAccess.
+ * 
+ * @since 7.1.8
+ * @author Vaadin Ltd
+ */
+public interface ErrorHandlingRunnable extends Runnable {
+
+    /**
+     * Handles exceptions thrown during the execution of a FutureAccess.
+     * 
+     * @since 7.1.8
+     * @param exception
+     *            the thrown exception.
+     */
+    public void handleError(Exception exception);
+
+}
diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index c9a5f0974a..44ceaaaf87 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -1745,6 +1745,13 @@ public abstract class VaadinService implements Serializable {
                             .getCurrentInstances());
                     CurrentInstance.setCurrent(session);
                     pendingAccess.run();
+
+                    try {
+                        pendingAccess.get();
+
+                    } catch (Exception exception) {
+                        pendingAccess.handleError(exception);
+                    }
                 }
             }
         } finally {
@@ -1752,5 +1759,4 @@ public abstract class VaadinService implements Serializable {
             CurrentInstance.restoreInstances(oldInstances);
         }
     }
-
 }
diff --git a/server/src/com/vaadin/server/VaadinSession.java b/server/src/com/vaadin/server/VaadinSession.java
index c73ac8b686..fd2ed79acd 100644
--- a/server/src/com/vaadin/server/VaadinSession.java
+++ b/server/src/com/vaadin/server/VaadinSession.java
@@ -35,6 +35,7 @@ import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import javax.portlet.PortletSession;
@@ -86,6 +87,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
         private final Map<Class<?>, CurrentInstance> instances = CurrentInstance
                 .getInstances(true);
         private final VaadinSession session;
+        private Runnable runnable;
 
         /**
          * Creates an instance for the given runnable
@@ -100,6 +102,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
         public FutureAccess(VaadinSession session, Runnable runnable) {
             super(runnable, null);
             this.session = session;
+            this.runnable = runnable;
         }
 
         @Override
@@ -129,6 +132,36 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
         public Map<Class<?>, CurrentInstance> getCurrentInstances() {
             return instances;
         }
+
+        /**
+         * Handles exceptions thrown during the execution of this task.
+         * 
+         * @since 7.1.8
+         * @param exception
+         *            the thrown exception.
+         */
+        public void handleError(Exception exception) {
+            try {
+                if (runnable instanceof ErrorHandlingRunnable) {
+                    ErrorHandlingRunnable errorHandlingRunnable = (ErrorHandlingRunnable) runnable;
+
+                    errorHandlingRunnable.handleError(exception);
+                } else {
+                    ErrorEvent errorEvent = new ErrorEvent(exception);
+
+                    ErrorHandler errorHandler = ErrorEvent
+                            .findErrorHandler(session);
+
+                    if (errorHandler == null) {
+                        errorHandler = new DefaultErrorHandler();
+                    }
+
+                    errorHandler.error(errorEvent);
+                }
+            } catch (Exception e) {
+                getLogger().log(Level.SEVERE, e.getMessage(), e);
+            }
+        }
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index 27f04c39e1..746fa194ac 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -24,6 +24,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Future;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import com.vaadin.event.Action;
@@ -35,6 +36,9 @@ import com.vaadin.navigator.Navigator;
 import com.vaadin.server.ClientConnector;
 import com.vaadin.server.ComponentSizeValidator;
 import com.vaadin.server.ComponentSizeValidator.InvalidLayout;
+import com.vaadin.server.DefaultErrorHandler;
+import com.vaadin.server.ErrorHandler;
+import com.vaadin.server.ErrorHandlingRunnable;
 import com.vaadin.server.LocaleService;
 import com.vaadin.server.Page;
 import com.vaadin.server.PaintException;
@@ -1287,11 +1291,36 @@ public abstract class UI extends AbstractSingleComponentContainer implements
             throw new UIDetachedException();
         }
 
-        return session.access(new Runnable() {
+        return session.access(new ErrorHandlingRunnable() {
             @Override
             public void run() {
                 accessSynchronously(runnable);
             }
+
+            @Override
+            public void handleError(Exception exception) {
+                try {
+                    if (runnable instanceof ErrorHandlingRunnable) {
+                        ErrorHandlingRunnable errorHandlingRunnable = (ErrorHandlingRunnable) runnable;
+
+                        errorHandlingRunnable.handleError(exception);
+                    } else {
+                        ConnectorErrorEvent errorEvent = new ConnectorErrorEvent(
+                                UI.this, exception);
+
+                        ErrorHandler errorHandler = com.vaadin.server.ErrorEvent
+                                .findErrorHandler(UI.this);
+
+                        if (errorHandler == null) {
+                            errorHandler = new DefaultErrorHandler();
+                        }
+
+                        errorHandler.error(errorEvent);
+                    }
+                } catch (Exception e) {
+                    getLogger().log(Level.SEVERE, e.getMessage(), e);
+                }
+            }
         });
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.html b/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.html
new file mode 100644
index 0000000000..94d8aa2777
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.html
@@ -0,0 +1,67 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="http://localhost:8888/" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.ui.UIAccessExceptionHandling?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_0</td>
+	<td>1. Exception catched on get: java.util.concurrent.ExecutionException</td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_1</td>
+	<td>0. Exception catched on execution with ConnectorErrorEvent : java.util.concurrent.ExecutionException</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[1]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_0</td>
+	<td>1. Exception catched on get: java.util.concurrent.ExecutionException</td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_1</td>
+	<td>0. Exception catched on execution with ErrorEvent : java.util.concurrent.ExecutionException</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::/VVerticalLayout[0]/Slot[2]/VVerticalLayout[0]/Slot[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_0</td>
+	<td>1. Exception catched on get: java.util.concurrent.ExecutionException</td>
+</tr>
+<tr>
+	<td>verifyText</td>
+	<td>vaadin=runcomvaadintestscomponentsuiUIAccessExceptionHandling::PID_SLog_row_1</td>
+	<td>0. Exception catched on execution with ConnectorErrorEvent : java.util.concurrent.ExecutionException</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java b/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java
new file mode 100644
index 0000000000..1cd4be576b
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java
@@ -0,0 +1,157 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed 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 com.vaadin.tests.components.ui;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+
+import com.vaadin.server.DefaultErrorHandler;
+import com.vaadin.server.ErrorHandler;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinService;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.UI;
+import com.vaadin.util.CurrentInstance;
+
+public class UIAccessExceptionHandling extends AbstractTestUIWithLog implements
+        ErrorHandler {
+
+    private Future<Void> future;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        getSession().setErrorHandler(this);
+
+        addComponent(new Button("Throw RuntimeException on UI.access",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        log.clear();
+
+                        // Ensure beforeClientResponse is invoked
+                        markAsDirty();
+
+                        future = access(new Runnable() {
+                            @Override
+                            public void run() {
+                                throw new RuntimeException();
+                            }
+                        });
+                    }
+                }));
+
+        addComponent(new Button("Throw RuntimeException on Session.access",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        log.clear();
+
+                        // Ensure beforeClientResponse is invoked
+                        markAsDirty();
+
+                        VaadinService service = VaadinService.getCurrent();
+
+                        future = service.accessSession(getSession(),
+                                new Runnable() {
+
+                                    @Override
+                                    public void run() {
+                                        throw new RuntimeException();
+                                    }
+                                });
+                    }
+                }));
+
+        addComponent(new Button(
+                "Throw RuntimeException after removing instances",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        log.clear();
+
+                        // Ensure beforeClientResponse is invoked
+                        markAsDirty();
+
+                        assert UI.getCurrent() == UIAccessExceptionHandling.this;
+
+                        Map<Class<?>, CurrentInstance> instances = CurrentInstance
+                                .getInstances(false);
+                        CurrentInstance.clearAll();
+
+                        assert UI.getCurrent() == null;
+
+                        future = access(new Runnable() {
+                            @Override
+                            public void run() {
+                                throw new RuntimeException();
+                            }
+                        });
+
+                        CurrentInstance.restoreInstances(instances);
+                    }
+                }));
+
+        addComponent(new Button("Clear", new Button.ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                log.clear();
+            }
+        }));
+    }
+
+    @Override
+    public void beforeClientResponse(boolean initial) {
+        if (future != null) {
+            try {
+                future.get();
+            } catch (InterruptedException e) {
+                e.printStackTrace();
+            } catch (ExecutionException e) {
+                log("Exception catched on get: " + e.getClass().getName());
+            } finally {
+                future = null;
+            }
+        }
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Test for handling exceptions in UI.access and Session.access";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(12703);
+    }
+
+    @Override
+    public void error(com.vaadin.server.ErrorEvent event) {
+        log("Exception catched on execution with "
+                + event.getClass().getSimpleName() + " : "
+                + event.getThrowable().getClass().getName());
+
+        DefaultErrorHandler.doDefault(event);
+    }
+
+}