Repository navigation
fix(python): preserve query parameters in CDP discovery URLs - #3221
Draft
Kayvan-Zahiri wants to merge 2 commits into
Draft
Kayvan-Zahiri wants to merge 2 commits into
Kayvan-Zahiri wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: c92515b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
Keep the query-preserving discovery URL and pass the new CDP headers through.
This branch has not been deployed
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.
An HTTP CDP endpoint such as
https://proxy.test/browser/?token=demois currently probed ashttps://proxy.test/browser/?token=demo/json/version. The discovery suffix becomes part of the query value, so the resolver polls the wrong URL. A fragment can similarly swallow the suffix.Build the discovery URL from parsed URL components: append
/json/versionto the path, preserve the original query (including percent encoding), and omit the fragment. Directws://andwss://endpoints continue to pass through unchanged.Added eight regression/control cases. Four query/fragment cases fail before the fix; four plain-HTTP/WebSocket controls already pass. All 62 CDP client tests pass with the fix. Python Ruff lint/format, ty, and git diff --check pass. Tests use a mocked discovery reader and fake sockets; no browser or external service was used. A Python-only patch changeset is included; no protocol/generated code changes.
Developed with AI assistance; reproduced the malformed discovery URLs locally and reviewed the patch and tests. The TypeScript resolver has similar string concatenation; this PR is scoped to the Python SDK.
Full Python suite: 794 passed, 1 skipped, 1 failed because the checkout lacks the built extension manifest. The same test fails on the unmodified base; it is unrelated to this patch. Cross-language
just check/just testwere not run.Summary by cubic
Fixes CDP discovery URL construction so HTTP endpoints with a query string or fragment are probed correctly instead of appending
/json/versionto the query value, and passes the new CDP headers through the discovery request./json/versionto the path, preserving the query (including percent encoding), and dropping the fragment.ws://andwss://endpoints pass through unchanged.Written for commit c92515b. Summary will update on new commits.