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 NODE_OPTIONS env forwarding #11587

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Conversation

cometkim
Copy link
Contributor

@cometkim cometkim commented Sep 19, 2024

I got this error when executing Run Dev Server on the VScode

api | Waiting for the debugger to disconnect...
api | node:events:497
api |       throw er; // Unhandled 'error' event
api |       ^
api | 
api | Error: spawn Studio ENOENT
api |     at ChildProcess._handle.onexit (node:internal/child_process:286:19)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
api | Emitted 'error' event on ChildProcess instance at:
api |     at ChildProcess._handle.onexit (node:internal/child_process:292:12)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
api |   errno: -2,
api |   code: 'ENOENT',
api |   syscall: 'spawn Studio',
api |   path: 'Studio',
api |   spawnargs: [
api |     'Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js  --inspect-publish-uid=http --enable-source-maps',
api |     'yarn',
api |     'nodemon',
api |     '--quiet',
api |     '--watch',
api |     '/Users/tim/Workspace/src/github.com/daangn/activity-badge-admin/redwood.toml',
api |     '--exec',
api |     'yarn rw-api-server-watch       --port 8911       --debug-port 18911       | rw-log-formatter'
api |   ]
api | }

Because the VSCode debugger injects this env variable,

 --require "/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js"  --inspect-publish-uid=http

But not escaped correctly

yarn cross-env NODE_ENV=development NODE_OPTIONS="--require "/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js"  --inspect-publish-uid=http --enable-source-maps"   yarn nodemon     --quiet     --watch "/Users/tim/Workspace/src/github.com/daangn/activity-badge-admin/redwood.toml"     --exec "yarn rw-api-server-watch       --port 8911       --debug-port 18911       | rw-log-formatter" exited with code 1

@cometkim
Copy link
Contributor Author

Btw, when Yarn (berry) supports "normalized shell", so no need to use cross-env

@ahaywood
Copy link
Contributor

@cometkim Thanks for taking the time report. I'll get someone on our team to take a look.

@Tobbe
Copy link
Member

Tobbe commented Sep 19, 2024

@cometkim Thanks for your PR. This looks very promising. I had no idea about yarn's updated abilities to help with cross environment stuff. That's great!

I'm seeing some messages about Studio in your log output. And something about a debugger. I don't often start things from within vscode, I usually just run things from the terminal. Do you think you could add some more detailed reproduction steps? So I can clearly see the problem you're fixing and also see the fix working :) Thank you! 🙏

@cometkim
Copy link
Contributor Author

I'm seeing some messages about Studio in your log output. And something about a debugger. I don't often start things from within vscode, I usually just run things from the terminal. Do you think you could add some more detailed reproduction steps?

"Studio" is a truncated portion of the path to VSCode installed via Homebrew. (The actual path is /Applications/Visual Studio Code.app/Contents/...)

The debug script is automatically injected when running with the VSCode luanch.json task. So if you can reproduce the issue by running Redwood in VSCode installed in a path with spaces (or any chars that need escaping).

@cometkim
Copy link
Contributor Author

cometkim commented Sep 20, 2024

Or I think you can reproduce the issue by providing a random NODE_OPTIONS environment variable with any escape seq.

export NODE_OPTIONS="--require /path/to/script with spaces.js"
yarn rw dev

@Josh-Walker-GM Josh-Walker-GM self-assigned this Sep 24, 2024
@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Sep 24, 2024
@Josh-Walker-GM
Copy link
Collaborator

Hey @cometkim 👋

I think I was able to reproduce the issue with:

  1. Export some NODE_OPTIONS with a space in it.
export NODE_OPTIONS="--require \"/Users/jgmw/Development/redwood/rw-test/node_options_fix/script space.js\""
  1. Run the api side dev server
yarn rw dev api
  1. Get an error:
api | node:events:497
api |       throw er; // Unhandled 'error' event
api |       ^
api | 
api | Error: spawn space.js --enable-source-maps ENOENT
api |     at ChildProcess._handle.onexit (node:internal/child_process:286:19)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
api | Emitted 'error' event on ChildProcess instance at:
api |     at ChildProcess._handle.onexit (node:internal/child_process:292:12)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
api |   errno: -2,
api |   code: 'ENOENT',
api |   syscall: 'spawn space.js --enable-source-maps',
api |   path: 'space.js --enable-source-maps',
api |   spawnargs: [
api |     'yarn',
api |     'nodemon',
api |     '--quiet',
api |     '--watch',
api |     '/Users/jgmw/Development/redwood/rw-test/node_options_fix/redwood.toml',
api |     '--exec',
api |     'yarn rw-api-server-watch       --port 8911       --debug-port 18911       | rw-log-formatter'
api |   ]
api | }
api | 
api | Node.js v20.17.0

Using your changes (which look great) then the error no longer happens.

One thing is that I still had to escape the value passed into require via NODE_OPTIONS - and that feel like it would always have to be the case?

@Josh-Walker-GM
Copy link
Collaborator

I'm happy with these changes. If you confirm that it fixes the problem you were experiencing @cometkim then I'd be happy to merge once we fix any tests.

@Josh-Walker-GM
Copy link
Collaborator

Okay I'll just merge this and get it out. If it doesn't quite fix the problem then we can follow up. It would be ideal to follow up switching the other commands to use env and such anyway.

@Josh-Walker-GM Josh-Walker-GM merged commit 6cc6e51 into redwoodjs:main Sep 24, 2024
50 checks passed
Josh-Walker-GM added a commit that referenced this pull request Sep 24, 2024
I got this error when executing `Run Dev Server` on the VScode

```
api | Waiting for the debugger to disconnect...
api | node:events:497
api |       throw er; // Unhandled 'error' event
api |       ^
api | 
api | Error: spawn Studio ENOENT
api |     at ChildProcess._handle.onexit (node:internal/child_process:286:19)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
api | Emitted 'error' event on ChildProcess instance at:
api |     at ChildProcess._handle.onexit (node:internal/child_process:292:12)
api |     at onErrorNT (node:internal/child_process:484:16)
api |     at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
api |   errno: -2,
api |   code: 'ENOENT',
api |   syscall: 'spawn Studio',
api |   path: 'Studio',
api |   spawnargs: [
api |     'Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js  --inspect-publish-uid=http --enable-source-maps',
api |     'yarn',
api |     'nodemon',
api |     '--quiet',
api |     '--watch',
api |     '/Users/tim/Workspace/src/github.com/daangn/activity-badge-admin/redwood.toml',
api |     '--exec',
api |     'yarn rw-api-server-watch       --port 8911       --debug-port 18911       | rw-log-formatter'
api |   ]
api | }
```

Because the VSCode debugger injects this env variable,

```
 --require "/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js"  --inspect-publish-uid=http
```

But not escaped correctly

```
yarn cross-env NODE_ENV=development NODE_OPTIONS="--require "/Applications/Visual Studio Code.app/Contents/Resources/app/extensions/ms-vscode.js-debug/src/bootloader.js"  --inspect-publish-uid=http --enable-source-maps"   yarn nodemon     --quiet     --watch "/Users/tim/Workspace/src/github.com/daangn/activity-badge-admin/redwood.toml"     --exec "yarn rw-api-server-watch       --port 8911       --debug-port 18911       | rw-log-formatter" exited with code 1
```

---------

Co-authored-by: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com>
@cometkim cometkim deleted the env-escape branch September 25, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants