[java] Allow EventFiringDecorator to throw exceptions #16470

WebDriverListener has now the throwsExceptions method to configure
its behavior with regard to exception management. By default,
it returns false, meaning exceptions are suppressed. If overridden
to return true, exceptions occurred in the listener execution
will be rethrown, so to allow users to manage them on their side.

Fixes #16470

Author: giulong

java/src/org/openqa/selenium/support/events/EventFiringDecorator.java

@@ -154,9 +154,10 @@
  *
  * <p>Just be careful to not block the current thread in a listener method!
  *
- * <p>Listeners can't affect driver behavior too much. They can't throw any exceptions (they can,
- * but the decorator suppresses these exceptions), can't prevent execution of the decorated methods,
- * can't modify parameters and results of the methods.
+ * <p>Listeners can't affect driver behavior too much. They can't prevent execution of the decorated
+ * methods, can't modify parameters and results of the methods. They can throw exceptions only if
+ * configured to do so by overriding {@link WebDriverListener#throwsExceptions}. By default,
+ * exceptions occurred in listeners execution are suppressed.
  *
  * <p>Decorators that modify the behaviour of the underlying drivers should be implemented by
  * extending {@link WebDriverDecorator}, not by creating sophisticated listeners.
@@ -217,6 +218,10 @@ private void fireBeforeEvents(
       listener.beforeAnyCall(target.getOriginal(), method, args);
     } catch (Throwable t) {
       LOG.log(Level.WARNING, t.getMessage(), t);
+
+      if (listener.throwsExceptions()) {
+        throw new WebDriverListenerException(method, t);
+      }
     }
 
     try {
@@ -240,6 +245,10 @@ private void fireBeforeEvents(
       }
     } catch (Throwable t) {
       LOG.log(Level.WARNING, t.getMessage(), t);
+
+      if (listener.throwsExceptions()) {
+        throw new WebDriverListenerException(method, t);
+      }
     }
 
     String methodName = createEventMethodName("before", method.getName());
@@ -291,12 +300,20 @@ private void fireAfterEvents(
       }
     } catch (Throwable t) {
       LOG.log(Level.WARNING, t.getMessage(), t);
+
+      if (listener.throwsExceptions()) {
+        throw new WebDriverListenerException(method, t);
+      }
     }
 
     try {
       listener.afterAnyCall(target.getOriginal(), method, args, res);
     } catch (Throwable t) {
       LOG.log(Level.WARNING, t.getMessage(), t);
+
+      if (listener.throwsExceptions()) {
+        throw new WebDriverListenerException(method, t);
+      }
     }
   }
 
@@ -355,6 +372,10 @@ private void callListenerMethod(Method m, WebDriverListener listener, Object[] a
       m.invoke(listener, args);
     } catch (Throwable t) {
       LOG.log(Level.WARNING, t.getMessage(), t);
+
+      if (listener.throwsExceptions()) {
+        throw new WebDriverListenerException(m, t);
+      }
     }
   }
 }

java/src/org/openqa/selenium/support/events/WebDriverListener.java

@@ -50,6 +50,18 @@
 @Beta
 public interface WebDriverListener {
 
+  // Listener configuration
+
+  /**
+   * This method configures the behavior of the listener with regard to exceptions occurred during
+   * its execution. By default, exceptions are suppressed.
+   *
+   * @return false by default. Override it and return true to throw exceptions instead.
+   */
+  default boolean throwsExceptions() {
+    return false;
+  }
+
   // Global
 
   /**

java/src/org/openqa/selenium/support/events/WebDriverListenerException.java

@@ -0,0 +1,36 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC 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.openqa.selenium.support.events;
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+public class WebDriverListenerException extends RuntimeException {
+
+  public WebDriverListenerException(Method method, Throwable cause) {
+    super(
+        "Exception executing listener method "
+            + method.getDeclaringClass().getSimpleName()
+            + "#"
+            + method.getName()
+            + " with parameter types "
+            + Arrays.toString(
+                Arrays.stream(method.getParameterTypes()).map(Class::getSimpleName).toArray()),
+        cause);
+  }
+}

java/test/org/openqa/selenium/support/events/EventFiringDecoratorTest.java

@@ -17,9 +17,7 @@
 
 package org.openqa.selenium.support.events;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
@@ -1050,6 +1048,29 @@ public void beforeAnyCall(Object target, Method method, Object[] args) {
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInBeforeAnyCall() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void beforeAnyCall(Object target, Method method, Object[] args) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInBeforeClassMethodCall() {
     WebDriver driver = mock(WebDriver.class);
@@ -1066,6 +1087,29 @@ public void beforeAnyWebDriverCall(WebDriver driver, Method method, Object[] arg
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInBeforeClassMethodCall() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void beforeAnyWebDriverCall(WebDriver driver, Method method, Object[] args) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInBeforeMethod() {
     WebDriver driver = mock(WebDriver.class);
@@ -1082,6 +1126,29 @@ public void beforeGetWindowHandle(WebDriver driver) {
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInBeforeMethod() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void beforeGetWindowHandle(WebDriver driver) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInAfterAnyCall() {
     WebDriver driver = mock(WebDriver.class);
@@ -1098,6 +1165,29 @@ public void afterAnyCall(Object target, Method method, Object[] args, Object res
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInAfterAnyCall() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void afterAnyCall(Object target, Method method, Object[] args, Object result) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInAfterClassMethodCall() {
     WebDriver driver = mock(WebDriver.class);
@@ -1115,6 +1205,30 @@ public void afterAnyWebDriverCall(
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInAfterClassMethodCall() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void afterAnyWebDriverCall(
+              WebDriver driver, Method method, Object[] args, Object result) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInAfterMethod() {
     WebDriver driver = mock(WebDriver.class);
@@ -1131,6 +1245,29 @@ public void afterGetWindowHandle(WebDriver driver, String result) {
     assertThatNoException().isThrownBy(decorated::getWindowHandle);
   }
 
+  @Test
+  void shouldReThrowExceptionInAfterMethod() {
+    WebDriver driver = mock(WebDriver.class);
+    WebDriverListener listener =
+        new WebDriverListener() {
+
+          @Override
+          public boolean throwsExceptions() {
+            return true;
+          }
+
+          @Override
+          public void afterGetWindowHandle(WebDriver driver, String result) {
+            throw new RuntimeException("listener");
+          }
+        };
+
+    WebDriver decorated = new EventFiringDecorator<>(listener).decorate(driver);
+
+    assertThatExceptionOfType(WebDriverListenerException.class)
+        .isThrownBy(decorated::getWindowHandle);
+  }
+
   @Test
   void shouldSuppressExceptionInOnError() {
     WebDriver driver = mock(WebDriver.class);