Skip to content

Conversation

giulong
Copy link
Contributor

@giulong giulong commented Oct 19, 2025

User description

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

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Add throwsExceptions() method to WebDriverListener interface

  • Allow listeners to rethrow exceptions instead of suppressing them

  • Create new WebDriverListenerException for wrapped listener exceptions

  • Add comprehensive test coverage for exception handling behavior


Diagram Walkthrough

flowchart LR
  A["WebDriverListener"] -->|"new method"| B["throwsExceptions()"]
  C["EventFiringDecorator"] -->|"checks"| B
  C -->|"throws"| D["WebDriverListenerException"]
  E["Tests"] -->|"verify"| B
Loading

File Walkthrough

Relevant files
Enhancement
WebDriverListener.java
Add throwsExceptions configuration method                               

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

  • Add new throwsExceptions() default method returning false
  • Method allows listeners to configure exception handling behavior
  • Includes documentation explaining default suppression behavior
+12/-0   
EventFiringDecorator.java
Implement conditional exception rethrow in decorator         

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

  • Update documentation to reflect new exception handling capability
  • Add exception rethrow logic in fireBeforeEvents() method
  • Add exception rethrow logic in fireAfterEvents() method
  • Add exception rethrow logic in callListenerMethod() method
  • Wrap listener exceptions in WebDriverListenerException where
    applicable
+24/-3   
WebDriverListenerException.java
Create WebDriverListenerException wrapper class                   

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

  • Create new exception class extending RuntimeException
  • Wrap listener method exceptions with method name and parameters
  • Provide clear error messages for debugging listener failures
+33/-0   
Tests
EventFiringDecoratorTest.java
Add comprehensive exception rethrow test coverage               

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

  • Add test for rethrow in beforeAnyCall() with throwsExceptions=true
  • Add test for rethrow in beforeAnyWebDriverCall() with
    throwsExceptions=true
  • Add test for rethrow in beforeGetWindowHandle() with
    throwsExceptions=true
  • Add test for rethrow in afterAnyCall() with throwsExceptions=true
  • Add test for rethrow in afterAnyWebDriverCall() with
    throwsExceptions=true
  • Add test for rethrow in afterGetWindowHandle() with
    throwsExceptions=true
  • Consolidate import statements using wildcard
+134/-3 

@selenium-ci selenium-ci added C-java Java Bindings B-support Issue or PR related to support classes labels Oct 19, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 19, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@selenium-ci
Copy link
Member

Thank you, @giulong for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

Copy link
Contributor

qodo-merge-pro bot commented Oct 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consistently wrap all listener exceptions

All exceptions from listener methods should be wrapped in
WebDriverListenerException for consistency, as the current implementation
re-throws some exceptions raw while wrapping others.

Examples:

java/src/org/openqa/selenium/support/events/EventFiringDecorator.java [222-224]
      if (listener.throwsExceptions()) {
        throw t;
      }
java/src/org/openqa/selenium/support/events/EventFiringDecorator.java [376-378]
      if (listener.throwsExceptions()) {
        throw new WebDriverListenerException(m, t);
      }

Solution Walkthrough:

Before:

// In EventFiringDecorator.java
private void fireBeforeEvents(WebDriverListener listener, ...) {
  try {
    listener.beforeAnyCall(target.getOriginal(), method, args);
  } catch (Throwable t) {
    if (listener.throwsExceptions()) {
      throw t; // Raw exception is re-thrown
    }
  }
  // ...
}

private void callListenerMethod(Method m, WebDriverListener listener, Object[] args) {
  try {
    m.invoke(listener, args);
  } catch (Throwable t) {
    if (listener.throwsExceptions()) {
      throw new WebDriverListenerException(m, t); // Exception is wrapped
    }
  }
}

After:

// In EventFiringDecorator.java
private void fireBeforeEvents(WebDriverListener listener, ...) {
  try {
    listener.beforeAnyCall(target.getOriginal(), method, args);
  } catch (Throwable t) {
    if (listener.throwsExceptions()) {
      // A Method object would be needed for the existing constructor
      throw new WebDriverListenerException("beforeAnyCall", t); // Exception is consistently wrapped
    }
  }
  // ...
}

private void callListenerMethod(Method m, WebDriverListener listener, Object[] args) {
  try {
    m.invoke(listener, args);
  } catch (Throwable t) {
    if (listener.throwsExceptions()) {
      throw new WebDriverListenerException(m, t); // Exception is wrapped
    }
  }
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant design inconsistency in the new exception handling feature, which would lead to a confusing and unpredictable API for users.

Medium
Learned
best practice
Wrap rethrown listener exceptions

When rethrowing listener failures, wrap them in a consistent, descriptive
runtime exception to provide context and avoid leaking checked errors. Use
WebDriverListenerException as done elsewhere.

java/src/org/openqa/selenium/support/events/EventFiringDecorator.java [219-225]

 } catch (Throwable t) {
   LOG.log(Level.WARNING, t.getMessage(), t);
 
   if (listener.throwsExceptions()) {
-    throw t;
+    throw new WebDriverListenerException(method, t);
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Keep API and documentation accurate and consistent by using precise exception types and consistent wrapping when propagating errors.

Low
Clarify exception message detail

Improve the message to include the declaring class and parameter types (not
reflection Parameter objects) for clarity.

java/src/org/openqa/selenium/support/events/WebDriverListenerException.java [25-32]

 public WebDriverListenerException(Method method, Throwable cause) {
   super(
-      "Exception executing method "
-          + method.getName()
-          + " with args "
-          + Arrays.toString(method.getParameters()),
+      "Exception executing listener method "
+          + method.getDeclaringClass().getSimpleName() + "#" + method.getName()
+          + " with parameter types "
+          + Arrays.toString(Arrays.stream(method.getParameterTypes()).map(Class::getSimpleName).toArray()),
       cause);
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Keep API and documentation accurate and consistent by providing precise, helpful error messages that include actual argument values where safe.

Low
  • Update

@giulong giulong force-pushed the feature/issue-16470 branch from 6052f1e to b189814 Compare October 19, 2025 19:56
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 SeleniumHQ#16470
@giulong giulong force-pushed the feature/issue-16470 branch from b189814 to 73b6263 Compare October 19, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-support Issue or PR related to support classes C-java Java Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: [java] Allow EventFiringDecorator to throw exceptions fired by WebDriverListeners

3 participants