[playwright-browser-tunnel] Fix stopAsync hanging while waiting for a connection - #5935
Open
Pham Manh Luc (MLuc24) wants to merge 1 commit into
Open
Conversation
… connection In poll-connection mode the init promise only settles once a client connects. stopAsync() cleared the polling interval but still awaited that promise, so stopping the tunnel before any client arrived never completed. The pending wait is now settled as part of the teardown, and a stop during the wait is treated as an ordinary shutdown by the start loop rather than an error.
Comment on lines
+90
to
+93
| /** | ||
| * Thrown internally to settle a connection wait that was still pending when the tunnel was stopped. | ||
| */ | ||
| class TunnelStoppedError extends Error { |
Comment on lines
+311
to
+317
| this._cancelPollConnection = (error: Error): void => { | ||
| if (this._pollInterval) { | ||
| clearInterval(this._pollInterval); | ||
| this._pollInterval = undefined; | ||
| } | ||
| this._pendingConnectionAttempt = undefined; | ||
| reject(error); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #5853
PlaywrightTunnel.stopAsync()never completes when the tunnel is stopped inpoll-connectionmode before any client has connected. The tunnel is left inwaiting-for-connectionand the caller waits forever.Details
_pollConnectionAsync()returns a promise that is only resolved from inside the polling interval, when_tryConnectAsync()finally succeeds — itsrejectis never called.startAsync()stores that promise as_initWsPromise, andstopAsync()clears the interval and then awaits it. Clearing the interval stops the polling but leaves the promise pending, so the await has nothing to wait for.The fix records a canceller while the poll is in flight and settles it during teardown:
_pollConnectionAsync()registers_cancelPollConnection, which clears the poll state and rejects the pending wait; it is dropped as soon as a client connects.stopAsync()invokes it before awaiting_initWsPromise, swallows that expected rejection, clears_initWsPromise, and sets the status tostopped.startAsync()loop treats such a cancellation as an ordinary shutdown, so stopping the tunnel does not surface as a failure to whoever awaitedstartAsync().This is the case JM (@justinTM)'s #5851 explicitly does not cover — that PR fixes the Start/Stop race in the VS Code extension, while this one is in the tunnel itself, so the two do not touch the same files.
How it was tested
Against the published
@rushstack/playwright-browser-tunnel0.3.27, driving a tunnel at an endpoint where nothing is listening, so it stays inwaiting-for-connection:With this change applied to the package's compiled output, the same script:
The package currently has no unit tests, so I did not add one rather than introduce a test harness in a bug-fix PR — glad to add one if you would like it here. I also could not compile the monorepo locally, so that is worth confirming in CI.