fix: VS Code plugin crash when debugging/running on Windows (#8851)

[LightQuanta]

What does this PR do?

Try to fix #8851 by modifying the way ws+unix:// URLs are parsed on Windows, and fixed build and test script issue causing VS Code plugin crash on Windows.

How did you verify your code works?

Tried on VS Code, and the URL parsing related issues has been partly resolved (URLs for socks can now be parsed correctly), but I cannot build bun for windows successfully, so I cannot test changes in src/js/internal/debugger.ts. Could someone assit with this?

[github-actions]

❌ @Jarred-Sumner, your commit has failing tests :(

💪 2 failing tests Darwin AARCH64

• test/cli/install/registry/bun-install-registry.test.ts 1 failing
• test/js/web/fetch/fetch.stream.test.ts 1 failing

💻 2 failing tests Darwin x64 baseline

• test/cli/install/bun-create.test.ts 1 failing
• test/js/web/workers/worker.test.ts 1 failing

🐧💪 1 failing tests Linux AARCH64

• test/js/bun/http/serve-body-leak.test.ts 1 failing

🐧🖥 1 failing tests Linux x64 baseline

• test/js/node/child_process/child_process-node.test.js 1 failing

🐧🖥 1 failing tests Linux x64

• test/js/node/child_process/child_process-node.test.js 1 failing

🪟💻 6 failing tests Windows x64 baseline

• test/cli/install/bunx.test.ts 1 failing
• test/js/bun/http/serve-body-leak.test.ts 1 failing
• test/js/bun/test/test-test.test.ts 1 failing
• test/js/node/process/process.test.js 2 failing
• test/js/node/tls/node-tls-context.test.ts code 1
• test/js/node/watch/fs.watchFile.test.ts 3 failing

🪟💻 6 failing tests Windows x64

• test/cli/install/bunx.test.ts 1 failing
• test/js/bun/test/test-test.test.ts 1 failing
• test/js/node/process/process.test.js 2 failing
• test/js/node/tls/node-tls-context.test.ts code 1
• test/js/node/watch/fs.watchFile.test.ts 3 failing
• test/js/web/fetch/fetch.tls.test.ts 1 failing

View logs

[Jarred-Sumner]

@Electroid can you take a look at this?

[mikeseese]

@LightQuanta I'm not 100% sure if the Unix socket approach on Windows will work. Using an example script provided by a user in #8851:

test.js example

const net = require("net")
const { userInfo } = require("os");

(async function () {

  let responded = false;

  var server = net.createServer(function (socket) {
      socket.on("data", function (data) {
          let message = data.toString();
          console.log("Message received from client: " + message);
          socket.write("Hello " + message + " !");
          responded = true;
      })
  })
  server.listen("./test.sock");

  let client = net.connect("./test.sock").
      on('data', (data) => {
          let message = data.toString();
          console.log("Message received from server: " + message);
      })

  client.write(userInfo().username);

  while (!responded) {
      await Bun.sleep(10);
  }

  client.destroy();
  server.close()
})();

Running this script in bun works fine:

$ bun test.js
Message received from client: seese
Message received from server: Hello seese !

but running it in node fails:

$ node test.js
node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: listen EACCES: permission denied ./test.sock
    at Server.setupListenHandle [as _listen2] (node:net:1881:21)
    at listenInCluster (node:net:1946:12)
    at Server.listen (node:net:2061:5)
    at D:\work\bun\packages\bun-vscode\test.js:16:10
    at Object.<anonymous> (D:\work\bun\packages\bun-vscode\test.js:32:3)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Module._load (node:internal/modules/cjs/loader:1024:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1925:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EACCES',
  errno: -4092,
  syscall: 'listen',
  address: './test.sock',
  port: -1
}

Node.js v20.14.0

So the extension in VSCode, running with electron's packaged Node, fails to listen for the BUN_INSPECT_NOTIFY. I believe at a minimum the BUN_INSPECT_NOTIFY will need to support a ws protocol, at which point, but I'm looking into options.

Note: I can compile on Windows and did test your changes, but couldn't get to the point of testing the substring(1) question due to the above.

[mikeseese]

It seems that NodeJS listen(path: string) for Windows uses named pipes:

On Windows, the local domain is implemented using a named pipe. The path must refer to an entry in \?\pipe\ or \.\pipe. Any characters are permitted, but the latter may do some processing of pipe names, such as resolving .. sequences. Despite how it might look, the pipe namespace is flat. Pipes will not persist. They are removed when the last reference to them is closed. Unlike Unix domain sockets, Windows will close and remove the pipe when the owning process exits.

I wonder if it'd be better to use WebSockets for the extension instead of Unix Sockets, at least for Windows (which I'd argue that other platforms should also adopt WebSockets for consistency in maintenance).

[nolios]

Why not use standard ws:// for windows instead of ws+unix?
Then update signal.ts to match this change?

I am not familiar with all the project code so this is may not be possible but thought to mention.

[mikeseese]

I believe it's possible! I've started working towards this. There are several more changes necessary (including in the Bun runtime) to support Windows. Someone can ping me if they think it's taking me too long and they want to pick up the effort from me; not sure if it's going to be days or weeks as it's not an immediate need for me anymore.

[nolios]

Great work on starting this @mikeseese! I have mentioned your reply in the active issue. I am in a similar situation as I am tied up with Company related work. I may contribute to/help with this when I have more time

[LightQuanta]

Why not use standard ws:// for windows instead of ws+unix? Then update signal.ts to match this change?

I am not familiar with all the project code so this is may not be possible but thought to mention.

Tried ws:, but seems that it still doesn't work. See #8851

[LightQuanta]

It seems that NodeJS listen(path: string) for Windows uses named pipes:

On Windows, the local domain is implemented using a named pipe. The path must refer to an entry in ?\pipe\ or .\pipe. Any characters are permitted, but the latter may do some processing of pipe names, such as resolving .. sequences. Despite how it might look, the pipe namespace is flat. Pipes will not persist. They are removed when the last reference to them is closed. Unlike Unix domain sockets, Windows will close and remove the pipe when the owning process exits.

I wonder if it'd be better to use WebSockets for the extension instead of Unix Sockets, at least for Windows (which I'd argue that other platforms should also adopt WebSockets for consistency in maintenance).

I've tried named pipe, seems that it do work in node.js, BUT it doesn't work for bun.

Use a modified version of the example script from #8851

const net = require("node:net")
const { userInfo } = require("node:os");

const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));

(async function () {
    let responded = false;
    const server = net.createServer(function (socket) {
        socket.on("data", function (data) {
            let message = data.toString();
            console.log("Message received from client: " + message);
            socket.write("Hello " + message + " !");
            responded = true;
        })
    })
    server.listen("\\\\.\\pipe\\test.sock");

    const client = net.connect("\\\\.\\pipe\\test.sock").
        on('data', (data) => {
            let message = data.toString();
            console.log("Message received from server: " + message);
        })

    client.write(userInfo().username);

    while (!responded) {
        await sleep(10);
    }

    client.destroy();
    server.close()
})();

Running this with node works properly:

node test.js

Message received from client: LightQuanta
Message received from server: Hello LightQuanta !

But bun does not support named pipes:

bun testnode.js

13 |             responded = true;
14 |         })
15 |     })
16 |     server.listen("\\\\.\\pipe\\test.sock");
17 |
18 |     const client = net.connect("\\\\.\\pipe\\test.sock").
                            ^
ENOENT: Failed to connect
   errno: 2
 syscall: "connect"

      at connect (node:net:302:10)
      at C:\path\to\file\test.js:18:24
      at C:\path\to\file\test.js:6:20
      at C:\path\to\file\test.jss:31:18

Bun v1.1.20 (Windows x64)

and vise versa, if you replace "\\\\.\\pipe\\test.sock" with "./test.sock", the script works on bun, but not on node:

bun test.js

Message received from client: Light_Quanta
Message received from server: Hello Light_Quanta !

node test.js

node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: listen EACCES: permission denied ./test.sock
    at Server.setupListenHandle [as _listen2] (node:net:1882:21)
    at listenInCluster (node:net:1961:12)
    at Server.listen (node:net:2080:5)
    at C:\path\to\file\test.js:16:12
    at Object.<anonymous> (C:\path\to\file\test.js:32:3)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Module._load (node:internal/modules/cjs/loader:1024:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1940:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EACCES',
  errno: -4092,
  syscall: 'listen',
  address: './test.sock',
  port: -1
}

Node.js v20.16.0

I think this is unsolvable. Either wait for named pipe support in bun or wait for unix socket support in node on Windows. Maybe you are right, we should migrate to WebSockets before this can be solved.

[ipundit]

This is a guess as I haven't looked into the code, but maybe the clues found in solving a related problem would help:
https://stackoverflow.com/questions/68791319/unix-domain-socket-bind-failed-in-windows

1. Did you try removing unused socket options?
2. Is Michael Chourdakis' 2021 comment still true in 2024?

You are wasting time. Windows has file mapping which is the preferred IPC mechanism. No need to be cross platform for something you would not need to interoperate. Generally, Linux stuff that is presumably portable does not work well on windows and windows developers don't bother with such stuff.

[Electroid]

This was fixed in another PR.