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

test: improve logged errors #31425

Closed
Closed
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
17 changes: 10 additions & 7 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,22 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process._rawDebug();
throw new Error(`same id added to destroy list twice (${id})`);
}
destroyListList[id] = new Error().stack;
destroyListList[id] = util.inspect(new Error());
Copy link
Member

Choose a reason for hiding this comment

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

What's an example of a situation where this will result in a better stack?

Copy link
Member Author

Choose a reason for hiding this comment

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

Node.js core frames are visualized in grey. It makes it easier to distinguish the test code from core code.

_queueDestroyAsyncId(id);
};

require('async_hooks').createHook({
init(id, ty, tr, r) {
init(id, ty, tr, resource) {
if (initHandles[id]) {
process._rawDebug(
`Is same resource: ${r === initHandles[id].resource}`);
`Is same resource: ${resource === initHandles[id].resource}`);
process._rawDebug(`Previous stack:\n${initHandles[id].stack}\n`);
throw new Error(`init called twice for same id (${id})`);
}
initHandles[id] = { resource: r, stack: new Error().stack.substr(6) };
initHandles[id] = {
resource,
stack: util.inspect(new Error()).substr(6)
};
},
before() { },
after() { },
Expand All @@ -161,7 +164,7 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
process._rawDebug();
throw new Error(`destroy called for same id (${id})`);
}
destroydIdsList[id] = new Error().stack;
destroydIdsList[id] = util.inspect(new Error());
},
}).enable();
}
Expand Down Expand Up @@ -345,7 +348,7 @@ function _mustCallInner(fn, criteria = 1, field) {
const context = {
[field]: criteria,
actual: 0,
stack: (new Error()).stack,
stack: util.inspect(new Error()),
name: fn.name || '<anonymous>'
};

Expand Down Expand Up @@ -530,7 +533,7 @@ function expectWarning(nameOrMap, expected, code) {
function expectsError(validator, exact) {
return mustCall((...args) => {
if (args.length !== 1) {
// Do not use `assert.strictEqual()` to prevent `util.inspect` from
// Do not use `assert.strictEqual()` to prevent `inspect` from
// always being called.
assert.fail(`Expected one argument, got ${util.inspect(args)}`);
}
Expand Down
3 changes: 2 additions & 1 deletion test/common/wpt.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const fs = require('fs');
const fsPromises = fs.promises;
const path = require('path');
const vm = require('vm');
const { inspect } = require('util');

// https://github.com/w3c/testharness.js/blob/master/testharness.js
// TODO: get rid of this half-baked harness in favor of the one
Expand Down Expand Up @@ -354,7 +355,7 @@ class WPTRunner {
this.fail(filename, {
name: '',
message: err.message,
stack: err.stack
stack: inspect(err)
}, 'UNCAUGHT');
this.inProgress.delete(filename);
}
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/uncaught-exceptions/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const domain = require('domain');

var d = domain.create();
d.on('error', function(err) {
console.log('[ignored]', err.stack);
console.log('[ignored]', err);
});

d.run(function() {
Expand Down
2 changes: 1 addition & 1 deletion test/message/vm_display_runtime_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ console.error('beginning');
try {
vm.runInThisContext('throw new Error("boo!")', { filename: 'test.vm' });
} catch (err) {
console.error(err.stack);
console.error(err);
}

vm.runInThisContext('throw new Error("spooky!")', { filename: 'test.vm' });
Expand Down
2 changes: 1 addition & 1 deletion test/message/vm_display_syntax_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ console.error('beginning');
try {
vm.runInThisContext('var 4;', { filename: 'foo.vm', displayErrors: true });
} catch (err) {
console.error(err.stack);
console.error(err);
}

vm.runInThisContext('var 5;', { filename: 'test.vm' });
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-assert-if-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ assert.ifError(undefined);
} catch (e) {
threw = true;
assert.strictEqual(e.message, 'Missing expected exception.');
assert(!e.stack.includes('throws'), e.stack);
assert(!e.stack.includes('throws'), e);
}
assert(threw);
}
2 changes: 1 addition & 1 deletion test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ assert.throws(
threw = true;
assert.ok(e.message.includes(rangeError.message));
assert.ok(e instanceof assert.AssertionError);
assert.ok(!e.stack.includes('doesNotThrow'), e.stack);
assert.ok(!e.stack.includes('doesNotThrow'), e);
}
assert.ok(threw);
}
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-request-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ server.listen(0, common.mustCall(function() {
res.resume();
server.close();
})).on('error', function(e) {
console.error(e.stack);
console.error(e);
process.exit(1);
});
}));
2 changes: 1 addition & 1 deletion test/parallel/test-http-unix-socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ server.listen(common.PIPE, common.mustCall(function() {
}));

req.on('error', function(e) {
assert.fail(e.stack);
assert.fail(e);
});

req.end();
Expand Down
11 changes: 6 additions & 5 deletions test/parallel/test-promises-unhandled-rejections.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const common = require('../common');
const assert = require('assert');
const domain = require('domain');
const { inspect } = require('util');

common.disableCrashOnUnhandledRejection();

Expand All @@ -14,8 +15,8 @@ const asyncTest = (function() {

function fail(error) {
const stack = currentTest ?
`${error.stack}\nFrom previous event:\n${currentTest.stack}` :
error.stack;
`${inspect(error)}\nFrom previous event:\n${currentTest.stack}` :
inspect(error);

if (currentTest)
process.stderr.write(`'${currentTest.description}' failed\n\n`);
Expand Down Expand Up @@ -44,11 +45,11 @@ const asyncTest = (function() {
}

return function asyncTest(description, fn) {
const stack = new Error().stack.split('\n').slice(1).join('\n');
const stack = inspect(new Error()).split('\n').slice(1).join('\n');
asyncTestQueue.push({
action: fn,
stack: stack,
description: description
stack,
description
});
if (!asyncTestsEnabled) {
asyncTestsEnabled = true;
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-min-max-version.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
const { inspect } = require('util');

// Check min/max protocol versions.

Expand All @@ -16,7 +17,7 @@ function test(cmin, cmax, cprot, smin, smax, sprot, proto, cerr, serr) {
// Report where test was called from. Strip leading garbage from
// at Object.<anonymous> (file:line)
// from the stack location, we only want the file:line part.
const where = (new Error()).stack.split('\n')[2].replace(/[^(]*/, '');
const where = inspect(new Error()).split('\n')[2].replace(/[^(]*/, '');
connect({
client: {
checkServerIdentity: (servername, cert) => { },
Expand Down
6 changes: 1 addition & 5 deletions test/parallel/test-tls-securepair-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const key = fixtures.readKey('rsa_private.pem');
const cert = fixtures.readKey('rsa_cert.crt');

function log(a) {
console.error(`***server*** ${a}`);
console.error('***server***', a);
}

const server = net.createServer(common.mustCall(function(socket) {
Expand Down Expand Up @@ -74,21 +74,18 @@ const server = net.createServer(common.mustCall(function(socket) {
pair.cleartext.on('error', function(err) {
log('got error: ');
log(err);
log(err.stack);
socket.destroy();
});

pair.encrypted.on('error', function(err) {
log('encrypted error: ');
log(err);
log(err.stack);
socket.destroy();
});

socket.on('error', function(err) {
log('socket error: ');
log(err);
log(err.stack);
socket.destroy();
});

Expand All @@ -99,7 +96,6 @@ const server = net.createServer(common.mustCall(function(socket) {
pair.on('error', function(err) {
log('secure error: ');
log(err);
log(err.stack);
socket.destroy();
});
}));
Expand Down
7 changes: 5 additions & 2 deletions test/parallel/test-tls-set-ciphers.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto) common.skip('missing crypto');
if (!common.hasCrypto)
common.skip('missing crypto');

const fixtures = require('../common/fixtures');
const { inspect } = require('util');

// Test cipher: option for TLS.

Expand All @@ -12,7 +15,7 @@ const {

function test(cciphers, sciphers, cipher, cerr, serr) {
assert(cipher || cerr || serr, 'test missing any expectations');
const where = (new Error()).stack.split('\n')[2].replace(/[^(]*/, '');
const where = inspect(new Error()).split('\n')[2].replace(/[^(]*/, '');
connect({
client: {
checkServerIdentity: (servername, cert) => { },
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-vm-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
'use strict';
require('../common');
const assert = require('assert');

const { inspect } = require('util');
const vm = require('vm');
const Script = vm.Script;
let script = new Script('"passed";');
Expand Down Expand Up @@ -59,7 +59,7 @@ try {
} catch (e) {
gh1140Exception = e;
assert.ok(/expected-filename/.test(e.stack),
`expected appearance of filename in Error stack: ${e.stack}`);
`expected appearance of filename in Error stack: ${inspect(e)}`);
}
// This is outside of catch block to confirm catch block ran.
assert.strictEqual(gh1140Exception.toString(), 'Error');
Expand Down