diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/ListenerCollection.java b/playwright/src/main/java/com/microsoft/playwright/impl/ListenerCollection.java index b0cce97d5..5f4159bf7 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/ListenerCollection.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/ListenerCollection.java @@ -35,6 +35,7 @@ class ListenerCollection { this.channelOwner = channelOwner; } + @SuppressWarnings("unchecked") void notify(EventType eventType, T param) { List> list = listeners.get(eventType); if (list == null) { @@ -42,7 +43,18 @@ void notify(EventType eventType, T param) { } for (Consumer listener: new ArrayList<>(list)) { - ((Consumer) listener).accept(param); + try { + ((Consumer) listener).accept(param); + } catch (RuntimeException e) { + // Listeners run on the message dispatch thread. A thrown exception + // must not break the dispatch loop or propagate to unrelated API + // calls that happen to pump messages while a listener runs (see + // issue #1952). Mirrors the JS client where async listener rejections + // surface as unhandled rejections on stderr without interrupting the + // caller — so print to stderr and keep dispatching. + System.err.println("Listener for " + eventType + " threw: " + e); + LoggingSupport.logApiIfEnabled("Listener threw exception: " + e.getMessage()); + } } } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestDownload.java b/playwright/src/test/java/com/microsoft/playwright/TestDownload.java index d391407cb..3830394bc 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestDownload.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestDownload.java @@ -250,6 +250,32 @@ void shouldErrorWhenSavingAfterDeletion() throws IOException { page.close(); } + @Test + void shouldNotBreakMessageDispatchWhenListenerThrows() throws IOException { + // Reproduces issue #1952: a listener throwing during the message pump of + // an API call (e.g. download.path() or page.close()) would propagate up + // through Connection.dispatch and break the unrelated API call. + Page page = browser.newPage(new Browser.NewPageOptions().setAcceptDownloads(true)); + page.setContent("download"); + Download download = page.waitForDownload(() -> page.click("a")); + // Verify the download path is accessible before the listener is wired up. + Path path = download.path(); + assertTrue(Files.exists(path)); + byte[] bytes = readAllBytes(path); + assertEquals("Hello world", new String(bytes, UTF_8)); + + // Register a close listener that throws, mimicking a listener that calls + // page.waitForLoadState() on a closing page — which throws TargetClosedError + // (see the stack trace in issue #1952). + page.onClose(p -> { + throw new RuntimeException("listener threw during message dispatch"); + }); + // page.close() pumps messages; the close listener fires and throws. + // Before the fix in ListenerCollection.notify, this exception would + // propagate up and break page.close(). + assertDoesNotThrow(() -> page.close()); + } + void shouldErrorWhenSavingAfterDeletionWhenConnectedRemotely() { // TODO: Support connect } diff --git a/playwright/src/test/java/com/microsoft/playwright/impl/TestListenerCollection.java b/playwright/src/test/java/com/microsoft/playwright/impl/TestListenerCollection.java new file mode 100644 index 000000000..9fe9b0120 --- /dev/null +++ b/playwright/src/test/java/com/microsoft/playwright/impl/TestListenerCollection.java @@ -0,0 +1,100 @@ +/* + * Copyright (c) Microsoft Corporation. + * + * 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.microsoft.playwright.impl; + +import org.junit.jupiter.api.Test; + +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class TestListenerCollection { + @Test + void shouldContinueNotifyingRemainingListenersWhenOneThrows() { + ListenerCollection collection = new ListenerCollection<>(); + List received = new ArrayList<>(); + + collection.add("event", (Consumer) received::add); + collection.add("event", (Consumer) v -> { + throw new RuntimeException("listener boom"); + }); + collection.add("event", (Consumer) received::add); + + // Suppress stderr so the expected listener error doesn't pollute test output. + PrintStream prevErr = System.err; + System.setErr(new PrintStream(new java.io.ByteArrayOutputStream())); + try { + assertDoesNotThrow(() -> collection.notify("event", "hello")); + } finally { + System.setErr(prevErr); + } + + // Both the listener before and after the throwing one must be called. + assertEquals(2, received.size()); + assertEquals("hello", received.get(0)); + assertEquals("hello", received.get(1)); + } + + @Test + void shouldNotThrowWhenSingleListenerThrows() { + ListenerCollection collection = new ListenerCollection<>(); + collection.add("event", (Consumer) v -> { + throw new RuntimeException("listener boom"); + }); + + PrintStream prevErr = System.err; + System.setErr(new PrintStream(new java.io.ByteArrayOutputStream())); + try { + assertDoesNotThrow(() -> collection.notify("event", "hello")); + } finally { + System.setErr(prevErr); + } + } + + @Test + void shouldNotThrowWhenNoListeners() { + ListenerCollection collection = new ListenerCollection<>(); + assertDoesNotThrow(() -> collection.notify("event", "hello")); + } + + @Test + void shouldPrintListenerExceptionToStderrForObservability() { + // Mirrors the JS client where async listener rejections surface as + // unhandled rejections on stderr by default. + ListenerCollection collection = new ListenerCollection<>(); + collection.add("event", (Consumer) v -> { + throw new RuntimeException("listener boom"); + }); + + java.io.ByteArrayOutputStream captured = new java.io.ByteArrayOutputStream(); + PrintStream prevErr = System.err; + System.setErr(new PrintStream(captured)); + try { + collection.notify("event", "hello"); + } finally { + System.setErr(prevErr); + } + String stderr = captured.toString(); + assertTrue(stderr.contains("[playwright]") && stderr.contains("listener boom"), + "stderr should contain the listener exception, got: " + stderr); + } +}