Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main Request service is throwing errors for valid requests #155204

Closed
sandy081 opened this issue Jul 14, 2022 · 12 comments · Fixed by #167075
Closed

Main Request service is throwing errors for valid requests #155204

sandy081 opened this issue Jul 14, 2022 · 12 comments · Fixed by #167075
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders network Network related issues on-testplan sandbox Running VSCode in a node-free environment
Milestone

Comments

@sandy081
Copy link
Member

sandy081 commented Jul 14, 2022

When requests from shared process are redirected to main, the main request service is throwing errors for some valid requests.

[2022-07-14 17:09:57.274] [main] [trace] RequestService#request (node) - error https://marketplace.visualstudio.com/_apis/public/gallery/extensionquery {}

[error] ["Error: net::ERR_INVALID_ARGUMENT"," at SimpleURLLoaderWrapper.<anonymous> (node:electron/js2c/browser_init:101:7169)"," at SimpleURLLoaderWrapper.emit (node:events:390:28)"]

The same request is working when using browser XHR in shared process.

To reproduce:

  • Start VS Code using fresh user data dir and extensions dir
  • Enable this setting developer.sharedProcess.redirectRequestsToMain - this is an internal developer setting, so you have to set manually in the settings.json file
  • Turn on settings sync using your insiders settings sync service (Disable syncing settings)
    🐛 Extensions Syncing fails because of above error

cc @bpasero

@bpasero bpasero added the sandbox Running VSCode in a node-free environment label Jul 14, 2022
@deepak1556 deepak1556 added bug Issue identified by VS Code Team member as probable bug network Network related issues labels Jul 14, 2022
@bpasero
Copy link
Member

bpasero commented Jul 14, 2022

What is the exact request URL and headers? We should be able to reproduce in a Electron fiddle maybe.

@deepak1556
Copy link
Collaborator

net::ERROR_INVALID_ARGUMENT is an error from the chromium network stack, it happens when the request contains one of the following forbidden headers https://source.chromium.org/chromium/chromium/src/+/master:services/network/public/cpp/header_util.cc;l=22-48

@sandy081
Copy link
Member Author

Here is the request info

URL: https://marketplace.visualstudio.com/_apis/public/gallery/extensionquery

Headers:

:authority: marketplace.visualstudio.com
:method: POST
:path: /_apis/public/gallery/extensionquery
:scheme: https
accept: application/json;api-version=3.0-preview.1
accept-encoding: gzip, deflate, br
accept-language: en-GB
content-length: 274
content-type: application/json
origin: vscode-file://vscode-app
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: cross-site
user-agent: VSCode 1.70.0 (Code - OSS Dev)
vscode-sessionid: 82585f7a-3ae5-491b-8db0-abfff4dea0ac1657812359083
x-market-client-id: VSCode 1.70.0
x-market-user-id: 901cfca6-8f3f-40f3-a0ee-cc4aaf37b1b8

Payload:

{"filters":[{"criteria":[{"filterType":7,"value":"undefined_publisher.msteams-liveshare-integration"},{"filterType":8,"value":"Microsoft.VisualStudio.Code"},{"filterType":12,"value":"4096"}],"pageNumber":1,"pageSize":1,"sortBy":0,"sortOrder":0}],"assetTypes":[],"flags":950}

@deepak1556
Copy link
Collaborator

Are we setting the content-length header ? The runtime will add that during payload upload, but if we override it then it will cause the above error.

@sandy081
Copy link
Member Author

Yes, we are here

@sandy081
Copy link
Member Author

BTW we do the same request also in the renderer for querying extensions, so does removing the content-length impacts the request in the renderer that uses XHR ?

@sandy081
Copy link
Member Author

Just tried removing the content-length header and I am getting following error

[error] ["Error: incorrect header check","    at Zlib.zlibOnError [as onerror] (node:zlib:190:17)"]

@deepak1556
Copy link
Collaborator

BTW we do the same request also in the renderer for querying extensions, so does removing the content-length impacts the request in the renderer that uses XHR ?

The header will get added by the runtime for payload requests, you should be able to confirm after removing the header. So, I don't think it will impact requests from renderer via XHR

Just tried removing the content-length header and I am getting following error

Hmm, why is Node.js zlib module being involved ? Are we using Node.js request API by any chance ?

@sandy081
Copy link
Member Author

We are not using Node.js request. Request service in main extends Node request service and pass electron net request API to Node request service for making request. See here:

function getRawRequest(options: IRequestOptions): IRawRequestFunction {
return net.request as any as IRawRequestFunction;
}
export class RequestMainService extends NodeRequestService {
override request(options: IRequestOptions, token: CancellationToken): Promise<IRequestContext> {
return super.request({ ...(options || {}), getRawRequest }, token);
}
}

While handling the response, Node request service is checking for content-encoding header and using zlib if it is set to gzip. See here:

if (res.headers['content-encoding'] === 'gzip') {
stream = res.pipe(createGunzip());
}

May be main request service shall not use node request service at all?

@lramos15
Copy link
Member

Possibly an easier repro that was discovered by @IanMatthewHuff is calling the 'workbench.extensions.installExtension' command also fails with the same network error. You should be able to do this via an extension

@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Nov 24, 2022
@bpasero
Copy link
Member

bpasero commented Nov 24, 2022

@sandy081 Can we now also make sure to go back to the Electron solution for maybe insiders to validate it works over some time?

@sandy081
Copy link
Member Author

Enabled - #167190

@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Nov 25, 2022
@sandy081 sandy081 removed the bug Issue identified by VS Code Team member as probable bug label Nov 28, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders network Network related issues on-testplan sandbox Running VSCode in a node-free environment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants