Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Closing the last page in the context inconsistently closes BrowserContext if persistent and not headless #2753

Open
nexus-os opened this issue Feb 24, 2025 · 1 comment

Comments

@nexus-os
Copy link

Version

1.50.0

Steps to reproduce

  1. pip install playwright (tested with 1.46 and 1.50 so far)
  2. playwright install
  3. run this test script
  4. For every combination of [sync, headless, persistent], the test will:
    1. Launch a ${persistent} and ${headless} chromium browser context using the ${sync} playwright API;
    2. Navigate to example.com
    3. Close all pages (for Chromium, this also includes the default new page)
    4. Close the browser context

Expected behavior

The test either prints "All good, commander" or fails with an exception consistently in all cases

Actual behavior

For most cases, the test just succeeds; but for both sync and async APIs, in the cases where headless=False and persistent=True, when the BrowserContext is closed, it throws an exception claiming that the browser has already been closed:

Broken: BrowserContext.close: Target page, context or browser has been closed
Traceback (most recent call last):
  File "/home/[redacted]/[redacted]/playwright_issue/test.py", line 51, in <module>
    asyncio.run(async_main(persistent, headless))
  File "/usr/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/[redacted]/[redacted]/playwright_issue/test.py", line 21, in async_main
    await context.close()
  File "/home/[redacted]/[redacted]/playwright_issue/venv/lib/python3.12/site-packages/playwright/async_api/_generated.py", line 13437, in close
    return mapping.from_maybe_impl(await self._impl_obj.close(reason=reason))
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/[redacted]/[redacted]/playwright_issue/venv/lib/python3.12/site-packages/playwright/_impl/_browser_context.py", line 599, in close
    await self._channel.send("close", {"reason": reason})
  File "/home/[redacted]/[redacted]/playwright_issue/venv/lib/python3.12/site-packages/playwright/_impl/_connection.py", line 61, in send
    return await self._connection.wrap_api_call(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/[redacted]/[redacted]/playwright_issue/venv/lib/python3.12/site-packages/playwright/_impl/_connection.py", line 528, in wrap_api_call
    raise rewrite_error(error, f"{parsed_st['apiName']}: {error}") from None
playwright._impl._errors.TargetClosedError: BrowserContext.close: Target page, context or browser has been closed

Additional context

The expected behaviour here is not well documented. Docs for the close method say that closing the browser will close all pages, but it doesn't say what happens the other way around: if closing all pages should imply closing the browser context, then the test should throw this exception also in all other cases. What I would expect to happen from the docs is that closing all pages has no effect on the browser context, and the browser context can be closed independently from page state.

Environment

- Operating System: Arch Linux
- CPU: x86_64
- Browser: Chromium
- Python Version: 3.12
- Other info: N/A
@mxschmitt
Copy link
Member

For most cases, the test just succeeds;

How often does it not succeed? Maybe you could adjust the reproduction so it has a loop and does it e.g. 1000 times to see if it passes all the time? I tried that but for me it was passing every time.

The expected behaviour here is not well documented. Docs for the close method say that closing the browser will close all pages, but it doesn't say what happens the other way around: if closing all pages should imply closing the browser context, then the test should throw this exception also in all other cases. What I would expect to happen from the docs is that closing all pages has no effect on the browser context, and the browser context can be closed independently from page state.

It does not imply that. You can close all the pages and then create new ones. It sounds like a race for what we need a reliable reproduction for.

Operating System: Arch Linux

Arch Linux is not supported, we don't run our tests on it. Might be related. See here for the supported operating systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants