launch: make browserClose actually close the context and browser (fixes #80) - #81
Open
wittjeff wants to merge 1 commit into
Open
launch: make browserClose actually close the context and browser (fixes #80)#81wittjeff wants to merge 1 commit into
wittjeff wants to merge 1 commit into
Conversation
browserClose referenced page.context and browserContext.browser as properties, but they are methods. page.context therefore yielded a function object, whose absent close method made the close attempt throw a TypeError that the empty catch block discarded, and whose absent browser property made the browser-close branch unreachable. So browserClose silently closed nothing: every browser launched in the main process outlived its act until the process exited, and browsers launched in forked tool children survived until the child exited. Call the methods instead. browserContext.browser() is null for a context not owned by a browser; the existing guard covers that. Verified: after browserClose(page), browser.isConnected() is now false (previously true); single-act and multi-act jobs produce results identical to before the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes #80.
browserClosereferencedpage.contextandbrowserContext.browseras properties, but they are Playwright methods.page.contexttherefore yielded a function object, whose absentclosemethod made the close attempt throw a TypeError that the empty catch block discarded, and whose absentbrowserproperty made the browser-close branch unreachable. SobrowserClosesilently closed nothing: every browser launched in the main process outlived its act until the process exited, and browsers launched in forked tool children survived until the child exited.This PR calls the methods instead.
browserContext.browser()returns null for a context not owned by a browser (e.g. a persistent context, as proposed in #77); the existingif (browser)guard covers that.Verification
browserClose(page),browser.isConnected()is nowfalse(previously remainedtrue).bulkonvalidation/tests/targets/bulk/bad.html) produces totals[1,0,0,0], identical to before the change.🤖 Generated with Claude Code