Skip to content

Commit

Permalink
Include anonymous functions in stacktraces (#1508)
Browse files Browse the repository at this point in the history
Closes #1313.
  • Loading branch information
jugglinmike authored and novemberborn committed Oct 23, 2017
1 parent eebf26e commit c72f4f2
Show file tree
Hide file tree
Showing 17 changed files with 196 additions and 223 deletions.
51 changes: 43 additions & 8 deletions lib/beautify-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ let ignoreStackLines = [];

const avaInternals = /\/ava\/(?:lib\/)?[\w-]+\.js:\d+:\d+\)?$/;
const avaDependencies = /\/node_modules\/(?:bluebird|empower-core|(?:ava\/node_modules\/)?(?:babel-runtime|core-js))\//;
const stackFrameLine = /^.+( \(.+:[0-9]+:[0-9]+\)|:[0-9]+:[0-9]+)$/;

if (!debug.enabled) {
ignoreStackLines = StackUtils.nodeInternals();
Expand All @@ -17,21 +18,55 @@ if (!debug.enabled) {

const stackUtils = new StackUtils({internals: ignoreStackLines});

function extractFrames(stack) {
return stack
.split('\n')
.map(line => line.trim())
.filter(line => stackFrameLine.test(line))
.join('\n');
}

/**
* Given a string value of the format generated for the `stack` property of a
* V8 error object, return a string that contains only stack frame information
* for frames that have relevance to the consumer.
*
* For example, given the following string value:
*
* ```
* Error
* at inner (/home/ava/ex.js:7:12)
* at /home/ava/ex.js:12:5
* at outer (/home/ava/ex.js:13:4)
* at Object.<anonymous> (/home/ava/ex.js:14:3)
* at Module._compile (module.js:570:32)
* at Object.Module._extensions..js (module.js:579:10)
* at Module.load (module.js:487:32)
* at tryModuleLoad (module.js:446:12)
* at Function.Module._load (module.js:438:3)
* at Module.runMain (module.js:604:10)
* ```
*
* ...this function returns the following string value:
*
* ```
* inner (/home/ava/ex.js:7:12)
* /home/ava/ex.js:12:5
* outer (/home/ava/ex.js:13:4)
* Object.<anonymous> (/home/ava/ex.js:14:3)
* ```
*/
module.exports = stack => {
if (!stack) {
return '';
}

stack = extractFrames(stack);
// Workaround for https://github.com/tapjs/stack-utils/issues/14
// TODO: fix it in `stack-utils`
stack = cleanStack(stack);

const title = stack.split('\n')[0];
const lines = stackUtils
.clean(stack)
.split('\n')
.map(x => ` ${x}`)
.join('\n');

return `${title}\n${lines}`;
return stackUtils.clean(stack)
// Remove the trailing newline inserted by the `stack-utils` module
.trim();
};
10 changes: 0 additions & 10 deletions lib/extract-stack.js

This file was deleted.

21 changes: 10 additions & 11 deletions lib/reporters/mini.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const cross = require('figures').cross;
const indentString = require('indent-string');
const ansiEscapes = require('ansi-escapes');
const trimOffNewlines = require('trim-off-newlines');
const extractStack = require('../extract-stack');
const codeExcerpt = require('../code-excerpt');
const colors = require('../colors');
const formatSerializedError = require('./format-serialized-error');
Expand Down Expand Up @@ -207,9 +206,9 @@ class MiniReporter {
}

if (test.error.stack) {
const extracted = extractStack(test.error.stack);
if (extracted.includes('\n')) {
status += '\n' + indentString(colors.errorStack(extracted), 2) + '\n';
const stack = test.error.stack;
if (stack.includes('\n')) {
status += '\n' + indentString(colors.errorStack(stack), 2) + '\n';
}
}

Expand All @@ -228,14 +227,14 @@ class MiniReporter {
status += ' ' + colors.error(cross + ' ' + err.message) + '\n\n';
} else {
const title = err.type === 'rejection' ? 'Unhandled Rejection' : 'Uncaught Exception';
let description = err.stack ? err.stack.trimRight() : JSON.stringify(err);
description = description.split('\n');
const errorTitle = err.name ? description[0] : 'Threw non-error: ' + description[0];
const errorStack = description.slice(1).join('\n');

status += ' ' + colors.title(title) + '\n';
status += ' ' + colors.stack(errorTitle) + '\n';
status += colors.errorStack(errorStack) + '\n\n';

if (err.name) {
status += ' ' + colors.stack(err.summary) + '\n';
status += colors.errorStack(err.stack) + '\n\n';
} else {
status += ' Threw non-error: ' + err.summary + '\n';
}
}
});
}
Expand Down
8 changes: 1 addition & 7 deletions lib/reporters/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@ const format = require('util').format;
const indentString = require('indent-string');
const stripAnsi = require('strip-ansi');
const yaml = require('js-yaml');
const extractStack = require('../extract-stack');

// Parses stack trace and extracts original function name, file name and line
function getSourceFromStack(stack) {
return extractStack(stack).split('\n')[0];
}

function dumpError(error, includeMessage) {
const obj = Object.assign({}, error.object);
Expand All @@ -35,7 +29,7 @@ function dumpError(error, includeMessage) {
}

if (error.stack) {
obj.at = getSourceFromStack(error.stack);
obj.at = error.stack.split('\n')[0];
}

return ` ---\n${indentString(yaml.safeDump(obj).trim(), 4)}\n ...`;
Expand Down
8 changes: 4 additions & 4 deletions lib/reporters/verbose.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const figures = require('figures');
const chalk = require('chalk');
const plur = require('plur');
const trimOffNewlines = require('trim-off-newlines');
const extractStack = require('../extract-stack');
const codeExcerpt = require('../code-excerpt');
const colors = require('../colors');
const formatSerializedError = require('./format-serialized-error');
Expand Down Expand Up @@ -70,6 +69,7 @@ class VerboseReporter {
let output = colors.error(types[err.type] + ':', err.file) + '\n';

if (err.stack) {
output += ' ' + colors.stack(err.title || err.summary) + '\n';
output += ' ' + colors.stack(err.stack) + '\n';
} else {
output += ' ' + colors.stack(JSON.stringify(err)) + '\n';
Expand Down Expand Up @@ -156,9 +156,9 @@ class VerboseReporter {
}

if (test.error.stack) {
const extracted = extractStack(test.error.stack);
if (extracted.includes('\n')) {
output += '\n' + indentString(colors.errorStack(extracted), 2) + '\n';
const stack = test.error.stack;
if (stack.includes('\n')) {
output += '\n' + indentString(colors.errorStack(stack), 2) + '\n';
}
}

Expand Down
9 changes: 7 additions & 2 deletions lib/serialize-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const cleanYamlObject = require('clean-yaml-object');
const StackUtils = require('stack-utils');
const assert = require('./assert');
const beautifyStack = require('./beautify-stack');
const extractStack = require('./extract-stack');

function isAvaAssertionError(source) {
return source instanceof assert.AssertionError;
Expand All @@ -20,7 +19,7 @@ function extractSource(stack) {
return null;
}

const firstStackLine = extractStack(stack).split('\n')[0];
const firstStackLine = stack.split('\n')[0];
return stackUtils.parseLine(firstStackLine);
}
function buildSource(source) {
Expand Down Expand Up @@ -90,5 +89,11 @@ module.exports = error => {
}
}

if (typeof error.stack === 'string') {
retval.summary = error.stack.split('\n')[0];
} else {
retval.summary = JSON.stringify(error);
}

return retval;
};
21 changes: 21 additions & 0 deletions test/beautify-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ const beautifyStack = proxyquire('../lib/beautify-stack', {
}
});

function fooFunc() {
barFunc();
}

function barFunc() {
throw new Error();
}

test('does not strip ava internals and dependencies from stack trace with debug enabled', t => {
const beautify = proxyquire('../lib/beautify-stack', {
debug() {
Expand Down Expand Up @@ -44,3 +52,16 @@ test('returns empty string without any arguments', t => {
t.is(beautifyStack(), '');
t.end();
});

test('beautify stack - removes uninteresting lines', t => {
try {
fooFunc();
} catch (err) {
const stack = beautifyStack(err.stack);
t.match(stack, /fooFunc/);
t.match(stack, /barFunc/);
t.match(err.stack, /Module._compile/);
t.notMatch(stack, /Module\._compile/);
t.end();
}
});
18 changes: 3 additions & 15 deletions test/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,10 @@ test('timeout', t => {
});
});

test('throwing a named function will report the to the console', t => {
execCli('fixture/throw-named-function.js', (err, stdout, stderr) => {
test('include anonymous functions in error reports', t => {
execCli('fixture/error-in-anonymous-function.js', (err, stdout, stderr) => {
t.ok(err);
t.match(stderr, /function fooFn\(\) \{\}/);
// TODO(jamestalmage)
// t.ok(/1 uncaught exception[^s]/.test(stdout));
t.match(stderr, /test\/fixture\/error-in-anonymous-function\.js:4:8/);
t.end();
});
});
Expand Down Expand Up @@ -202,16 +200,6 @@ test('babel require hook only does not apply to source files', t => {
});
});

test('throwing a anonymous function will report the function to the console', t => {
execCli('fixture/throw-anonymous-function.js', (err, stdout, stderr) => {
t.ok(err);
t.match(stderr, /\(\) => \{\}/);
// TODO(jamestalmage)
// t.ok(/1 uncaught exception[^s]/.test(stdout));
t.end();
});
});

test('pkg-conf: defaults', t => {
execCli([], {dirname: 'fixture/pkg-conf/defaults'}, err => {
t.ifError(err);
Expand Down
36 changes: 0 additions & 36 deletions test/extract-stack.js

This file was deleted.

10 changes: 10 additions & 0 deletions test/fixture/error-in-anonymous-function.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import test from '../../';

const getAnonymousFn = () => () => {
throw new Error();
};

test(t => {
getAnonymousFn()();
t.pass();
});
8 changes: 0 additions & 8 deletions test/fixture/throw-anonymous-function.js

This file was deleted.

10 changes: 0 additions & 10 deletions test/fixture/throw-named-function.js

This file was deleted.

23 changes: 23 additions & 0 deletions test/helper/error-from-worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';

const serializeError = require('../../lib/serialize-error');

module.exports = function (err, options) {
options = Object.assign({}, options);

if (options.stack) {
err.stack = options.stack;
}

const serialized = serializeError(err);

if (options.type) {
serialized.type = options.type;
}

if (options.file) {
serialized.file = options.file;
}

return serialized;
};
Loading

0 comments on commit c72f4f2

Please sign in to comment.