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

fix(worker): get dirname by fileURLToPath #2621

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sorich87
Copy link

@roggervalf tried to fix an issue with running sandboxed jobs in ESM environments by using module.filename (see #2296), but module.filename returns undefined and main.js path cannot be resolved.

Changing to fileURLToPath(import.meta.url) fixes the issue.

Context: We're trying to run sandboxed jobs with Next.js and getting the following error:

ENOENT: no such file or directory, stat '/xxx/nextjs/dist/cjs/classes/main.js'
    at Object.statSync (node:fs:1658:25)
    at new Worker (webpack-internal:///(instrument)/./node_modules/bullmq/dist/esm/classes/worker.js:114:53)
    at emailWorker (webpack-internal:///(instrument)/./src/jobs/email.ts:44:25)
    at Module.register (webpack-internal:///(instrument)/./src/instrumentation.ts:17:9)
    at async DevServer.runInstrumentationHookIfAvailable (/xxx/nextjs/node_modules/next/dist/server/dev/next-dev-server.js:437:17)
    at async Span.traceAsyncFn (/xxx/nextjs/node_modules/next/dist/trace/trace.js:154:20)
    at async DevServer.prepareImpl (/xxx/nextjs/node_modules/next/dist/server/dev/next-dev-server.js:214:9)
    at async NextServer.prepare (/xxx/nextjs/node_modules/next/dist/server/next.js:161:13)
    at async initializeImpl (/xxx/nextjs/node_modules/next/dist/server/lib/render-server.js:98:5)
    at async initialize (/xxx/nextjs/node_modules/next/dist/server/lib/router-server.js:423:22)
    at async Server.<anonymous> (/xxx/nextjs/node_modules/next/dist/server/lib/start-server.js:249:36) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: '/xxx/nextjs/dist/cjs/classes/main.js'
}

@@ -260,7 +260,7 @@ export class Worker<
}

// Separate paths so that bundling tools can resolve dependencies easier
const dirname = path.dirname(module.filename || __filename);
const dirname = path.dirname(fileURLToPath(import.meta.url) || __filename);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, do you have some explanation or link on to why this change is better than the existing code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be safer the following:

const dirname = path.dirname(module.filename || fileURLToPath(import.meta.url) || __filename);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even:

const dirname = path.dirname(module.filename || __filename || fileURLToPath(import.meta.url) );

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

Successfully merging this pull request may close these issues.

2 participants