Improve error messages #123
Conversation
… parent file in the message
|
3 year bump. |
There was a problem hiding this comment.
coming up on 4 years 😬
I like the error message, but I don't think throwing away the original stack trace is better. If the error indicates 'not found' it makes sense, but if there is a bug in the resolve implementation that causes a TypeError for example, having that context is important.
In modern Node.js versions, the 'error' event propagates the stack trace, so we get both the trace from this PR and the trace from the original error. With the below change, the final result is:
Error: Can't walk dependency graph: Cannot find module 'x' from '~/Code/browserify/module-deps'
required by ~/Code/browserify/module-deps/t.js
at ~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:46:17
at process (~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:173:43)
at ondir (~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:188:17)
at load (~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:69:43)
at onex (~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:92:31)
at ~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:22:47
at FSReqCallback.oncomplete (fs.js:170:21)
Emitted 'error' event on Deps instance at:
at ~/Code/browserify/module-deps/index.js:383:25
at onresolve (~/Code/browserify/module-deps/index.js:181:25)
at ~/Code/browserify/module-deps/node_modules/browser-resolve/index.js:265:24
at ~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:46:14
at process (~/Code/browserify/module-deps/node_modules/browser-resolve/node_modules/resolve/lib/async.js:173:43)
[... lines matching original stack trace ...]
at FSReqCallback.oncomplete (fs.js:170:21)
what do you think about that?
|
Hey @goto-bus-stop! Looking at the problem again, I'm not sure it would have made sense to change only this error's stack and let all of the other possible error paths continue to show the original stack. That's without considering what you mentioned about newer versions of Node. With an event trace like you showed, there is absolutely no reason to replace the stack. Thanks for looking at this! I appreciate the solo mission that you've undertaken to maintain this project and sort through the backlog of issues and PRs. I appreciate this because I believe in open source, and I'd would very much like for this project to continue to be maintained and supported to benefit those who still use it. I, however, no longer do. Seeing as this PR and Browserify#1663 both went unacknowleged, and noticing the downward trend in commits in the project, I eventually switched to the excellent Parcel.js. Best of luck! |
|
applied my suggestion to this branch & merged—thanks for the PR! |
|
Thanks for the merge! |
A typical error message (e.g. Browserify) with this module looks like the following:
This has several issues. First, it does not show the file from which the missing module is being required (it does show the name of the missing file/module and the directory from which it is searching). Second, the stack trace doesn't provide a good indication of the source of the modules involved. Users with monolithic build tasks will find this error extremely confusing, as it does not provide a clear indication of what action could have generated this error, why it was generated, where it was generated or how to fix it.
This PR changes the error such that it appears like so:
This error:
Ideally, this error would give the entire traceback (in the form of
required by) from the unresolved module./nonexistantall the way back up the require chain to the entry file, but that is a non-trivial change.60/60 tests passing.