feat: support adding extra headers to mcp for each request#692
feat: support adding extra headers to mcp for each request#692clChenLiang wants to merge 2 commits intoChromeDevTools:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
cc1113f to
49be28e
Compare
|
can @OrKoN @Lightning00Blade to see this pr ? |
Lightning00Blade
left a comment
There was a problem hiding this comment.
Thank you for your PR, we will discuss if this is something we want to include.
It would be nice to open an issue so other people can find it easier if they want the same feature.
In the mean while I left some comments for the PR itself.
| this.#headers = options.headers; | ||
|
|
||
| this.#networkCollector = new NetworkCollector(this.browser); | ||
| this.#networkCollector = new NetworkCollector(this.browser, undefined, { |
There was a problem hiding this comment.
This should not happen in the NetworkCollector. Collectors are there to get "collect" data from the pages as things happen. The code should live inside the McpContext class.
|
|
||
| // If we reach here without errors, headers functionality is working | ||
| assert.ok(true); | ||
| }, { debug: false }); |
There was a problem hiding this comment.
| }, { debug: false }); | |
| }); |
| const navigationPromise = page.goto('data:text/html,<html><body>Test</body></html>'); | ||
| await navigationPromise; |
There was a problem hiding this comment.
| const navigationPromise = page.goto('data:text/html,<html><body>Test</body></html>'); | |
| await navigationPromise; | |
| await page.goto('data:text/html,<html><body>Test</body></html>'); |
| }); | ||
|
|
||
| describe('McpContext headers functionality', () => { | ||
| it('works with headers in context options', async () => { |
There was a problem hiding this comment.
This test seems to test nothing.
We don't set the headers and we don't check that the headers are properly send on the response attached to the page navigation.
In test scenarios,
the test environment of the production domain is typically used, which is usually distinguished by headers.
This merge request supports adding headers parameters when accessing the test environment of a project.