Skip to content

Support pausing the debugger on script loadΒ #24687

Description

@roblourens

There is a Node debugging scenario that doesn't work very well in vscode (or chrome devtools), but does in debugging Chrome, and I want to start a discussion about how we can improve this.

Typically when debugging a Node project with sourcemaps, the scripts are transpiled to disk, vscode's launch config points at these files with the "outFiles" parameter, then vscode preloads the sourcemaps so it can set breakpoints in the correct locations before the scripts are actually loaded.

But there is a scenario where scripts are transpiled on demand, so vscode can't preload their sourcemaps, and can't set breakpoints at the right spots in those scripts until some time after they are loaded. This means that the breakpoints may not be hit, due to the race between running the code and setting the breakpoint at the same time. The best way for vscode to deal with this would be to have node pause execution each time a script is loaded, giving it a chance to load its sourcemaps and set the breakpoint before running the code in the script.

The Chrome devtools protocol for Chrome actually gives us a way to pause execution each time a script is loaded, via https://chromedevtools.github.io/devtools-protocol/1-3/DOMDebugger#method-setEventListenerBreakpoint. But this is in the DOMDebugger domain which doesn't exist in Node.

So what do you think it would take to get an api like this for node as well? Can nodejs implement a subset of the DOMDebugger domain, does a new domain need to be defined? Or maybe someone has an idea for another solution entirely.

Activity

  1. mike-kaufman commented on Nov 28, 2018

    @mike-kaufman

    /cc @ofrobots, @eugeneo - I think you two have context here on the CrDP protocol integration into Node?

  2. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    I believe that this method should be called setInstrumentationBreakpoint and should be part of Debugger domain, so it should be implemented in V8 and can be backported to Node 10.
    It could be even smarter and be something like scriptWithSourceMapParsed to reduce performance overhead.

  3. added
    diag-agendaIssues and PRs to discuss during Diagnostics Working Group meetings.
    on Nov 28, 2018
  4. hashseed commented on Nov 28, 2018

    @hashseed
    Member

    Seems like we should just expose the after-compile event to the devtools protocol?

  5. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    After compile events are exposed as Debugger.scriptParsed and Debugger.scriptFailedToParse events. In this use case it is important to not just send event but to pause JavaScript execution as well to give DevTools or VSCode frontends chance to fetch source map and set breakpoints based on this source map.

  6. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    DOMDebugger.setEventListenerBreakpoint for pause at first line should actually be moved all together to Debugger.setInstrumentationBreakpoint. It will help with crbug.com/724793 as well.

  7. hashseed commented on Nov 28, 2018

    @hashseed
    Member

    So I guess could expose an option that can be enabled which causes the Debugger.scriptParsed event to behave like a debugger pause that needs manual resume?

  8. hashseed commented on Nov 28, 2018

    @hashseed
    Member

    Do you envision this breakpoint to break at parsing/compiling, or upon execution?

  9. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    I prefer to emit regular Debugger.paused event, implementation wise it should be easier, with reason - instrumentationBreakpoint and instrumentationType inside data object, it should be less 'surprising' for different protocol clients and better aligned with existing DOMDebugger behavior.
    If this breakpoint is set we schedule pause on next function call using v8::debug::SetBreakOnNextFunctionCall. Ideally we should get another callback from V8 when top level function is finished to clear requested break if no javascript was executed but I am not sure that this case is possible.
    I believe that this breakpoint should trigger break at first script line.

  10. roblourens commented on Nov 28, 2018

    @roblourens
    Author

    Fixing http://crbug.com/724793 would be very helpful too!

    I think the break should happen immediately before execution. It should happen after Debugger.scriptParsed, because we need the sourcemapURL that comes with that event. And in some cases we would need the script id to set breakpoints.

    +1 for

    I prefer to emit regular Debugger.paused event, implementation wise it should be easier, with reason - instrumentationBreakpoint and instrumentationType inside data object, it should be less 'surprising' for different protocol clients and better aligned with existing DOMDebugger behavior.

  11. hashseed commented on Nov 28, 2018

    @hashseed
    Member

    Iirc SetBreakOnNextFunctionCall would pause before we even enter the top-level function. Sgtm.

  12. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    We can actually provide sourceMappingURL and sourceURL inside data param of Debugger.paused to make it easier.

  13. hashseed commented on Nov 28, 2018

    @hashseed
    Member

    Now that I think about this, triggering the pause at the after-compile event is probably more straightforward to implement. Otherwise we would have to schedule a pause and remember until that pause for what reason we paused (and what script URL to pass).

  14. alexkozy commented on Nov 28, 2018

    @alexkozy
    Member

    We already have infrastructure for this use case - V8DebuggerAgentImpl::schedulePauseOnNextStatement method. So we need to call this method from V8DebuggeragentImpl::didParseSource, we just need to add another flag to this method to distinguish call to this method from V8 and from V8DebuggerAgentImpl::enable method.

  15. alexkozy commented on Nov 29, 2018

    @alexkozy
    Member

    I uploaded patch for V8: CL. It looks good to me but definitely requires accurate review πŸ˜„

  16. 5 remaining items

  17. mhdawson commented on May 15, 2019

    @mhdawson
    Member

    Just waiting on a backport to Node.js 12.x and then we can close

  18. removed
    diag-agendaIssues and PRs to discuss during Diagnostics Working Group meetings.
    on May 15, 2019
  19. alexkozy commented on May 19, 2019

    @alexkozy
    Member

    Could someone approve backport #27720 please? :)

  20. added a commit that references this issue on Aug 2, 2019
  21. added
    feature requestIssues requesting new Node.js features.
    inspectorIssues and PRs related to the V8 inspector protocol.
    on Jun 26, 2020
  22. github-actions commented on Mar 8, 2022

    @github-actions
    Contributor

    There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

    For more information on how the project manages feature requests, please consult the feature request management document.

  23. added
    staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.
    on Mar 8, 2022
  24. github-actions commented on Apr 8, 2022

    @github-actions
    Contributor

    There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

    For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.inspectorIssues and PRs related to the V8 inspector protocol.staleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions