diff options
3 files changed, 189 insertions, 1 deletions
diff --git a/server/src/com/vaadin/server/DefaultErrorHandler.java b/server/src/com/vaadin/server/DefaultErrorHandler.java index 2f46354500..f8b684f1d1 100644 --- a/server/src/com/vaadin/server/DefaultErrorHandler.java +++ b/server/src/com/vaadin/server/DefaultErrorHandler.java @@ -16,11 +16,14 @@ package com.vaadin.server; +import java.lang.reflect.InvocationTargetException; import java.net.SocketException; import java.util.logging.Level; import java.util.logging.Logger; +import com.vaadin.event.ListenerMethod.MethodException; import com.vaadin.server.ClientConnector.ConnectorErrorEvent; +import com.vaadin.server.ServerRpcManager.RpcInvocationException; import com.vaadin.shared.Connector; import com.vaadin.ui.AbstractComponent; import com.vaadin.ui.Component; @@ -32,7 +35,7 @@ public class DefaultErrorHandler implements ErrorHandler { } public static void doDefault(ErrorEvent event) { - final Throwable t = event.getThrowable(); + Throwable t = event.getThrowable(); if (t instanceof SocketException) { // Most likely client browser closed socket getLogger().info( @@ -41,6 +44,8 @@ public class DefaultErrorHandler implements ErrorHandler { return; } + t = findRelevantThrowable(t); + // Finds the original source of the error/exception AbstractComponent component = findAbstractComponent(event); if (component != null) { @@ -54,6 +59,40 @@ public class DefaultErrorHandler implements ErrorHandler { getLogger().log(Level.SEVERE, "", t); } + /** + * Vaadin wraps exceptions in its own and due to reflection usage there + * might be also other irrelevant exceptions that make no sense for Vaadin + * users (~developers using Vaadin). This method tries to choose the + * relevant one to be reported. + * + * @since 7.2 + * @param t + * throwable given for default error handler + * @return the throwable that is relevant for Vaadin users + */ + private static Throwable findRelevantThrowable(Throwable t) { + try { + if ((t instanceof RpcInvocationException) + && (t.getCause() instanceof InvocationTargetException)) { + /* + * RpcInvocationException (that always wraps irrelevant + * java.lang.reflect.InvocationTargetException) might only be + * relevant for core Vaadin developers. + */ + return findRelevantThrowable(t.getCause().getCause()); + } else if (t instanceof MethodException) { + /* + * Method exception might only be relevant for core Vaadin + * developers. + */ + return t.getCause(); + } + } catch (Exception e) { + // NOP, just return the original one + } + return t; + } + private static Logger getLogger() { return Logger.getLogger(DefaultErrorHandler.class.getName()); } diff --git a/uitest/src/com/vaadin/tests/components/SaneErrors.java b/uitest/src/com/vaadin/tests/components/SaneErrors.java new file mode 100644 index 0000000000..b82c1dd18b --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/SaneErrors.java @@ -0,0 +1,86 @@ +package com.vaadin.tests.components; + +import com.vaadin.event.ItemClickEvent; +import com.vaadin.event.ItemClickEvent.ItemClickListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.ui.AbstractComponent; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Label; +import com.vaadin.ui.Table; +import com.vaadin.ui.Table.RowHeaderMode; +import com.vaadin.ui.VerticalLayout; + +public class SaneErrors extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final Button b = new Button("Show me my NPE!"); + b.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + throwError(); + } + + }); + + /* + * Errors from "legacy variable changes" + */ + final Table table = new Table(); + table.addItem("Show me my NPE!"); + table.setRowHeaderMode(RowHeaderMode.ID); + table.addItemClickListener(new ItemClickListener() { + @Override + public void itemClick(ItemClickEvent event) { + throwError(); + } + }); + + final VerticalLayout content = new VerticalLayout(b, table); + + /** + * Button that shows reported exception for TB integration test + */ + Button button = new Button("Collect exceptions", new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + reportException(b, content); + reportException(table, content); + } + + private void reportException(final AbstractComponent b, + final VerticalLayout content) { + String message = b.getErrorMessage().getFormattedHtmlMessage(); + message = message.replaceAll(".", "."); + message = message.substring(message.indexOf("h2>") + 3, + message.indexOf(" ")); + Label label = new Label(message); + content.addComponent(label); + } + }); + content.addComponent(button); + + setContent(content); + + } + + private void throwError() { + Object o = null; + o.getClass(); + } + + @Override + protected String getTestDescription() { + return "Vaadin should by default report exceptions relevant for the developer."; + } + + @Override + protected Integer getTicketNumber() { + return 11599; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/SaneErrorsTest.java b/uitest/src/com/vaadin/tests/components/SaneErrorsTest.java new file mode 100644 index 0000000000..902266ede3 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/SaneErrorsTest.java @@ -0,0 +1,63 @@ +/* + * 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; + +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.tests.tb3.AbstractTB3Test.RunLocally; +import com.vaadin.tests.tb3.MultiBrowserTest; + +@RunLocally +public class SaneErrorsTest extends MultiBrowserTest { + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.tb3.MultiBrowserTest#getBrowsersToTest() + */ + @Override + public List<DesiredCapabilities> getBrowsersToTest() { + return Collections.singletonList(DesiredCapabilities.firefox()); + } + + @Test + public void test() { + openTestURL(); + List<WebElement> elements = getDriver().findElements( + By.xpath("//*[text() = 'Show me my NPE!']")); + for (WebElement webElement : elements) { + webElement.click(); + } + + getDriver().findElement(By.xpath("//*[text() = 'Collect exceptions']")) + .click(); + + List<WebElement> errorMessages = getDriver().findElements( + By.className("v-label")); + for (WebElement webElement : errorMessages) { + String text = webElement.getText(); + Assert.assertEquals("java.lang.NullPointerException", text); + } + } + +} |