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 debugger port #60919

Closed
wants to merge 2 commits into from
Closed

Conversation

HelmerBarcos
Copy link

@HelmerBarcos HelmerBarcos commented Jan 20, 2024

What?

This PR is clone of #53683 after Tim's recommendation. See #53683 (comment)

This enables debugging sessions from a remote host. This is helpful while debugging a Next.js application that runs inside a Docker container with following possibilities inside the package.json:

    "dev": "NODE_OPTIONS='--inspect=localhost:9250' next dev",
    "dev": "NODE_OPTIONS='--inspect=co.localhost:9250' next dev",
    "dev": "NODE_OPTIONS='--inspect=barcos.co:9250' next dev",
    "dev": "NODE_OPTIONS='--inspect=barcos.co.co.hzoe.com:9250' next dev",
    "dev": "NODE_OPTIONS='--inspect=0.0.0.0:9250' next dev",
    "dev": "NODE_OPTIONS='--inspect=9250' next dev",
    "dev": "NODE_OPTIONS='--inspect' next dev",
    "dev": "NODE_OPTIONS='--inspect=127.0.0.1:9250' next dev",

Bug

This fixes:

Other "fixed" bugs

Why?

It was an attempt to fix the bug, but it didn't work for all the cases.

Keep in mind that the default host will be 127.0.0.1, and this will be blocked for external hosts trying to attach to the debugger. The NODE_OPTIONS should be set to NODE_OPTIONS='--inspect=0.0.0.0:SOME_PORT'.
https://nodejs.org/en/docs/guides/debugging-getting-started#security-implications

How?

By improving the regex used for matching a debug port and adding some conditions to be met.

@PierreCavalet
Copy link

Any news on this ? the issue is several months old...

Can you either merge it or provide change requests so we can properly debug next.js application running in docker container ?

@antoninhpix
Copy link

meanwhile what I did (gross but it works -- I'm using vscode)

the whole point is to be able to pass 0.0.0.0 in the --inspect argument so we can attach to it from outside docker.

this default to 127.0.0.1 which is why we can't attach from a docker machine.

Right now nextjs thinks there can only be a port in this --inspect command

open ./node_modules/next/dist/server/lib/utils.d.ts

and add line 4

export declare const getDebugAddr: () => string;
image

open ./node_modules/next/dist/server/lib/utils.js

and add getDebugAddr function (copy paste from getDebugPort function) and change line 70 to 76 and export it like the getDebugPort function

const getDebugAddr = ()=>{
  var _process_execArgv_find, _process_env_NODE_OPTIONS_match, _process_env_NODE_OPTIONS_match1, _process_env_NODE_OPTIONS;
  const debugPortStr = ((_process_execArgv_find = process.execArgv.find((localArg)=>localArg.startsWith("--inspect") || localArg.startsWith("--inspect-brk"))) == null ? void 0 : _process_execArgv_find.split("=", 2)[1]) ?? ((_process_env_NODE_OPTIONS = process.env.NODE_OPTIONS) == null ? void 0 : (_process_env_NODE_OPTIONS_match1 = _process_env_NODE_OPTIONS.match) == null ? void 0 : (_process_env_NODE_OPTIONS_match = _process_env_NODE_OPTIONS_match1.call(_process_env_NODE_OPTIONS, /--inspect(-brk)?(=(\S+))?( |$)/)) == null ? void 0 : _process_env_NODE_OPTIONS_match[3]);
  const debugAddrStr = debugPortStr.split(':', 2)
  let addr = "127.0.0.1"
  if (debugAddrStr.length > 1) {
    addr = debugAddrStr[0]
  }
  console.log('nextjs open debug connection ', addr)
  return addr
};
image

open ./node_modules/next/dist/cli/next-dev.js

and change line 231 to add the getDebugAddr function

if (nodeDebugType) {
 NODE_OPTIONS = `${NODE_OPTIONS} --${nodeDebugType}=${(0, _utils.getDebugAddr)()}:${(0, _utils.getDebugPort)() + 1}`;
}
image

so that in the dev script we can add an address as well as a port

"dev": "npm run next-config && NODE_OPTIONS='--inspect=0.0.0.0:9229' next dev",
image

and finally this is the vscode configuration.

The port differ because nextjs open 2 ports. The one you pass plus the port right after it. Looks like we need to attach to this one and not the one we pass

{
      "type": "node",
      "request": "attach",
      "address": "localhost",
      "port": 9230,
      "name": "Docker: Attach to Node",
      "remoteRoot": "/var/www",
      "localRoot": "${workspaceFolder}",
      "skipFiles": ["<node_internals>/**"]
}
image

I hope for a fix as soon as possible so that I can delete this monstruosity from the internet ... ❤️

@solverat
Copy link

solverat commented Apr 15, 2024

@HelmerBarcos nice! regarding the getDebugPort regex pattern. You just may have to extend the regex pattern within the getDebugPort method:

process.env.NODE_OPTIONS?.match?.(/--inspect(-brk)?(=(\S+))?( |$)/)?.[3]
return debugPortStr ? parseInt(debugPortStr, 10) : 9229

...to --inspect(-brk)?(=(?:\S+)(?::([0-9]+)))?( |$), and it should work. See regex101

Edit: I've just updated the regex.

@ijjk
Copy link
Member

ijjk commented Apr 15, 2024

Allow CI Workflow Run

  • approve CI run for commit: 0ced190

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@solverat
Copy link

@HelmerBarcos Also please consider the NODE_OPTIONS node here:

NODE_OPTIONS = `${NODE_OPTIONS} --${nodeDebugType}=${
getDebugPort() + 1

nextjs adds an additional debuggerlistener (default on 9230) which currently does not work because it does not respect the host/ip. You may want to adopt some hints from @antoninhpix comment (getDebugAddr).

Hopefully this will get fixed soon. Currently, nextjs is broken when running in docker.

Thanks!

@wyattjoh wyattjoh mentioned this pull request Apr 24, 2024
@github-actions github-actions bot added the locked label May 9, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants