Use electron.net in the github-authentication extension

[devm33]

With #192899 and #206822 extensions have the ability to use electron.net.fetch to benefit from improved proxy and networking capabilities offered by Electron and Chromium.

For github-authentication, this could replace the use of node-fetch.

[TylerLeonhardt]

@deepak1556 @alexdima @chrmarti do either of you know what work is involved for me to adopt this?

[deepak1556]

Fetch API in Node.js and Electron share the same API design, so you should be able to just swap in the imports and everything should just work. The minimal changes needed would be

import net from 'electron';

export const fetching = net.fetch;

However, I would recommend to try this out as experiment similar to how copilot extension does it, refs https://github.com/microsoft/vscode-copilot/pull/3461

[devm33]

@TylerLeonhardt I noticed #229202 going in and just wanted make sure github-authentication doesn't get left behind 😄 I don't think it will pick up #228476 currently since it's using node-fetch

vscode/extensions/github-authentication/src/node/fetch.ts

Lines 5 to 7 in f26394d

	import fetch from 'node-fetch';
	export const fetching = fetch;

[chrmarti]

Note that I will change the default for #229202 to be off for the stable release. Since this affects all extensions using fetch we first want to test it more in next milestone's Insiders.

[TylerLeonhardt]

@devm33 do you have any way of confirming that this is now working for folks in Insiders?

[vs-code-engineering]

This bug has been fixed in the latest release of VS Code Insiders!

@devm33, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version a1fc8c1 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

[devm33]

@devm33 do you have any way of confirming that this is now working for folks in Insiders?

@TylerLeonhardt I think I'm seeing an issue testing on latest insiders:

Version: 1.97.0-insider (Universal)
Commit: a1fc8c144985285527fcceb7adfa5f66b6bb5399
Date: 2025-01-27T05:04:55.863Z
Electron: 32.2.7
ElectronBuildId: 10660205
Chromium: 128.0.6613.186
Node.js: 20.18.1
V8: 12.8.374.38-electron.0
OS: Darwin arm64 23.6.0

2025-01-28 02:42:27.506 [info] Got 0 sessions for user:email...
2025-01-28 02:43:35.995 [info] Logging in with 'any' account...
2025-01-28 02:43:35.995 [info] Logging in for the following scopes: user:email
2025-01-28 02:43:36.004 [info] Trying without local server... (user:email)
2025-01-28 02:43:39.911 [info] Exchanging code for token...
2025-01-28 02:43:39.930 [error] fetch failed
2025-01-28 02:44:11.828 [info] Trying with local server... (user:email)
2025-01-28 02:44:20.666 [info] Exchanging code for token...
2025-01-28 02:44:20.699 [error] fetch failed
2025-01-28 02:47:27.204 [info] Logging in with 'any' account...
2025-01-28 02:47:27.204 [info] Logging in for the following scopes: user:email
2025-01-28 02:47:27.207 [info] Trying without local server... (user:email)
2025-01-28 02:47:32.387 [info] Exchanging code for token...
2025-01-28 02:47:32.408 [error] fetch failed

Is there a way to get more verbose logging or see if any error was thrown?

It looks like it's failing on this fetch:

vscode/extensions/github-authentication/src/flows.ts

Line 103 in 7a3d738

const result = await fetching(endpointUri.toString(true), {

But I don't see where the [error] fetch failed is coming from in the extension?

[chrmarti]

I have seen fetch failed as the only error message with Node.js' fetch. Could you check the output of F1 > Developer: GitHub Copilot Chat Diagnostics? Are you using a proxy? Or a Remote Development extension?

[devm33]

@chrmarti Yes that appears to be spot on

GitHub Copilot Chat Diagnostics output

GitHub Copilot Chat

• Extension Version: 0.24.2025012802 (prod)
• VS Code: vscode/1.97.0-insider
• OS: Mac

Network

User Settings:

  "http.proxy": "http://localhost:9999",
  "github.copilot.advanced.debug.useElectronFetcher": true,
  "github.copilot.advanced.debug.useNodeFetcher": false,
  "github.copilot.advanced.debug.useNodeFetchFetcher": true

Connecting to https://api.github.com:

• DNS ipv4 Lookup: 140.82.114.5 (11 ms)
• DNS ipv6 Lookup: ::ffff:140.82.114.5 (5 ms)
• Proxy URL: http://localhost:9999 (0 ms)
• Proxy Connection: 407 Proxy Authentication Required
  proxy-authenticate: Basic realm="mitmproxy" (34 ms)
• Electron fetch (configured): HTTP 200 (82 ms)
• Node.js https: HTTP 200 (80 ms)
• Node.js fetch: Error (23 ms): fetch failed
  Request was cancelled.
  Proxy response (407) !== 200 when HTTP Tunneling
• Helix fetch: Error (174 ms): Miscellaneous failure (see text): no credential for 1221457B-73E4-463F-9A4A-B270AD5C8FF3

Connecting to https://api.githubcopilot.com/_ping:

• DNS ipv4 Lookup: 140.82.114.22 (27 ms)
• DNS ipv6 Lookup: ::ffff:140.82.114.22 (2 ms)
• Proxy URL: http://localhost:9999 (1 ms)
• Proxy Connection: 407 Proxy Authentication Required
  proxy-authenticate: Basic realm="mitmproxy" (31 ms)
• Electron fetch (configured): HTTP 200 (74 ms)
• Node.js https: HTTP 200 (88 ms)
• Node.js fetch: Error (24 ms): fetch failed
  Request was cancelled.
  Proxy response (407) !== 200 when HTTP Tunneling
• Helix fetch: Error (12 ms): Miscellaneous failure (see text): no credential for 1221457B-73E4-463F-9A4A-B270AD5C8FF3 (negative cache)

Documentation

In corporate networks: Troubleshooting firewall settings for GitHub Copilot.

Running a proxy, but not in wsl or any vs code remote, so I'm not sure why electron wouldn't be enabled, per: https://github.com/devm33/vscode/blob/419205a8ff1252fc6a3a8e085b8645f2ab51a0ec/extensions/github-authentication/src/node/fetch.ts#L6-L12

The github-authentication extension runs in the standard extension host right?

[TylerLeonhardt]

The github-authentication extension runs in the standard extension host right?

Yes, nothing special there. It runs on the client's extension host

[devm33]

😕 Ok I'm lost on the difference here then:

vscode/extensions/microsoft-authentication/src/node/fetch.ts

Lines 6 to 12 in 724d644

	let _fetch: typeof fetch;
	try {
	_fetch = require('electron').net.fetch;
	} catch {
	_fetch = fetch;
	}
	export default _fetch;

vscode/extensions/github-authentication/src/node/fetch.ts

Lines 6 to 12 in 724d644

	let _fetch: typeof fetch;
	try {
	_fetch = require('electron').net.fetch;
	} catch {
	_fetch = fetch;
	}
	export const fetching = _fetch;

It's also confusing the vscode-proxy-agent would fail for the node based fetch for github-authentication. Other requests in the extension host are working fine

[chrmarti]

I'll open a new issue for Node.js' fetch failing. Not sure why the above code snippets wouldn't do the same, maybe something with module loading? Best to step through with the debugger.

[devm33]

Should 6263fff be in the latest insiders? Looking at /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/github-authentication/dist/extension.js I don't see any require("electron")

Is the github-authentication extension published from the same commit as the main app?

Version: 1.97.0-insider (Universal)
Commit: a1fc8c1
Date: 2025-01-27T05:04:55.863Z
Electron: 32.2.7
ElectronBuildId: 10660205
Chromium: 128.0.6613.186
Node.js: 20.18.1
V8: 12.8.374.38-electron.0
OS: Darwin arm64 23.6.0

[TylerLeonhardt]

It's there I'm pretty sure:

"use strict";let r;Object.defineProperty(t,"__esModule",{value:!0}),t.fetching=void 0;try{r=n(2556).net.fetch}catch{r=fetch}t.fetching=r}

n(2556) probably maps to require('electron')

also commits are almost always in Insiders in the next business day, for context.

[devm33]

Thanks @TylerLeonhardt I think I see the issue. In the bundled output, 2556 is mapped to:

2556: (e, t, n) => {
        const r = n(9896),
          i = n(6928),
          o = i.join(__dirname, "path.txt");
        e.exports = (function () {
          let e;
          if (
            (r.existsSync(o) && (e = r.readFileSync(o, "utf-8")),
            process.env.ELECTRON_OVERRIDE_DIST_PATH)
          )
            return i.join(
              process.env.ELECTRON_OVERRIDE_DIST_PATH,
              e || "electron"
            );
          if (e) return i.join(__dirname, "dist", e);
          throw new Error(
            "Electron failed to install correctly, please delete node_modules/electron and try installing again"
          );
        })();

which is the https://www.npmjs.com/package/electron package. I think we need to add electron to the webpack config externals.

Though notably that's not what extensions/microsoft-authentication/extension.webpack.config.js has so I'm not sure how (if?) that's working.

[TylerLeonhardt]

Keep in mind, Microsoft Auth has a new implementation that doesn't use 'electron' unless you explicitly ask for the classic version of the extension. In my testing, this new implementation provided by MSAL-node was quite thorough with proxy stuff.

[TylerLeonhardt]

there are only 2 references of this require... so maybe we just add electron here:

vscode/extensions/shared.webpack.config.js

Lines 58 to 65 in bda07cf

	externals: {
	'vscode': 'commonjs vscode', // ignored because it doesn't exist,
	'applicationinsights-native-metrics': 'commonjs applicationinsights-native-metrics', // ignored because we don't ship native module
	'@azure/functions-core': 'commonjs azure/functions-core', // optioinal dependency of appinsights that we don't use
	'@opentelemetry/tracing': 'commonjs @opentelemetry/tracing', // ignored because we don't ship this module
	'@opentelemetry/instrumentation': 'commonjs @opentelemetry/instrumentation', // ignored because we don't ship this module
	'@azure/opentelemetry-instrumentation-azure-sdk': 'commonjs @azure/opentelemetry-instrumentation-azure-sdk', // ignored because we don't ship this module
	},

that way future stuff (if so) doesn't fall for the same trap.

[devm33]

Keep in mind, Microsoft Auth has a new implementation that doesn't use 'electron' unless you explicitly ask for the classic version of the extension. In my testing, this new implementation provided by MSAL-node was quite thorough with proxy stuff.

Oh that's interesting will have to take a look at what they're doing.

Here's PR for the electron change #239134

Edit: I'll update to move it to the defaults

[devm33]

Updated c4b0c17

[TylerLeonhardt]

Fixed in #239134