Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Delay run until breakpoints are restored #34

Merged
merged 5 commits into from
Mar 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion examples/alive.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
'use strict';
let x = 0;
function heartbeat() {
++x;
Expand Down
1 change: 0 additions & 1 deletion examples/backtrace.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
'use strict';
const { exports: moduleScoped } = module;

function topFn(a, b = false) {
Expand Down
4 changes: 2 additions & 2 deletions examples/cjs/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const fourty = 40;
const { add } = require('./other');

const sum = add(40, 2);
const sum = add(fourty, 2);
module.exports = sum;
1 change: 0 additions & 1 deletion examples/cjs/other.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
'use strict';
exports.add = function add(a, b) {
return a + b;
};
1 change: 0 additions & 1 deletion examples/exceptions.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
'use strict';
let error = null;
try {
throw new Error('Caught');
Expand Down
1 change: 0 additions & 1 deletion examples/three-lines.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
'use strict';
let x = 1;
x = x + 1;
module.exports = x;
2 changes: 2 additions & 0 deletions examples/use-strict.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use strict';
console.log('first real line');
65 changes: 58 additions & 7 deletions lib/_inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
'use strict';
const { spawn } = require('child_process');
const { EventEmitter } = require('events');
const net = require('net');
const util = require('util');

const runAsStandalone = typeof __dirname !== 'undefined';
Expand Down Expand Up @@ -90,6 +91,45 @@ function createAgentProxy(domain, client) {
});
}

function portIsFree(host, port, timeout = 2000) {
const retryDelay = 150;
let didTimeOut = false;

return new Promise((resolve, reject) => {
setTimeout(() => {
didTimeOut = true;
reject(new Error(
`Timeout (${timeout}) waiting for ${host}:${port} to be free`));
}, timeout);

function pingPort() {
if (didTimeOut) return;

const socket = net.connect(port, host);
let didRetry = false;
function retry() {
if (!didRetry && !didTimeOut) {
didRetry = true;
setTimeout(pingPort, retryDelay);
}
}

socket.on('error', (error) => {
if (error.code === 'ECONNREFUSED') {
resolve();
} else {
retry();
}
});
socket.on('connect', () => {
socket.destroy();
retry();
});
}
pingPort();
});
}

class NodeInspector {
constructor(options, stdin, stdout) {
this.options = options;
Expand Down Expand Up @@ -130,8 +170,9 @@ class NodeInspector {
process.once('SIGHUP', process.exit.bind(process, 0));

this.run()
.then(() => {
this.repl = startRepl();
.then(() => startRepl())
.then((repl) => {
this.repl = repl;
this.repl.on('exit', () => {
process.exit(0);
});
Expand All @@ -141,15 +182,19 @@ class NodeInspector {
}

suspendReplWhile(fn) {
this.repl.rli.pause();
if (this.repl) {
this.repl.rli.pause();
}
this.stdin.pause();
this.paused = true;
return new Promise((resolve) => {
resolve(fn());
}).then(() => {
this.paused = false;
this.repl.rli.resume();
this.repl.displayPrompt();
if (this.repl) {
this.repl.rli.resume();
this.repl.displayPrompt();
}
this.stdin.resume();
}).then(null, (error) => process.nextTick(() => { throw error; }));
}
Expand All @@ -164,7 +209,14 @@ class NodeInspector {

run() {
this.killChild();
return this._runScript().then((child) => {
const { host, port } = this.options;

const runOncePortIsFree = () => {
return portIsFree(host, port)
.then(() => this._runScript());
};

return runOncePortIsFree().then((child) => {
this.child = child;

let connectionAttempts = 0;
Expand All @@ -189,7 +241,6 @@ class NodeInspector {
});
};

const { host, port } = this.options;
this.print(`connecting to ${host}:${port} ..`, true);
return attemptConnect();
});
Expand Down
15 changes: 1 addition & 14 deletions lib/internal/inspect_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,20 +334,7 @@ class Client extends EventEmitter {
this.emit('close');
});

Promise.all([
this.callMethod('Runtime.enable'),
this.callMethod('Debugger.enable'),
this.callMethod('Debugger.setPauseOnExceptions', { state: 'none' }),
this.callMethod('Debugger.setAsyncCallStackDepth', { maxDepth: 0 }),
this.callMethod('Profiler.enable'),
this.callMethod('Profiler.setSamplingInterval', { interval: 100 }),
this.callMethod('Debugger.setBlackboxPatterns', { patterns: [] }),
this.callMethod('Runtime.runIfWaitingForDebugger'),
]).then(() => {
this.emit('ready');
}, (error) => {
this.emit('error', error);
});
this.emit('ready');
};

return new Promise((resolve, reject) => {
Expand Down
47 changes: 34 additions & 13 deletions lib/internal/inspect_repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -748,8 +748,8 @@ function createRepl(inspector) {
.filter(({ location }) => !!location.scriptUrl)
.map(({ location }) =>
setBreakpoint(location.scriptUrl, location.lineNumber + 1));
if (!newBreakpoints.length) return;
Promise.all(newBreakpoints).then((results) => {
if (!newBreakpoints.length) return Promise.resolve();
return Promise.all(newBreakpoints).then((results) => {
print(`${results.length} breakpoints restored.`);
});
}
Expand All @@ -770,16 +770,19 @@ function createRepl(inspector) {
const breakType = reason === 'other' ? 'break' : reason;
const script = knownScripts[scriptId];
const scriptUrl = script ? getRelativePath(script.url) : '[unknown]';
print(`${breakType} in ${scriptUrl}:${lineNumber + 1}`);

const header = `${breakType} in ${scriptUrl}:${lineNumber + 1}`;

inspector.suspendReplWhile(() =>
Promise.all([formatWatchers(true), selectedFrame.list(2)])
.then(([watcherList, context]) => {
if (watcherList) {
return `${watcherList}\n${inspect(context)}`;
}
return context;
}).then(print));
return inspect(context);
}).then((breakContext) => {
print(`${header}\n${breakContext}`);
}));
});

function handleResumed() {
Expand Down Expand Up @@ -1026,7 +1029,30 @@ function createRepl(inspector) {
aliasProperties(context, SHORTCUTS);
}

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);
}

return function startRepl() {
inspector.client.on('close', () => {
resetOnStart();
});
inspector.client.on('ready', () => {
initAfterStart();
});

const replOptions = {
prompt: 'debug> ',
input: inspector.stdin,
Expand All @@ -1035,6 +1061,7 @@ function createRepl(inspector) {
useGlobal: false,
ignoreUndefined: true,
};

repl = Repl.start(replOptions); // eslint-disable-line prefer-const
initializeContext(repl.context);
repl.on('reset', initializeContext);
Expand All @@ -1044,14 +1071,8 @@ function createRepl(inspector) {
repl.rli.emit('SIGINT');
});

inspector.client.on('close', () => {
resetOnStart();
});

inspector.client.on('ready', () => {
restoreBreakpoints();
Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });
});
// Init once for the initial connection
initAfterStart();

return repl;
};
Expand Down
4 changes: 2 additions & 2 deletions test/cli/backtrace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ test('display and navigate backtrace', (t) => {
.then(() => cli.stepCommand('c'))
.then(() => cli.command('bt'))
.then(() => {
t.match(cli.output, `#0 topFn ${script}:8:2`);
t.match(cli.output, `#0 topFn ${script}:7:2`);
})
.then(() => cli.command('backtrace'))
.then(() => {
t.match(cli.output, `#0 topFn ${script}:8:2`);
t.match(cli.output, `#0 topFn ${script}:7:2`);
})
.then(() => cli.quit())
.then(null, onFatal);
Expand Down
6 changes: 3 additions & 3 deletions test/cli/exceptions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ test('break on (uncaught) exceptions', (t) => {
.then(() => cli.command('breakOnException'))
.then(() => cli.stepCommand('c'))
.then(() => {
t.match(cli.output, `exception in ${script}:4`);
t.match(cli.output, `exception in ${script}:3`);
})
.then(() => cli.stepCommand('c'))
.then(() => {
t.match(cli.output, `exception in ${script}:10`);
t.match(cli.output, `exception in ${script}:9`);
})

// Next run: With `breakOnUncaught` it only pauses on the 2nd exception
Expand All @@ -46,7 +46,7 @@ test('break on (uncaught) exceptions', (t) => {
})
.then(() => cli.stepCommand('c'))
.then(() => {
t.match(cli.output, `exception in ${script}:10`);
t.match(cli.output, `exception in ${script}:9`);
})

// Next run: Back to the initial state! It should die again.
Expand Down
4 changes: 3 additions & 1 deletion test/cli/launch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ const startCLI = require('./start-cli');
test('examples/empty.js', (t) => {
const script = Path.join('examples', 'empty.js');
const cli = startCLI([script]);
return cli.waitForPrompt()

return cli.waitFor(/break/)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches how all other test files are doing it. There really should be a cli.waitForInitialPrompt helper.

.then(() => cli.waitForPrompt())
.then(() => {
t.match(cli.output, 'debug>', 'prints a prompt');
t.match(
Expand Down
13 changes: 11 additions & 2 deletions test/cli/preserve-breaks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,17 @@ test('run after quit / restart', (t) => {
})
.then(() => cli.command('breakpoints'))
.then(() => {
t.match(cli.output, `#0 ${script}:2`);
t.match(cli.output, `#1 ${script}:3`);
if (process.platform === 'aix') {
// TODO: There is a known issue on AIX where the breakpoints aren't
// properly resolved yet when we reach this point.
// Eventually that should be figured out but for now we don't want
// to fail builds because of it.
t.match(cli.output, /#0 [^\n]+three-lines\.js\$?:2/);
t.match(cli.output, /#1 [^\n]+three-lines\.js\$?:3/);
} else {
t.match(cli.output, `#0 ${script}:2`);
t.match(cli.output, `#1 ${script}:3`);
}
})
.then(() => cli.quit())
.then(null, onFatal);
Expand Down
27 changes: 27 additions & 0 deletions test/cli/use-strict.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
const Path = require('path');

const { test } = require('tap');

const startCLI = require('./start-cli');

test('for whiles that starts with strict directive', (t) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this test for documentation purposes and to make sure that adding a strict directive doesn't actually break everything.

const script = Path.join('examples', 'use-strict.js');
const cli = startCLI([script]);

function onFatal(error) {
cli.quit();
throw error;
}

return cli.waitFor(/break/)
.then(() => cli.waitForPrompt())
.then(() => {
t.match(
cli.output,
/break in [^:]+:(?:1|2)[^\d]/,
'pauses either on strict directive or first "real" line');
})
.then(() => cli.quit())
.then(null, onFatal);
});