[Alok Pandey (alok-108)]

Fixes #3293

Problem

When an unhandled exception is raised in an event listener (e.g. page.on("response", ...)), Connection._on_event_listener_error stores the exception in self._error to be raised on the next API call.

However, Connection.send_message_to_server() previously checked:

if self._error and method != "__waitInfo__":
    error = self._error
    self._error = None
    raise error

When user code runs cleanup in finally: or via context manager __exit__, calling browser.close() or context.close() invokes send_message_to_server("close"). Because "close" and "dispose" were not exempted, the method aborted before transmitting the message to the driver. As a result, the driver never received the close command, and browser child processes were leaked in the background (browser.is_connected() == True).

Solution

1. In Connection.send_message_to_server(), exempt "close" and "dispose" from pre-dispatch abortion so that teardown and disposal commands are actually sent to the driver.
2. In Connection.dispatch(), when the server responds to "close", if self._error was set, reject the close callback with that error. This delivers the exception to the caller while guaranteeing that the remote browser/page was cleanly terminated on the server beforehand.
3. Added a regression test in tests/common/test_events.py verifying that browser.close() terminates the browser (assert not browser.is_connected()) even when an event listener threw an error.

[Alok Pandey (alok-108)]

Pavel Feldman (@pavelfeldman) could you please take a look when you have a moment? Thank you!

[Dmitry Gozman (dgozman)]

I don't think that special-casing close is the right solution.