-
Notifications
You must be signed in to change notification settings - Fork 13
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
Incompatibility with bluebird long stack traces - thoughts on workarounds? #12
Comments
What about appending the 'at ' only when BLUEBIRD_DEBUG is set? That would leave the behavior untouched for most people but would still work better when bluebird is used? I haven't seen this problem, but I also have stopped using Bluebird/Q since the newer versions of node have async/await support. |
That'd work for me. Sorry for the spam - forgot to rebase first. Regarding async/await - it still seems like some people might continue to use bluebird along with async/await for the convenience functions that native promises don't support - e.g. timeouts, tap, etc. Just realized I didn't add any documentation to point out this new behavior if either of the environment variables are set. I'll add a commit in a sec. |
… stack traces chomping
… stack traces chomping
I just hit the same issue. I haven't looked into how either library is implemented, but if you say that Bluebird removes parts of the stack trace, would it be better to fix it on their end? |
@zommerfelds Initially, I looked into fixing the issue on Bluebird's side, but I felt conflicted about what the right approach was. While Bluebird is making some strict assumptions about the stack and doing some manipulation around those assumptions, I don't feel like their assumptions are necessarily bad. Appending newlines and strings to a stack is probably a bit unusual. Not bad in this case (as they are very helpful), but it certainly doesn't fit the shape of a normal stack that gets generated when an error is created. I considered adding an option/env variable to bluebird, but then users of nested-error-stacks would have to know about the issue and would have to go out of their way to opt into the option. Not to mention - getting a change into bluebird and/or getting everyone on board with such an option might be difficult since it'd effectively be an option for just this library (or maybe a small handful of others). I'm open to alternative ideas, though. |
When enabling bluebird's long stack traces, the appended
Caused By...
gets removed and a confusing/misleading stacktrace is created. The resulting stacktrace is the first part of the nested error stacktrace plus the first few lines of theCaused By:
stacktrace appended with no indication of start/stop of stacktraces (plus theFrom previous event:
trace after). Likewise, if the error has been nested multiple times, all of the nested (Caused By:
) stacks get chopped/appending to the first stack.https://github.com/petkaantonov/bluebird/blob/fdd44ee3af6d4fc2570a0f877ca82805a477c6b8/src/debuggability.js#L554 is to blame. I was considering adding a bluebird option to opt out of it, but was curious if this was something you had encountered and worked around previously. The regex being used is
/^\s*at\s*/
, so prependingat
i.e.at Caused by: ...
is another possible workaround.After changing how stacks are combined:
stack += '\nat Caused By: ' + nested.stack;
, the result is:note: the shorter stacks are due to bluebird automatically trimming common ancestors, internal trace lines, etc, but those lines aren't very useful anyway.
FWIW, this isn't a problem with Q's longStackSupport.
I can open a PR to prepend
at
, but I assume that would lead to a major version bump and might not be the preferred approach. Getting an opt-out merged into bluebird and released would probably take a while. Do you have a preference @mdlavin, or any other alternative ideas?The text was updated successfully, but these errors were encountered: