fix: throw pretty error if opening userDataDir twice #36007
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mxschmitt
force-pushed
the
fix-throw-pretty-error-userDataDir
branch
from
79f0638 to
a2231b6
Compare
yury-s
reviewed
May 20, 2025
| }); | ||
| }); | ||
|
|
||
| test('should throw when connecting twice to an already running persistent context (--remote-debugging-port)', async ({ browserType, channel, createUserDataDir }) => { |
Member
There was a problem hiding this comment.
Why can't we connect to the same instance twice without trying to recreate the user-data-dir?
This comment has been minimized.
This comment has been minimized.
| const wsEndpoint = (await readyState?.waitUntilReady())?.wsEndpoint; | ||
| if (options.cdpPort !== undefined || !this.supportsPipeTransport()) { | ||
| transport = await WebSocketTransport.connect(progress, wsEndpoint!); | ||
| if (!wsEndpoint) |
Member
There was a problem hiding this comment.
I don't think this is related to wsEndpoint. We should rather throw if the browser is launched with the same profile more than once. Saying "Browser with the given profile is already running".
Contributor
Author
There was a problem hiding this comment.
if the browser is launched with the same profile more than once
This is written to the stderr by the browser but only for Chrome/Edge. Normal Chromium allows opening it multiple times. The error I throw contains this message as well, since the stderr is wrapped inside it.
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
fix-throw-pretty-error-userDataDir
branch
from
d10ace0 to
f0f3592
Compare
mxschmitt
force-pushed
the
fix-throw-pretty-error-userDataDir
branch
from
f0f3592 to
496faf7
Compare
This comment has been minimized.
This comment has been minimized.
pavelfeldman
approved these changes
May 27, 2025
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
fix-throw-pretty-error-userDataDir
branch
from
eb25825 to
77afbd1
Compare
Contributor
Test results for "tests 1"3 failed 8 flaky39246 passed, 827 skipped Merge workflow run. |
Contributor
Test results for "tests others"18 failed 5 flaky22003 passed, 527 skipped Merge workflow run. |
Contributor
Test results for "tests 2"20 failed 183 flaky239978 passed, 9434 skipped Merge workflow run. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What happened?
In the scenario of that a persistent browser profile is already used, the browser is directly exiting with an error message to stderr. This only happens with Chrome/Edge. Not with normal Chromium.
When that happens, we resolve the readyState to undefined. This will then resolve the wsEndpoint and try to connect to it even tho its
undefined.Relates microsoft/playwright-mcp#366 (does not fix it but throws a better error now)