Not helpful error message after upgrading Node

[martinheidegger]

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:

Error: Module version mismatch. Expected 48, got 47.
    at Error (native)
    at Object.Module._extensions..node (module.js:583:18)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/project-x/node_modules/nodegit/dist/nodegit.js:11:12)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
> [1]    79902 segmentation fault  node

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)

[silverwind]

Agree, shall/can we suggest a npm rebuild or npm install?

[cjihrig]

+1. This seems like a simple thing that could go a long way for people new to Node.

[Fishrock123]

I think I've suggested making one of these errors friendlier before, but definitely +1

[jasnell]

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.

[richardlau]

I think I've seen modules (bson?) with pure JavaScript fallback if the native module fails to load so we need to be careful if changing the behaviour to no longer throw an error.

[jasnell]

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?

[addaleax]

there shouldn't be an error or stack trace here at all

I’d still like to see some way of determining where the offending require() call comes from, in larger applications that’s not necessarily obvious.

[silverwind]

deprecating an error message

Why not keep it an error and just change the message? Anyone parsing error messages shall rot in hell :)

[jasnell]

Hmm.. ok, perhaps a couple of things:

1. Update the error message to something more friendly;
2. Trim the stack trace so that all of the internal stuff is hidden (anything at require (internal/module.js:20:19) and above)

Would need to be a semver-major, of course

[jasnell]

Item 1 is trivial. Figuring out how to trim the stack trace is a bit more complicated. Perhaps someone from @nodejs/v8 could help :-)

[jasnell]

Have a PR started here: #8391

[cjihrig]

Why are we trimming the stack trace?

[jasnell]

To cut down on the noise by removing the internal bits...

e.g. to turn:

Error: Module version mismatch. Expected 48, got 47.
    at Error (native)
    at Object.Module._extensions..node (module.js:583:18)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/project-x/node_modules/nodegit/dist/nodegit.js:11:12)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)

into

Error: Module version mismatch. Expected 48, got 47.
    at Object.<anonymous> (/project-x/node_modules/nodegit/dist/nodegit.js:11:12)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)

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.

[silverwind]

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?

[jasnell]

Yep.. likely a good idea to separate that out.

[jasnell]

Also, there are other error messages in here that could use some improvement.

[ofrobots]

To customize/trim stack traces you can use this API: https://github.com/v8/v8/wiki/Stack-Trace-API#customizing-stack-traces

[jasnell]

Thank you Ali. That's helpful. This appears to be limited to the js API,
however. Is there something equivalent for the C/C++ API?

On Friday, September 2, 2016, Ali Ijaz Sheikh notifications@github.com
wrote:

To customize/trim stack traces you can use this API:
https://github.com/v8/v8/wiki/Stack-Trace-API#customizing-stack-traces

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#8379 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2edbF_7MBDjUFFWHE2CRTvxGf8OD2ks5qmH4bgaJpZM4Jzq-z
.

[ofrobots]

@jasnell I don't think prepareStackTrace is exposed on the C++ side. Probably the simplest thing would be to catch Error on the JS side and rethrow with a modified stack?

[jasnell]

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 !

[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?

[jasnell]

I haven't been able to recreate the segfault in any way.

[martinheidegger]

@jasnell I have been using nothing but nodegit. (Regarding the segfault)

[ofrobots]

@martinheidegger would you be able to provide repro instructions? Or if are familiar with lldb, a backtrace.