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

async is taking over my stack traces #405

Closed
ronkorving opened this issue Nov 9, 2013 · 7 comments
Closed

async is taking over my stack traces #405

ronkorving opened this issue Nov 9, 2013 · 7 comments

Comments

@ronkorving
Copy link

Don't get me wrong, I love this project. But whenever I look at a stack trace where async is involved, all I see is async async async, and every once in a while something relevant to my actual code base. I can appreciate that certain things have been implemented with elegant helper functions inside of async, but it creates a horrible penalty on the call stack.

Some things to think about:

  • This makes debugging my code harder (I sometimes have to manually set Error.stackTraceLimit to overcome this).
  • It gets people closer to the stack overflow problems I see mentioned so often in the issue tracker here.
  • It may penalize performance, and for this type of library that is never a good thing.

Shouldn't async get out of the way as much as possible? I think the footprint is simply too big.

@Volox
Copy link

Volox commented Nov 13, 2013

+1

1 similar comment
@bjornstar
Copy link

👍

@rlidwka
Copy link

rlidwka commented Nov 14, 2013

-1

Guys, it isn't worth it to increase complexity of async library (and very likely add bugs) just because Error doesn't filter out all this stuff.

This makes debugging my code harder (I sometimes have to manually set Error.stackTraceLimit to overcome this).

If you care about it, override Error handler just like coffee-script does and remove those lines.

It gets people closer to the stack overflow problems I see mentioned so often in the issue tracker here.

Use process.setImmediate for your functions. By the way, async library can do it by itself. Anyway, it has nothing to do with function calls.

It may penalize performance, and for this type of library that is never a good thing.

No it's not, because v8 inlines small functions.

@ronkorving
Copy link
Author

setImmediate will ruin my stack even more by simply dropping more.

Other than that I can only say, you're right, let's make it the user's problem.

@rlidwka
Copy link

rlidwka commented Nov 14, 2013

setImmediate will ruin my stack even more by simply dropping more.

It's clearing the stack and allowing other IO stuff to slip in if you're doing computationally-intensive stuff. Usually a good thing.

Other than that I can only say, you're right, let's make it the user's problem.

The fact that async is calling multiple functions is not an issue. The issue is that Error prints those functions in the stack trace. Therefore all I'm saying is that it is not this library's issue.

Maybe there are libraries out there that fix this behaviour.

@petkaantonov
Copy link

Function smallness is only one out of dozen or so requirements for inlining, likely nothing gets inlined in async library due to heavy use of closures, especially if you are using 64-bit node.

@caolan caolan closed this as completed Mar 28, 2014
@ZacharyRSmith
Copy link

ZacharyRSmith commented Mar 10, 2018

I created async-error-stack to solve:

Problem 1: By default, err stack is limited to 10 lines.

Problem 2: the event loop's stack frame context not maintained.

Problem 3: the async module cluttering up the stack.

Problem 2 is seen in this:

const async = require('async');

async.series({
    one: function (callback) {
        setTimeout(function () {
            callback(null, 1);
        }, 200);
    },
    two: function (callback) {
        setTimeout(function () {
            callback(new Error('BOOM!'));
        }, 100);
    }
}, function (err, results) {
    if (err) return void console.error(err);
    /*
    Error: BOOM!
        at Timeout._onTimeout (/Users/zacharyryansmith/Code/sandbox/index.js:26:22)
        at ontimeout (timers.js:475:11)
        at tryOnTimeout (timers.js:310:5)
        at Timer.listOnTimeout (timers.js:270:5)
    */
});

async-error-stack uses longjohn to solve problems 2 and 3, and own code to solve 1. Log after using async-error-stack:

STACK FILTERED BY !line.includes('node_modules/async/')
Error: BOOM!
    at Timeout.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:35:22)
    at ontimeout (timers.js:475:11)
    at tryOnTimeout (timers.js:310:5)
    at Timer.listOnTimeout (timers.js:270:5)
---------------------------------------------
    at two (/Users/zacharyryansmith/Code/sandbox/index.js:34:9)
    at Timeout.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:30:13)
    at ontimeout (timers.js:475:11)
---------------------------------------------
    at one (/Users/zacharyryansmith/Code/sandbox/index.js:29:9)
    at Object.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:27:7)
---------------------------------------------
FULL STACK
---------------------------------------------
Error: BOOM!
    at Timeout.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:35:22)
    at ontimeout (timers.js:475:11)
    at tryOnTimeout (timers.js:310:5)
    at Timer.listOnTimeout (timers.js:270:5)
---------------------------------------------
    at two (/Users/zacharyryansmith/Code/sandbox/index.js:34:9)
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:3866:24
    at replenish (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:998:17)
    at iterateeCallback (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:983:17)
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:958:16
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:3871:13
    at Timeout.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:30:13)
    at ontimeout (timers.js:475:11)
---------------------------------------------
    at one (/Users/zacharyryansmith/Code/sandbox/index.js:29:9)
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:3866:24
    at replenish (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:998:17)
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:1002:9
    at eachOfLimit (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:1027:24)
    at /Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:1032:16
    at _parallel (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:3865:5)
    at Object.series (/Users/zacharyryansmith/Code/sandbox/node_modules/async/dist/async.js:4721:5)
    at Object.<anonymous> (/Users/zacharyryansmith/Code/sandbox/index.js:27:7)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants