Skip to content
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

System Errors don't seem to have a stack trace #3

Open
phated opened this issue Jan 15, 2018 · 9 comments
Open

System Errors don't seem to have a stack trace #3

phated opened this issue Jan 15, 2018 · 9 comments

Comments

@phated
Copy link
Member

phated commented Jan 15, 2018

In exploring #2 and gulpjs/async-done#45, I found that Errors generated by syscalls (like ENOENT from a fs.stat) don't seem to have a Stack Trace. You can get them to have one by calling Error.captureStackTrace(err) but we don't do that on any of our errors that come from node's fs module.

@erikkemperman @contra @terinjokes do ya'll think we should capture stack traces before forwarding the errors? It's pretty annoying to track down Error: ENOENT: no such file or directory, stat '/Users/phated/test/testcase-vinyl-bug/outbad'

@erikkemperman
Copy link
Member

Traces are good. But it looks like it has to be done for every error instance, so in a lot of places. Would it make sense to try and bolt this onto graceful-fs, behind an option, since we are using that instead of vanilla fs pretty consistently across all modules?

@phated
Copy link
Member Author

phated commented Jan 15, 2018

Not sure that isaacs will accept something like that to graceful-fs but maybe we can try?

@erikkemperman
Copy link
Member

Oh! Didn’t know that was his. Yeah, no, I’m not holding my breath...

@yocontra
Copy link
Member

yocontra commented Jan 16, 2018

@phated graceful-fs is less of a hairball than node-glob so I would be in favor of forking it if the maintainers aren't being responsive.

We could merge any of these in that seem useful as well: https://github.com/isaacs/node-graceful-fs/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc and finally fix a bunch of our longstanding bugs with that project.

Eventually it might be a good idea to fork node-glob as well and finally have independence to solve our own problems 100%.

@phated
Copy link
Member Author

phated commented Jan 17, 2018

I've already had to take on a lot of dependencies to make sure everything was stable back to node 0.10 - I don't want to take on an FS module also 😞

@phated
Copy link
Member Author

phated commented Jan 23, 2018

There's a chance that graceful-fs is losing some error improvements from newer node versions. 😭

@BridgeAR
Copy link

BridgeAR commented Jan 29, 2018

Heads up: In Node.js there is currently work in progress to improve that and in new versions the errors will also contain stack traces.

See nodejs/node#18106 as tracker.
Some errors are already getting improved right now: nodejs/node#17914 and nodejs/node#18348

@coreyfarrell
Copy link
Member

I just ran a test for this, I created issue-3.js:

'use strict';

var fs = require('fs');
var path = require('path');

var outputDirpath = path.resolve('link');

try {
  fs.symlinkSync('not-exist', outputDirpath);
} catch (error) {
  // This shows an EEXIST stack trace on second run
  console.log(error);
}

fs.mkdir(outputDirpath, function(err) {
  // No stack trace here
  console.log(err);
});

fs.stat(outputDirpath, function(err) {
  // No stack trace here
  console.log(err);
});

The above script shows that the lack of stack trace is not caused by graceful-fs or fs-mkdirp-stream. Verified the issue on latest releases of node.js 4/6/8/10/12/14. Using graceful-fs instead of fs in the script yields the exact same results.

@arogg
Copy link

arogg commented Jun 22, 2020

My solution gives very nice stacktraces.

Can someone tell me WHY this works? (because it does)
(fs is fs.promises for me BTW)

for(const name in fs){
    const func = fs[name];
    if (typeof func==='function'){
        fs[name]=async (...args)=>{
            const mystack = new Error().stack;
            try{
                return await func(...args);
            }catch(e){
                e.message+=mystack;
                throw e;
            }
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

6 participants