-
Notifications
You must be signed in to change notification settings - Fork 56
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
Suppress "raw" stack? #204
Comments
I see two reasons to have it:
But I agree that 99% of the time you just don't care about the raw stack. I'd rather keep it by default (because of the pedagogic value) but have an option to turn it off. |
I'd still love to see this (and personally, as long as the async stack trace behavior is documented, I don't think raw stack traces needs to be the default just for educational purposes), but quick thought: this is only a really significant issue, I realized, when the error message is multi-line. Streamline thinks every line of the (An example of multi-line messages are Mocha's "diff" assertions, when using assertion libraries like expect.js and comparing objects for loose equality. That's what I meant by "comparing JSON" above.) Is there a way for Streamline to properly handle multi-line messages in the stack trace? E.g. could it look at Thanks Bruno! |
I'll think more about it. Detecting |
@aseemk I did a first pass on it. I've eliminated the duplicate |
I'm not closing yet because I may need to propagate the fix the generators runtime (galaxy). |
Yes, mutli-line I'll still request some improvement here, because this is the kind of heavy duplication I commonly see:
At least with the code I work with, it looks like the async stack is always the raw stack plus the extra/smart async stuff. Is that always the case? If so, can I suggest this instead?
Or alternately, have the async stack simply scrap the raw part (since that's rarely useful in my experience — yours too?):
WDYT? |
@aseemk I looks more like a bug in the way the async stack is generated. Can you send me a repro. I'd like to fix it before publishing to npm. |
Here you go Bruno:
var Request = require('request');
function foo(_) {
var resp = Request.get({
url: 'http://www.google.com/foo/bar'
}, _);
if (resp.statusCode !== 200) {
throw new Error([
'This', 'is', 'a', 'multi-', 'line', 'error', 'message'
].join('\n'));
}
return true;
}
setTimeout(_, 1);
console.log(foo(_));
This is with the current master Streamline. |
Thanks. I can reproduce. It's really a bug. With this variant: "use strict";
var Request = require('request');
function bar(_, resp) {
if (resp.statusCode !== 200) {
throw new Error([
'This', 'is', 'a', 'multi-', 'line', 'error', 'message'
].join('\n'));
}
}
function foo(_) {
var resp = Request.get({
url: 'http://www.google.com/foo/bar'
}, _);
bar(_, resp);
return true;
}
setTimeout(_, 1);
console.log(foo(_)); I get:
|
@aseemk I just committed a fix. Can you test and let me know if it's ok. I'll publish afterwards. |
Awesome! Did a quick test and it works fantastically. Thank you! |
I'm happy to close this issue then. Now that the two stacks are no longer redundant in our use cases, I definitely see the value of keeping both. |
I love the async stack feature of Streamline.js — one of its most useful, underrated features, I think. It seems to always be a superset of the "raw" stack. Is there any reason the "raw" stack is still shown, then?
If there is a reason, perhaps it doesn't apply to most common uses. Could showing or suppressing this raw stack be made into an option? I'd think the default should be to not show it.
It's not a huge deal, but it makes every error stack trace twice as long, which is particularly annoying and noisy with tests where you're comparing JSON etc.
The text was updated successfully, but these errors were encountered: