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

Issue with nodejs_compat + dynamic require of node:events #854

Open
dja opened this issue May 2, 2023 · 15 comments
Open

Issue with nodejs_compat + dynamic require of node:events #854

dja opened this issue May 2, 2023 · 15 comments
Labels
feature request Request for Workers team to add a feature

Comments

@dja
Copy link

dja commented May 2, 2023

Which Cloudflare product(s) does this pertain to?

Workers/Other, Wrangler

What version of Wrangler are you using?

2.18.0

What operating system are you using?

Mac

Describe the Bug

Attempting to upload a worker with Wrangler using compatibility_flags = ['nodejs_compat'] or command line --compatibility-flags="nodejs_compat" fails with this error message:

  Uncaught Error: Dynamic require of "node:events" is not supported
    at worker.js:13:11
    at worker.js:13348:28
    at worker.js:13631:3
   [code: 10021]

The package using node:events is this LaunchDarkly SDK: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/cloudflare

@dja dja added the bug Something isn't working label May 2, 2023
@dja dja changed the title 🐛 BUG: 🐛 BUG: Issue with nodejs_compat + LaunchDarkly SDK May 2, 2023
@dja dja changed the title 🐛 BUG: Issue with nodejs_compat + LaunchDarkly SDK 🐛 BUG: Issue with nodejs_compat + dynamic require of node:events May 2, 2023
@ra-lfaro
Copy link

ra-lfaro commented May 9, 2023

Same exact issue for me as well using same package

@dcrodman
Copy link

dcrodman commented May 19, 2023

Having the same issue with trying to use import { Buffer } from "node:buffer"; per the docs:

service core:user:worker: Uncaught Error: Dynamic require of "node:buffer" is not supported
  at core:user:worker:14:11
  at core:user:worker:1749:28
  at core:user:worker:2897:3

This was with Wrangler version v.2.20.0 as well as v3.0.0. Also encountering this error with wrangler dev.

@ra-lfaro
Copy link

ra-lfaro commented Jun 5, 2023

Anyone have any luck resolving this?

@petebacondarwin
Copy link
Contributor

Right now, the only way to access these compatible libraries is by a static import {Buffer} from "node:buffer"; statement or a dynamic await import("node:Buffer") expression. Not a require() I believe.

@penalosa penalosa transferred this issue from cloudflare/workers-sdk Jul 11, 2023
@dcrodman
Copy link

I'm using import { Buffer } from "node:buffer"; (as opposed to require()) as the docs recommended and am still encountering the issue

@jasnell
Copy link
Member

jasnell commented Jul 12, 2023

Which workers syntax are y'all using? The older "server worker" syntax (using addEventListener) or the new ESM syntax e.g. export default { async fetch .... } ? If you have a simple repro script handy that doesn't use any specific dependencies that'll be helpful too.

@dcrodman
Copy link

@jasnell here's useless but functional barebones example where I see the problem:

worker.ts

import { Buffer } from 'node:buffer';

addEventListener('fetch', (event) => {
	let fetchEvent: FetchEvent = event as FetchEvent;
	fetchEvent.respondWith(handle(fetchEvent));
});

async function handle(event: FetchEvent): Promise<Response> {
	let b = Buffer.from(event.request.body);
	console.log(b);
	return new Response(event.request.body);
}

wrangler.toml

name = "cloudflare-broken-node-demo"
main = "src/worker.ts"
compatibility_date = "2023-07-10"
compatibility_flags = [ "nodejs_compat" ]

package.json

{
  "name": "cloudflare-broken-node-demo",
  "version": "0.0.0",
  "private": true,
  "scripts": {
    "deploy": "wrangler deploy",
    "start": "wrangler dev"
  },
  "devDependencies": {
    "@cloudflare/workers-types": "^4.20230419.0",
    "typescript": "^5.0.4",
    "wrangler": "^3.0.0"
  }
}

I haven't deployed this to the actual worker instance, but I see the issue I was seeing before with miniflare locally:

❯ npx wrangler dev
▲ [WARNING] It looks like you have used Wrangler v1's `config` command to login with an API token.

  This is no longer supported in the current version of Wrangler.
  If you wish to authenticate via an API token then please set the `CLOUDFLARE_API_TOKEN`
  environment variable.


 ⛅️ wrangler 3.2.0
------------------
wrangler dev now uses local mode by default, powered by 🔥 Miniflare and 👷 workerd.
To run an edge preview session for your Worker, use wrangler dev --remote
⎔ Starting local server...
service core:user:cloudflare-broken-node-demo: Uncaught Error: Dynamic require of "node:buffer" is not supported
  at core:user:cloudflare-broken-node-demo:8:11
  at core:user:cloudflare-broken-node-demo:256:28
  at core:user:cloudflare-broken-node-demo:266:3
✘ [ERROR] MiniflareCoreError [ERR_RUNTIME_FAILURE]: The Workers runtime failed to start. There is likely additional logging output above.

@irvinebroque
Copy link
Collaborator

@dcrodman — looks like this issue is specific to the legacy service worker syntax. I can reproduce it, but only when using the addEventListener syntax.

addEventListener('fetch', (event) => {
	let fetchEvent: FetchEvent = event as FetchEvent;
	fetchEvent.respondWith(handle(fetchEvent));
});

Instead, please use the ESM syntax:

export default {
	async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
		let b = Buffer.from(request.body);
		console.log(b);
		return new Response('Hello World!');
	},
};

I'll make sure to update our docs to be clearer about this, and we'll take a look at our error messages in this case.

For more on ESM syntax, and to migrate from using service workers, refer to: https://developers.cloudflare.com/workers/learning/migrate-to-module-workers/

@dcrodman
Copy link

Thank you @irvinebroque , the docs update feels like the most important part here since this can create a potentially difficult situation for people trying to upgrade to Wrangler 3. Migrating is still not necessarily easy and knowing this up front would've been super helpful.

@irvinebroque
Copy link
Collaborator

I think my statement above was misleading and not quite right.

Simple reproduction of the issue:

https://github.com/irvinebroque/nodejs_compat_issue/blob/main/src/worker.ts

// This does not work
const { Writable } = require('node:stream');

// This works
// import { Writable } from "node:stream";

const myStream = new Writable();

export default {
	async fetch(request) {
		return new Response("hello");
	},
};

We will take a look at what we can do here.

@mrbbot
Copy link
Contributor

mrbbot commented Aug 14, 2023

next-on-pages has a nice fix for this issue we could consider applying to all Wrangler users: https://github.com/cloudflare/next-on-pages/blob/7a18efb5cab4d86c8e3e222fc94ea88ac05baffd/packages/next-on-pages/src/buildApplication/processVercelFunctions/build.ts#L86-L112. I think this will only fix the case of require() inside an ES module worker though.

@g-wozniak
Copy link

The issue still persists in the latest @sveltejs/adapter-cloudflare :(

@jasnell
Copy link
Member

jasnell commented Mar 10, 2024

@irvinebroque ... the require(...) method is not present in regular ESM modules so I wouldn't expect that to work. The require(...) method is only injected into common js and node.js compat modules

@irvinebroque
Copy link
Collaborator

irvinebroque commented Mar 10, 2024

Ah right, unless the userland code is uploaded as a Node.js module type (#564, only supported in workerd, not by surrounding tooling) or CommonJS module type (only used if manually configured AFAIK) — then you can't use require() directly.

But that because Wrangler runs esbuild, it effectively papers over this — it looks like require() is supported but that's really just bundler magic.

Scaffolding out some better docs in cloudflare/cloudflare-docs#13344

@jasnell
Copy link
Member

jasnell commented Mar 18, 2024

I'm going to remove the bug label from this issue as this really isn't a bug in the runtime as much as it is just a limitation of the current implementation. I think we can recharacterize this, however, as a possible feature request asking for the module registry to work even with the old service worker syntax.

@jasnell jasnell added feature request Request for Workers team to add a feature and removed bug Something isn't working labels Mar 18, 2024
@jasnell jasnell changed the title 🐛 BUG: Issue with nodejs_compat + dynamic require of node:events Issue with nodejs_compat + dynamic require of node:events Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for Workers team to add a feature
Projects
None yet
Development

No branches or pull requests

8 participants