introduce a remote allocator option: NoModifyURL #1184
Merged
Conversation
This file contains 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
In headful mode, the remote debugging address will always be 127.0.0.1 or ::1, the command line option --remote-debugging-address is ignored. See https://github.com/chromium/chromium/blob/106.0.5249.119/chrome/browser/devtools/remote_debugging_server.cc#L48-L58 In headless mode, the default remote debugging address is "localhost". If --remote-debugging-address is specified, its value should be an IP literal. See https://github.com/chromium/chromium/blob/106.0.5249.119/headless/app/headless_shell.cc#L750-L758 The ExecAllocator reads the remote debugging address from the stdout of the browser. In all the cases listed above, the host of the address can only be "localhost" or an IP. So forceIP is not needed. This is a preparing step for further refactoring.
…or).Allocate Because detectURL could fail. But there is not way to report the failure in NewRemoteAllocator (except for panicking). This is a preparing step for further refactoring.
Changes: 1. make it context aware. 2. make it return an error if it fails to modify the URL. 3. bypass resolution when the host is "localhost" or an IP address. 4. improve comments.
The remote allocator tries to modify the websocket debugger url passed to it. This behavior breaks browserless (https://www.browserless.io/). The option NoModifyURL is introduced to address this issue.
|
@andyMrtnzP (from #990) and @AlexLoyola @joelgriffith @kotchuprik (from #972) Sorry for the belated fix! Would you like to review this PR and give it a try? The commits are better to be reviewed one by one because each commit just focuses on one thing and is relatively small. Thank you! |
|
Works like a charm! |
|
Thank you for the feedback! I'm going to merge this PR. |
This was referenced Oct 31, 2022
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.
The feature implemented by #817 makes chromedp not work with browserless. In order to address this issue, a new remote allocator option is introduced:
chromedp.NoModifyURL. Usage:Fixes #972.