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

[NEXT-1176] Debugging with non-localhost address stopped working #47083

Closed
1 task done
gregmartyn opened this issue Mar 13, 2023 · 10 comments · Fixed by #48019 or #65006
Closed
1 task done

[NEXT-1176] Debugging with non-localhost address stopped working #47083

gregmartyn opened this issue Mar 13, 2023 · 10 comments · Fixed by #48019 or #65006
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@gregmartyn
Copy link

gregmartyn commented Mar 13, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP PREEMPT_DYNAMIC Fri Mar  3 17:29:44 UTC 2023
    Binaries:
      Node: 17.9.0
      npm: 8.5.5
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 13.2.4
      eslint-config-next: 13.2.4
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

CLI (create-next-app)

Link to the code that reproduces this issue

https://github.com/vercel/next.js/blame/8d83d85e5359146bbbd0f96ea2f8490e7d6bc9cd/packages/next/src/cli/next-dev.ts#L488

To Reproduce

NODE_OPTIONS=--inspect=0.0.0.0:9229 next dev

Describe the Bug

NodeJS's --inspect switch takes an optional address as a parameter that isn't being honored by Next. Next crashes when that is present.
spec: --inspect[=[host:]port]
https://nodejs.org/api/cli.html#--inspecthostport

In order to be able to expose a debugger from inside a Docker container, it has to listen on 0.0.0.0 instead of the default 127.0.0.1. To do that, we have to specify --inspect=0.0.0.0:9229.

https://github.com/vercel/next.js/blame/8d83d85e5359146bbbd0f96ea2f8490e7d6bc9cd/packages/next/src/cli/next-dev.ts#L488

assumes the parameter is an integer: parseInt(debugPortStr, 10), but debugPortStr could be e.g. 0.0.0.0:9229

https://github.com/vercel/next.js/blob/canary/packages/next/src/cli/next-dev.ts#L524
then does
--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort + 1}

which crashes:

warn  - the --inspect option was detected, the Next.js server should be inspected at port 1.
/usr/bin/node:  must be 0 or in range 1024 to 65535.

Expected Behavior

Next should parse out and honor the address part of the --inspect switch.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

Docker for local dev, Vercel for hosting

NEXT-1176

@gregmartyn gregmartyn added the bug Issue was opened via the bug report template. label Mar 13, 2023
@JackWatson06
Copy link

Thank you for posting this! You beat me to it. Was having this EXACT same issue yesterday.

@itsravenous
Copy link

I have this issue too (using e.g. --inspect=:9230). Curious, why does Next try to increment the port number?

@JackWatson06
Copy link

I have this issue too (using e.g. --inspect=:9230). Curious, why does Next try to increment the port number?

The dev server spawns a new child node.js process (which has been occurring since recent updates to Next.js) - as such, it increments the inspect debug port as it's different from the primary process which started next.js. This comment somewhat discusses this concept: https://github.com/vercel/next.js/pull/45745/files#r1101862213

@edorgeville
Copy link

edorgeville commented Mar 17, 2023

@JackWatson06 points in the right direction.
This line more specifically:

return debugPortStr ? parseInt(debugPortStr, 10) : 9229

It expects debugPortStr to be shaped like an integer. But if you use --inspect:0.0.0.0:12345, debugPortStr is now 0.0.0.0:12345, thus the parse fails and returns 0.
Then not long after, it tries using this 0 port and does +1 on it.
`--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort + 1}`

Causing the /usr/bin/node: must be 0 or in range 1024 to 65535. error.

EDIT: woops, turns out @gregmartyn had already provided this explanation in the issue body 😅

@pirtlj
Copy link

pirtlj commented Mar 27, 2023

I'm seeing this as well, using docker running

"dev": "NODE_OPTIONS=\"--inspect=0.0.0.0:9229\" next dev -p 3001",

and no longer hitting break points

I added the port to my docker-compose and it worked

ports:
- "3001:3001"
- "9229:9229"
- "9230:9230"

@Cow258
Copy link
Contributor

Cow258 commented Apr 6, 2023

13.2.4 is working well, but it not working on the latest Next.js canary release.

Expected(v13.2.4): both 9229 and 9230 should be used
image
image

Currently(v13.3.1-canary.0): only 9229 used, 9230 is missing
image
image

@Cow258
Copy link
Contributor

Cow258 commented Apr 12, 2023

Hi @huozhi,

If you have some spare time, would you be able to help me further with this issue?

Debugger is something I often use to quickly find problems,
but the latest release is not working for me and it's causing me some trouble.

Thank you.

@pirtlj
Copy link

pirtlj commented May 1, 2023

I'm having this issue still 13.3.1 i'm using

NODE_OPTIONS=\"--inspect=0.0.0.0:9229\" next dev -p 3001

I'm also running in docker with

ports:
  - "3001:3001"
  - "9229:9229"
  - "9230:9230"

@timneutkens timneutkens added the linear: next Confirmed issue that is tracked by the Next.js team. label May 17, 2023
@timneutkens timneutkens changed the title Debugging with non-localhost address stopped working [NEXT-1176] Debugging with non-localhost address stopped working May 17, 2023
ijjk added a commit that referenced this issue May 23, 2023
## Bug

This fix closes #47083 issue 
This fix closes #47561 issue
This fix closes #48376 issue
**Invalid repetition PRs:** #47671 (this PR changing expired code)
(This issue still exist on
[v13.4.3-canary.1](https://github.com/vercel/next.js/releases/tag/v13.4.3-canary.1)

- [x] Related issues linked using `fixes #number`

### What?
When running `NODE_OPTIONS='--inspect' next dev`, 
the render server didn't start with `--inspect`. 
In some cases, 
the `--inspect` flag will be passed when `__NEXT_DISABLE_MEMORY_WATCHER`
was set.

### Why?
Since #47208 revamped some startup processes, the `NODE_OPTIONS`
environment parameter is not passed down to the render server worker.

### How?
Just add back the original startup process.


![image](https://user-images.githubusercontent.com/14261588/230398898-791e6909-6f4c-493b-937d-058a7b788849.png)


link NEXT-1176

---------

Co-authored-by: JJ Kasper <jj@jjsweb.site>
@gregmartyn
Copy link
Author

@ijjk I don't see the fix addressing an inspect arg with an IP address like --inspect=0.0.0.0:9229

hydRAnger pushed a commit to hydRAnger/next.js that referenced this issue Jun 12, 2023
## Bug

This fix closes vercel#47083 issue 
This fix closes vercel#47561 issue
This fix closes vercel#48376 issue
**Invalid repetition PRs:** vercel#47671 (this PR changing expired code)
(This issue still exist on
[v13.4.3-canary.1](https://github.com/vercel/next.js/releases/tag/v13.4.3-canary.1)

- [x] Related issues linked using `fixes #number`

### What?
When running `NODE_OPTIONS='--inspect' next dev`, 
the render server didn't start with `--inspect`. 
In some cases, 
the `--inspect` flag will be passed when `__NEXT_DISABLE_MEMORY_WATCHER`
was set.

### Why?
Since vercel#47208 revamped some startup processes, the `NODE_OPTIONS`
environment parameter is not passed down to the render server worker.

### How?
Just add back the original startup process.


![image](https://user-images.githubusercontent.com/14261588/230398898-791e6909-6f4c-493b-937d-058a7b788849.png)


link NEXT-1176

---------

Co-authored-by: JJ Kasper <jj@jjsweb.site>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 2, 2023
timneutkens pushed a commit that referenced this issue Apr 25, 2024
<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

### What?

Previously, parsing and managing the `NODE_OPTIONS` was performed using
a series of regular expressions. These were prone to bugs, and have
already caused a few issues. This moves us over to the standard
`parseArgs`
([docs](https://nodejs.org/docs/latest/api/util.html#utilparseargsconfig)):

```js
import { parseArgs } from "node:utils"
```

### Why?

This simplifies the argument parser dramatically, removing the need for
any special patterns or accommodations. No need to maintain all these
patterns when there's a lightweight built-in parser already available.

Fixes #53127
Fixes #53757
Fixes #47083
Fixes #50489
Closes #60919 
Closes #59410
Closes NEXT-3219
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.
Projects
None yet
7 participants