yarnpkg / berry

📦🐈 Active development trunk for Yarn ⚒
https://yarnpkg.com
BSD 2-Clause "Simplified" License
7.43k stars 1.11k forks source link

[Bug] `pnpapi.setup()` in main thread not affecting worker threads #2476

Open clemyan opened 3 years ago

clemyan commented 3 years ago

Describe the bug

Calling pnpapi.setup() in the main thread does not set up PnP insider worker threads, causing worker threads to fail when require-ing dependencies.

This affects stuff like the eslint pnpify sdk, since that just calls pnpapi.setup() and eslint plugins may spawn worker threads.

To Reproduce

Reproduction ```js repro const fs = require('fs/promises') const mainContent = ` const { Worker, isMainThread } = require('worker_threads') if (isMainThread) { require('./.pnp.cjs').setup() new Worker(__filename) } else { require('answer/linked.js') } ` await Promise.all([ fs.writeFile('./linked.js', ''), fs.writeFile('./main.js', mainContent) ]) await packageJsonAndInstall({ dependencies: { 'answer': 'link:.' }, }) await expect(yarn('node', 'main.js')).resolves.toBe('') const { execSync } = require('child_process') // Clear NODE_OPTIONS set by the sherlock invocation expect(() => execSync('node main.js', { env: { NODE_OPTIONS: '' }, encoding: 'utf8' })).not.toThrow() ```

Environment if relevant (please complete the following information):

yarnbot commented 3 years ago

This issue reproduces on master:

Error: expect(received).resolves.toBe()

Received promise rejected instead of resolved
Rejected to value: [Error: Command failed: /usr/bin/node /github/workspace/scripts/actions/../run-yarn.js node main.js

events.js:292
      throw er; // Unhandled 'error' event
      ^
Error: Qualified path resolution failed - none of those files can be found on the disk.

Source path: /tmp/tmp-22700eKZPGtB2E/linked.js/
Not found: /tmp/tmp-22700eKZPGtB2E/linked.js/
Not found: /tmp/tmp-22700eKZPGtB2E/linked.js/.js
Not found: /tmp/tmp-22700eKZPGtB2E/linked.js/.json
Not found: /tmp/tmp-22700eKZPGtB2E/linked.js/.node

Require stack:
- /tmp/tmp-22700eKZPGtB2E/main.js
    at Function.external_module_.Module._resolveFilename (/tmp/tmp-22700eKZPGtB2E/.pnp.cjs:5144:55)
    at Function.external_module_.Module._load (/tmp/tmp-22700eKZPGtB2E/.pnp.cjs:4972:48)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/tmp/tmp-22700eKZPGtB2E/main.js:7:3)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.external_module_.Module._load (/tmp/tmp-22700eKZPGtB2E/.pnp.cjs:5003:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:545:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:358:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26)

]
    at expect (/github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-0ac41999f0.zip/node_modules/expect/build/index.js:138:15)
    at module.exports (evalmachine.<anonymous>:23:7)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
    at async Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)
yarnbot commented 3 years ago

This issue reproduces on master:

Error: expect(received).resolves.toBe()

Received promise rejected instead of resolved
Rejected to value: [Error: Command failed: /usr/bin/node /github/workspace/scripts/actions/../run-yarn.js node main.js

events.js:292
      throw er; // Unhandled 'error' event
      ^
Error: Qualified path resolution failed - none of those files can be found on the disk.

Source path: /tmp/tmp-21lwAkNp9SVDaq/linked.js/
Not found: /tmp/tmp-21lwAkNp9SVDaq/linked.js/
Not found: /tmp/tmp-21lwAkNp9SVDaq/linked.js/.js
Not found: /tmp/tmp-21lwAkNp9SVDaq/linked.js/.json
Not found: /tmp/tmp-21lwAkNp9SVDaq/linked.js/.node

Require stack:
- /tmp/tmp-21lwAkNp9SVDaq/main.js
    at Function.external_module_.Module._resolveFilename (/tmp/tmp-21lwAkNp9SVDaq/.pnp.cjs:5144:55)
    at Function.external_module_.Module._load (/tmp/tmp-21lwAkNp9SVDaq/.pnp.cjs:4972:48)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/tmp/tmp-21lwAkNp9SVDaq/main.js:7:3)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.external_module_.Module._load (/tmp/tmp-21lwAkNp9SVDaq/.pnp.cjs:5003:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:545:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:358:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26)

]
    at expect (/github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-0ac41999f0.zip/node_modules/expect/build/index.js:138:15)
    at module.exports (evalmachine.<anonymous>:23:7)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
    at async Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)
clemyan commented 3 years ago

Hmm.. that wasn't the error I was expecting. Looks like some how the sherlock environment cannot directly link: to a file. I will edit the repro.

yarnbot commented 3 years ago

The reproduction case in your issue seems broken (ie it neither pass nor fail due to throwing an unmanaged exception):

Error: Command failed: node main.js

events.js:292
      throw er; // Unhandled 'error' event
      ^
Error: Cannot find module 'answer/linked.js'
Require stack:
- /tmp/tmp-21Uju8FVC8BG1Q/main.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/tmp/tmp-21Uju8FVC8BG1Q/main.js:7:3)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:545:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:358:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/tmp/tmp-21Uju8FVC8BG1Q/main.js' ]
}

    at checkExecSyncError (child_process.js:616:11)
    at execSync (child_process.js:652:15)
    at module.exports (evalmachine.<anonymous>:27:8)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
    at async Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)

Remember: any non-Jest exceptions will cause the test to be reported as broken. If you expect something to pass without throwing, you must wrap it into something like await expect(...).resolves.toBeTruthy(). If you instead expect something to throw, you need to wrap it into await expect(...).rejects.toThrow().

yarnbot commented 3 years ago

This issue reproduces on master:

Error: expect(received).not.toThrow()

Error name:    "Error"
Error message: "Command failed: node main.js

events.js:292
      throw er; // Unhandled 'error' event
      ^
Error: Cannot find module 'answer/linked.js'
Require stack:
- /tmp/tmp-23II8su6BG9fO0/main.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/tmp/tmp-23II8su6BG9fO0/main.js:7:3)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:545:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:358:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/tmp/tmp-23II8su6BG9fO0/main.js' ]
}
"

      5 |   new Worker(__filename)
      6 | } else {
    > 7 |   require('answer/linked.js')
        |   ^
      8 | }
      9 | 

      at Object.<anonymous> (main.js:7:3)
      Emitted 'error' event on process instance at:
      at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26) {
        code: 'MODULE_NOT_FOUND',
        requireStack: [ '/tmp/tmp-23II8su6BG9fO0/main.js' ]
      }
      at evalmachine.<anonymous>:27:14
      at Object.<anonymous> (../../github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-0ac41999f0.zip/node_modules/expect/build/toThrowMatchers.js:81:11)
      at Object.throwingMatcher [as toThrow] (../../github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-0ac41999f0.zip/node_modules/expect/build/index.js:342:33)
      at module.exports (evalmachine.<anonymous>:27:93)
      at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
      at async executeInTempDirectory (../../github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
      at async Object.executeRepro (../../github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at module.exports (evalmachine.<anonymous>:27:93)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
    at async Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-63f998598d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-a57989414f.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)
clemyan commented 3 years ago

More investigation:

The core of the issue is how (the various ways of setting) the --require flag affects workers. So, I have done some tests:

cosnt tests = [
    /* 1 */ () => {
        const worker = new Worker(__filename)
    },
    /* 2 */ () => {
        const orig = process.execArgv
        process.execArgv = ['--require', './preload.js', ...process.execArgv]
        const worker = new Worker(__filename)
        process.execArgv = orig
    },
    /* 3 */ () => {
        const worker = new Worker(__filename, { execArgv: ['--require', './preload.js', ...process.execArgv] })
    },
    /* 4 */ () => {
        const orig = process.env.NODE_OPTIONS
        process.env.NODE_OPTIONS = `--require ./preload.js ${process.env.NODE_OPTIONS}`
        const worker = new Worker(__filename)
        process.env.NODE_OPTIONS = orig
    },
    /* 5 */ () => {
        const orig = process.env.NODE_OPTIONS
        process.env.NODE_OPTIONS = `--require ./preload.js ${process.env.NODE_OPTIONS}`
        const worker = new Worker(__filename, { env: SHARE_ENV })
        process.env.NODE_OPTIONS = orig
    },
    /* 6 */ () => {
        const worker = new Worker(__filename, { env: { NODE_OPTIONS: `--require ./preload.js ${process.env.NODE_OPTIONS}` } })
    },
]

where preload.js is simply

console.log('preload', process.pid, require('worker_threads').threadId)

Out of these, only 3 and 6 (explicitly passing execArgv and env.NODE_OPTIONS to the Worker constructor, respectively) works.

I also tested the equivalent invocations of child_process.fork (except SHARE_ENV since that has no equivalent).

cosnt tests = [
    /* 1 */ () => {
        const cp = fork('./foo.js')
    },
    /* 2 */ () => {
        const orig = process.execArgv
        process.execArgv = ['--require', './preload.js', ...process.execArgv]
        const cp = fork('./foo.js')
        process.execArgv = orig
    },
    /* 3 */ () => {
        const cp = fork('./foo.js', { execArgv: ['--require', './preload.js', ...process.execArgv] })
    },
    /* 4 */ () => {
        const orig = process.env.NODE_OPTIONS
        process.env.NODE_OPTIONS = `--require ./preload.js ${process.env.NODE_OPTIONS}`
        const cp = fork('./foo.js')
        process.env.NODE_OPTIONS = orig
    },
    /* 6 */ () => {
        const cp = fork('./foo.js', { env: { NODE_OPTIONS: `--require ./preload.js ${process.env.NODE_OPTIONS}` } })
    },
]

Out of these, all but 1 works. This show there is a discrepancy between how worker_threads and child_process handle process.execArgv and process.env.NODE_OPTIONS.

Question: Should we just handle it in Yarn (e.g. monkey-patch Worker constructor) or report this as a bug in Node? Related: https://github.com/nodejs/node/issues/29117