fix: review round 2 — per-browser UA cache, leak fixes, docs#2
Merged
Conversation
Review fixes: - CRITICAL: Move UA cache from global package var to per-browser sync.Once on browserImpl. Fixes incorrect cache sharing across browsers with different versions/channels. Eliminates global mutex contention during 50+ browser cold starts. - HIGH: Fix context leak in NewStealthPage when NewPage fails (add context.Close on error path) - HIGH: Guard type assertion in getPatchedUA with comma-ok to prevent panic on unexpected navigator.userAgent type - HIGH: Log ctx.Close errors in getPatchedUA instead of discarding - HIGH: Reset routeInjecting atomic.Bool on Route failure in both page.go and browser_context.go so retries aren't silently skipped Docs: - README: Add stealth API section, update quick start to use NewStealthPage, add Chrome channel docs, update credits - CLAUDE.md: Add stealth.go and new files to file structure, document patchrightInitScript and stealth API - LLM guide: Update concurrent browsers example to use NewStealthPage - Example: Update basic example to use NewStealthPage
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.
Summary
sync.Once— fixes incorrect cache sharing across browsers with different versions/channels, eliminates global mutex contention at scaleNewStealthPagewhenNewPage()failsgetPatchedUArouteInjectingflag reset onRoute()failure so retries workReview findings addressed
patchedUACachewrong for multi-browser fleetssync.OnceonbrowserImplNewStealthPageleaks context onNewPagefailurecontext.Close()on error pathgetPatchedUActx.Close()errors silently discardedrouteInjectingstuck onRoute()failurefalseon errorTest plan
go build ./...andgo vet ./...clean