fix(fs): no root path in walk error

[Nautigsam]

This adds the root path to error object. I am not sure this is the way to go, maybe I should put it in the message?

Fix #863

[CLAassistant]

All committers have signed the CLA.

[kt3k]

I think we should show the path in error message because otherwise it doesn't show up when the user didn't catch it. I also think we should show the full path which caused the error, instead of the root path.

[Nautigsam]

@kt3k I put the path in the error message.

I also think we should show the full path which caused the error, instead of the root path.

Do you mean if the root path is a relative path we should make it absolute by joining it with Deno.cwd() ?

[kt3k]

@Nautigsam
I mean we should print more granular path which caused the actual permission error. For example when walking from './foo' and './foo/bar.txt' caused the permission error, I think the error message should be something like Error: Could not read the path /abs/path/to/foo/bar.txt.

I think that's possible by moving the try-catch clause to nearer place to the actual fs API calls.

[Nautigsam]

I think that's possible by moving the try-catch clause to nearer place to the actual fs API calls.

@kt3k I don't think it is possible. The error is fired by Deno.readDir in walk() function. I put my try-catch right around it.

[kt3k]

@Nautigsam Ok. Let's show root path in the error message. BTW I think it might be better to surround only Deno.readDir() call with try-catch to be specific about what is being caught here.

let files
try {
  files = Deno.readDir(root);
} catch (e) {
  throw new Error("Couldn't read...");
}
for await (const entry of files) { ... }

[Nautigsam]

Ok I'll do it. One last question: I don't know if there is some kind of convention about composing error messages? I chose to make it more natural by appending ... at path X at the end of the message:

Uncaught Permission Denied: ... at path X

We could as well put it at the start but it feels more robotic when it's thrown:

Uncaught At path X: Permission Denied: ...

[kt3k]

Ah, I missed your latest update. The error message now looks good to me!

[Nautigsam]

I could narrow the try-catch arount readDirSync but not readDir. Since it returns an AsyncIterable, the error is thrown by the for await..of. If you think it's okay then I'm done.

[kt3k]

@Nautigsam LGTM. Thank you for your contribution!