Skip to content

Conversation

@isaacs
Copy link
Contributor

@isaacs isaacs commented Sep 10, 2017

Generating a call stack when using this library can trigger a call to
fs.readFileSync. That call can sometimes fail, resulting in a very
confusing stack trace (or none at all, which is even more confusing).

This patch wraps calls to fs.readFileSync() in a try/catch. If the read
fails, then the file contents are omitted, but this is far better than
total failure.

Tracking down this one was fun, as you can probably imagine :)

Generating a call stack when using this library can trigger a call to
fs.readFileSync.  That call can sometimes fail, resulting in a very
confusing stack trace (or none at all, which is even more confusing).

This patch wraps calls to fs.readFileSync() in a try/catch.  If the read
fails, then the file contents are omitted, but this is far better than
total failure.
@LinusU
Copy link
Collaborator

LinusU commented Sep 10, 2017

Tracking down this one was fun, as you can probably imagine :)

Hehe, sorry 🙈

Thanks for the patch!

@LinusU LinusU merged commit dda2727 into evanw:master Sep 10, 2017
@LinusU
Copy link
Collaborator

LinusU commented Sep 10, 2017

Released as 0.4.18 🎉

@isaacs
Copy link
Contributor Author

isaacs commented Sep 11, 2017

Thanks for the swift release!

It's kind of an interesting story, really, and nothing to apologize for. I found this while writing unit tests for a module that reads files, so I was triggering fs.read errors (by monkey-patching) in order to exercise the error handling code. But when tap went to print the error, this module got called to update the error stack before I could un-monkeypatch fs.read, resulting in a really deep yak shave. console.trace was also affected, so when I added that to try to figure out what was calling into it, it printed out [object Object].

Fun things happen when you write tests that intentionally break random things :)

@LinusU
Copy link
Collaborator

LinusU commented Sep 11, 2017

Haha, that is amazing 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants