"https" and "http" deno compatibility shims do not support WebSocket connections

[zookatron]

If you run this code sample with NodeJS v18.12.1, it will output "upgraded":

import https from "https";

const req = https.request({
  hostname: "socketsbay.com",
  port: 443,
  path: "/wss/v2/1/demo/",
  method: "GET",
  headers: {
    "Sec-WebSocket-Version": 13,
    "Sec-WebSocket-Key": "xqaVaSr2HxSeH/R2p2J2Hw==",
    Connection: "Upgrade",
    Upgrade: "websocket",
  },
});

req.on("response", () => console.log("not upgraded"));
req.on("upgrade", () => console.log("upgraded"));

req.end();

If you run this equivalent code sample with Deno v1.29.3, it will output "not upgraded":

import https from "https://deno.land/std@0.172.0/node/https.ts";

const req = https.request({
  hostname: "socketsbay.com",
  port: 443,
  path: "/wss/v2/1/demo/",
  method: "GET",
  headers: {
    "Sec-WebSocket-Version": 13,
    "Sec-WebSocket-Key": "xqaVaSr2HxSeH/R2p2J2Hw==",
    Connection: "Upgrade",
    Upgrade: "websocket",
  },
});

req.on("response", () => console.log("not upgraded"));
req.on("upgrade", () => console.log("upgraded"));

req.end();

This looks like it is due to the fact that Deno NodeJS compatibility shims for the http and https NodeJS built-in packages always use the "fetch" method which doesn't support websocket upgrades: https://deno.land/std@0.172.0/node/http.ts?source#L137

It seems like this should be fixable without a huge amount of work just by modifying the _final function in ClientRequest to look at the headers that will be used for the request and have it use the WebSocket class instead of fetch if the necessary WebSocket headers like Upgrade: websocket are set. Let me know if this is something you would accept contributions on, this is something I could potentially help out with if this approach seems acceptable.

This seems to be the only blocker preventing the popular discord.js library from working with Deno NodeJS compatibility mode, as discussed here: #17391

[bartlomieju]

Thanks for the report, transferring to deno_std where this needs to be fixed.

[kt3k]

currently http(s).request uses fetch under the hood, and fetch doesn't have ability to handle upgrade event.

Maybe we need to switch the client to WebSocket by sniffing the contents of option passed to http(s).request

[kt3k]

Or maybe we can extend fetch by adding new option to Deno.HttpClient and support upgrade event in fetch in our own way.

[kt3k]

related: #17847

[uwuLegacy]

hello, was wondering if this is still active. also, shouldn't this be on #17841?

[kt3k]

@uwuLegacy Good point. Added

[evantill]

Trying to migrate a bot using discord.js to Deno , I am facing the same problem. Is there any workaround ?

[zookatron]

Trying to migrate a bot using discord.js to Deno , I am facing the same problem. Is there any workaround ?

In versions of Deno before 1.31 it was possible to work around this bug by using an import map to override the https://deno.land/std/node/https.ts file but this no longer works after the Node compatibility layer was moved into the runtime ☹️ https://deno.com/blog/v1.31#compatibility-layer-is-now-part-of-the-runtime