Purpose of double Debugger.setPauseOnExceptions

Hi

During the init phase: https://github.com/nodejs/node-inspect/blob/master/lib/internal/inspect_repl.js#L1041

  function initAfterStart() {
    const setupTasks = [
      Runtime.enable(),
      Profiler.enable(),
      Profiler.setSamplingInterval({ interval: 100 }),
      Debugger.enable(),
      Debugger.setPauseOnExceptions({ state: 'none' }),
      Debugger.setAsyncCallStackDepth({ maxDepth: 0 }),
      Debugger.setBlackboxPatterns({ patterns: [] }),
      Debugger.setPauseOnExceptions({ state: pauseOnExceptionState }),
      restoreBreakpoints(),
      Runtime.runIfWaitingForDebugger(),
    ];
    return Promise.all(setupTasks);
  }

What's the purpose of doing Debugger.setPauseOnExceptions twice?

Debugger.setPauseOnExceptions({ state: 'none' }),
Debugger.setPauseOnExceptions({ state: pauseOnExceptionState }),

Hm, I don't think this is on purpose. I assume that the first got copied from somewhere and then I later added the 2nd line without looking properly. Nice find! I assume the first one could just be removed.

It looks like pauseOnExceptionState always 'none' in second invocation. In any case from protocol point of view there is no sense to call Debugger.setPauseOnExceptions twice. Implementation of this method just changes a flag inside V8.

@ak239

@ak239 Thanks for confirming!

Btw, we don't reset it before connecting/restarting. So it might not be 'none' when connecting after restart. So the hardcoded 'none' is definitely wrong and shouldn't be there.

I think we should support this case on backend side. When at least one protocol client requests pause on all exceptions - debugger should issue Debugger.paused event for each exception, if at least one client request pause on uncaught exception - issue event for uncaught exception.
I filled bug against V8: issue

Fixed link, edited: https://bugs.chromium.org/p/chromium/issues/detail?id=1246881

New location:

node/lib/internal/debugger/inspect_repl.js

Lines 1125 to 1128 in 4fbe9e5

    
           await Debugger.setPauseOnExceptions({ state: 'none' }); 
        
           await Debugger.setAsyncCallStackDepth({ maxDepth: 0 }); 
        
           await Debugger.setBlackboxPatterns({ patterns: [] }); 
        
           await Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });

The fix here may be to just remove the first call with the hard-coded 'none'. If the tests pass, that can likely just land.

debugger: remove duplicate setPauseOnException call

Navigation Menu

Repository navigation

Purpose of double Debugger.setPauseOnExceptions #41795

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	await Debugger.setPauseOnExceptions({ state: 'none' });
	await Debugger.setAsyncCallStackDepth({ maxDepth: 0 });
	await Debugger.setBlackboxPatterns({ patterns: [] });
	await Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });

Purpose of double Debugger.setPauseOnExceptions #41795

Description

Activity

hybrist commented on Nov 13, 2017

alexkozy commented on Nov 13, 2017

hybrist commented on Nov 13, 2017

alexkozy commented on Nov 13, 2017

hybrist commented on Nov 13, 2017

hybrist commented on Jan 31, 2022

hybrist commented on Jan 31, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions