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

adapter-cloudflare doesn't work with ws #12007

Closed
maemigh opened this issue Mar 20, 2024 · 8 comments
Closed

adapter-cloudflare doesn't work with ws #12007

maemigh opened this issue Mar 20, 2024 · 8 comments

Comments

@maemigh
Copy link

maemigh commented Mar 20, 2024

Describe the bug

ws (https://github.com/websockets/ws/) doesn't work when using adapter-cloudflare. ws fails to run because it thinks it's running in the browser. I think this is because adapter-cloudflare is built with platform: browser and the import ends up calling its browser.js containing

'use strict';

module.exports = function () {
  throw new Error(
    'ws does not work in the browser. Browser clients must use the native ' +
      'WebSocket object'
  );
};

Reproduction

When used inside +server.ts

import ws, { createWebSocketStream } from 'ws';

results in the error

Error: ws does not work in the browser. Browser clients must use the native WebSocket object
    at new module.exports (file:///Users/maemigh/arcana-website/node_modules/ws/browser.js:4:9)

Logs

No response

System Info

System:
    OS: macOS 14.3.1
    CPU: (8) arm64 Apple M1
    Memory: 39.66 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.5.0 - /opt/homebrew/bin/node
    npm: 10.2.4 - /opt/homebrew/bin/npm
    pnpm: 8.7.0 - ~/Library/pnpm/pnpm
  Browsers:
    Brave Browser: 122.1.63.174
    Edge: 122.0.2365.92
    Firefox Nightly: 92.0a1
    Safari: 17.3.1
  npmPackages:
    @sveltejs/adapter-auto: ^3.0.0 => 3.1.1
    @sveltejs/adapter-cloudflare: ^4.1.0 => 4.1.0
    @sveltejs/kit: ^2.0.6 => 2.5.0
    @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.0.2
    svelte: ^4.2.8 => 4.2.9
    vite: ^5.0.0 => 5.0.12

Severity

annoyance

Additional Information

			const result = await esbuild.build({
				platform: 'browser',
				conditions: ['worker', 'browser'],
				sourcemap: 'linked',
				target: 'es2022',
				entryPoints: [`${tmp}/_worker.js`],
				outfile: `${dest}/_worker.js`,
				allowOverwrite: true,
				format: 'esm',
				bundle: true,
				loader: {
					'.wasm': 'copy'
				},
				external,
				alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])),
				logLevel: 'silent'
			});
@eltigerchino
Copy link
Member

eltigerchino commented Mar 20, 2024

I think you have to use the workers WebSocket API instead of the node.js ws since many node.js APIs are not available in workers.
https://developers.cloudflare.com/workers/runtime-apis/websockets/

Unless you specifically need it maybe it can be pollyfilled or enabled through node compat options?
https://developers.cloudflare.com/workers/runtime-apis/nodejs/

@maemigh
Copy link
Author

maemigh commented Mar 20, 2024

I have enabled the node compat options, but this is specifically failing (on the +server.ts endpoint) because the ws package.json has

"main": "index.js",
"exports": {
    ".": {
      "browser": "./browser.js",
      "import": "./wrapper.mjs",
      "require": "./index.js"
    },
    "./package.json": "./package.json"
  },
"browser": "browser.js",

and the adapter is being built with platform: 'browser',

The changelog indicates that adapter-cloudflare was once platform: 'neutral' which I think might fix this, but was changed because of cross-fetch. I'm thinking neutral or node would still be a more appropriate setting though, perhaps with setting conditions or some other setting to specifically use browser just for cross-fetch.

There are more details here: #8122

Also, I believe ws should work on cloudflare--the neonserverless driver mentions it's required for use on Cloudflare when using Pool or Client connect methods.

@eltigerchino
Copy link
Member

eltigerchino commented Mar 20, 2024

Can you provide a minimal reproduction in the form of a downloadable / online repository? That would be a great help.

@maemigh
Copy link
Author

maemigh commented Mar 21, 2024

Sure.
https://github.com/maemigh/cloudflare-ws-test

Install wranger, run with
npx wrangler pages dev .svelte-kit/cloudflare

Then browse to http://localhost:8788/websocket and check the wranger console for the internal server error saying it can't run in the browser (coming from +server.ts).

@eltigerchino
Copy link
Member

Sure. maemigh/cloudflare-ws-test

Install wranger, run with npx wrangler pages dev .svelte-kit/cloudflare

Then browse to http://localhost:8788/websocket and check the wranger console for the internal server error saying it can't run in the browser (coming from +server.ts).

The repository you've linked only has a README.md

@maemigh
Copy link
Author

maemigh commented Mar 21, 2024

sorry, cant believe i did that. just synced it.

@eltigerchino
Copy link
Member

eltigerchino commented Mar 22, 2024

After patching the adapter with the build options from this PR https://github.com/sveltejs/kit/pull/8122/files it does give a better error message. But I don't think it works with Cloudflare Workers.

> Using @sveltejs/adapter-cloudflare
✘ [ERROR] Could not resolve "zlib"

    node_modules/ws/lib/permessage-deflate.js:3:21:
      3 │ const zlib = require('zlib');~~~~~~

  Cannot use "zlib" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "http"

    node_modules/ws/lib/websocket-server.js:6:21:
      6 │ const http = require('http');~~~~~~

  Cannot use "http" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "https"

    node_modules/ws/lib/websocket.js:6:22:
      6 │ const https = require('https');~~~~~~~

  Cannot use "https" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "http"

    node_modules/ws/lib/websocket.js:7:21:
      7 │ const http = require('http');~~~~~~

  Cannot use "http" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "net"

    node_modules/ws/lib/websocket.js:8:20:
      8 │ const net = require('net');~~~~~

  Cannot use "net" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "tls"

    node_modules/ws/lib/websocket.js:9:20:
      9 │ const tls = require('tls');~~~~~

  Cannot use "tls" when deploying to Cloudflare.


✘ [ERROR] Could not resolve "url"

    node_modules/ws/lib/websocket.js:12:24:
      12 │ const { URL } = require('url');~~~~~

  Cannot use "url" when deploying to Cloudflare.

The neon docs states you only need ws where there is no WebSocket support. Cloudflare Workers support the WebSocket API so just don't do the below. https://neon.tech/docs/serverless/serverless-driver#pool-and-client-usage-notes

// don't do this if you have access to the WebSocket API
import ws from 'ws';
neonConfig.webSocketConstructor = ws;

@eltigerchino eltigerchino closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2024
@aolose
Copy link
Contributor

aolose commented Apr 2, 2024

In fact you don't need to use any additional third-party websockets.
Local development can use websocket from vite
cloudflare please use WebSocketPair

For convenience, I wrote a vite plug-in to integrate websocket.
You can refer to this:https://github.com/aolose/sk-cf-ws-demo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants