-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Not helpful error message after upgrading Node #8379
Comments
Agree, shall/can we suggest a |
+1. This seems like a simple thing that could go a long way for people new to Node. |
I think I've suggested making one of these errors friendlier before, but definitely +1 |
Big +1 ... in fact, I would argue that there shouldn't be an error or stack trace here at all... just a helpful message printed and an exit with non-zero exit code. |
I think I've seen modules ( |
Sigh... that would mean deprecating an error message. That makes me sad... but ok. Perhaps we document the pending change in behavior but hold off on landing it until v8 then? |
I’d still like to see some way of determining where the offending |
Why not keep it an error and just change the message? Anyone parsing error messages shall rot in hell :) |
Hmm.. ok, perhaps a couple of things:
Would need to be a semver-major, of course |
Item 1 is trivial. Figuring out how to trim the stack trace is a bit more complicated. Perhaps someone from @nodejs/v8 could help :-) |
Have a PR started here: #8391 |
Why are we trimming the stack trace? |
To cut down on the noise by removing the internal bits... e.g. to turn:
into
But with a better error message as in #8391. I don't think it's critical, btw, it's just something that would make these internal errors a bit easier for a user to digest. |
I'd say if we start trimming stack trace (which is a useful feature imho, even if just optional), we should do so consistently everywhere. Maybe it warrants a separate issue? |
Yep.. likely a good idea to separate that out. |
Also, there are other error messages in here that could use some improvement. |
To customize/trim stack traces you can use this API: https://github.com/v8/v8/wiki/Stack-Trace-API#customizing-stack-traces |
Thank you Ali. That's helpful. This appears to be limited to the js API, On Friday, September 2, 2016, Ali Ijaz Sheikh [email protected]
|
@jasnell I don't think |
Yeah, given that this is module loader code I'll have to benchmark that and make sure it's not too much of a perf hit. Will play around with it. Thanks @ofrobots ! |
BTW, I personally don't think we should be abbreviating the stack. The main pain point here was that the error message could have been more helpful. Is anyone looking at why there was a segfault? I don't think that is expected behaviour? |
I haven't been able to recreate the segfault in any way. |
@jasnell I have been using nothing but nodegit. (Regarding the segfault) |
@martinheidegger would you be able to provide repro instructions? Or if are familiar with lldb, a backtrace. |
Fixes: #8379 PR-URL: #8391 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
I just upgraded Node from 5.11.1 to 6.5.0 (on latest MacOSX) and then started my node project again.
My project uses nodegit (with c-bindings). When starting the project I get this error message:
Obviously this error is caused by upgrading node and can be fixed quite easily by reinstalling the npm packages (post build scripts etc.). But for an inexperienced user this looks like something horrible just happened, particularly the "segmentation fault"-bit at the end.
It would be awesome if the error could be a little bit friendlier (and helpful)
The text was updated successfully, but these errors were encountered: