Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,26 @@ class ListenerCollection <EventType> {
this.channelOwner = channelOwner;
}

@SuppressWarnings("unchecked")
<T> void notify(EventType eventType, T param) {
List<Consumer<?>> list = listeners.get(eventType);
if (list == null) {
return;
}

for (Consumer<?> listener: new ArrayList<>(list)) {
((Consumer<T>) listener).accept(param);
try {
((Consumer<T>) 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());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("<a href='" + server.PREFIX + "/download'>download</a>");
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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> collection = new ListenerCollection<>();
List<String> received = new ArrayList<>();

collection.add("event", (Consumer<String>) received::add);
collection.add("event", (Consumer<String>) v -> {
throw new RuntimeException("listener boom");
});
collection.add("event", (Consumer<String>) 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<String> collection = new ListenerCollection<>();
collection.add("event", (Consumer<String>) 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<String> 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<String> collection = new ListenerCollection<>();
collection.add("event", (Consumer<String>) 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);
}
}