Skip to content

std/fs: walk not found error is not Deno.errors.NotFound #1310

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

Closed
lucacasonato opened this issue Sep 25, 2021 · 13 comments
Closed

std/fs: walk not found error is not Deno.errors.NotFound #1310

lucacasonato opened this issue Sep 25, 2021 · 13 comments
Labels
bug Something isn't working

Comments

@lucacasonato
Copy link
Member

If you pass a path to walk that does not exist on the fs, the returned promise rejects with an Error, not a Deno.errors.NotFound.

@lucacasonato lucacasonato added the bug Something isn't working label Sep 25, 2021
@getspooky
Copy link
Contributor

I tried to reproduce the bug but it return Deno.errors.NotFound

import { walk } from "https://deno.land/[email protected]/fs/mod.ts";

try {
  for await (const entry of walk("./deno")) {
    console.log(entry.path);
  }
} catch (error) {
  console.log(error instanceof Deno.errors.NotFound);
}

The result:

true

My configuration:

deno 1.14.0 (release, x86_64-unknown-linux-gnu)
v8 9.4.146.15
typescript 4.4.2

@zhangyuannie
Copy link
Contributor

Related #1216 which contains steps to reproduce.

After some investigations, I believe it is due to the same underlying issue: Rust std::fs categorise Not a directory (ENOTDIR) error as Other instead of NotFound. That error then get propagated from lstat/stat op in Deno.

Worth noting that a new NotADirectory is available in Rust on nightly right now which will help with the issue.

So although far form ideal, it is expected that Deno.errors.NotFound is not returned from walk.

@iuioiua
Copy link
Contributor

iuioiua commented Aug 24, 2022

I got the same as @getspooky. Is this still an issue?

@kt3k
Copy link
Member

kt3k commented Aug 26, 2022

Seems like not an issue. Let's close

@kt3k kt3k closed this as completed Aug 26, 2022
@mikeatbuilder
Copy link

mikeatbuilder commented Aug 28, 2022

@kt3k
Copy link
Member

kt3k commented Aug 28, 2022

@mikeatbuilder Thanks for the input. Sounds interesting. I think we should try to remove that workaround in fresh as this was a non issue

@lucacasonato
Copy link
Member Author

@kt3k Have you tried on Linux. This may be fine on macOS, but it's still broken on Linux.

@kt3k
Copy link
Member

kt3k commented Aug 29, 2022

Ok. I'll take a look

@kt3k
Copy link
Member

kt3k commented Aug 29, 2022

It still seems NotFound in ubuntu

ubuntu@ip-172-31-255-98:~$ ls
main.js
ubuntu@ip-172-31-255-98:~$ cat main.js 
import { walk } from "https://deno.land/[email protected]/fs/mod.ts";

try {
  for await (const entry of walk("./deno")) {
    console.log(entry.path);
  }
} catch (error) {
  console.log(error instanceof Deno.errors.NotFound);
}

ubuntu@ip-172-31-255-98:~$ deno -V
deno 1.25.0
ubuntu@ip-172-31-255-98:~$ deno run -A main.js 
true
ubuntu@ip-172-31-255-98:~$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04 LTS"

@kt3k
Copy link
Member

kt3k commented Aug 29, 2022

If I put a file of the same name of walk entrypoint, then I get Error: Not a directory (os error 20), readdir './deno'. This might be the actual problem in the above line in fresh?

@iuioiua
Copy link
Contributor

iuioiua commented May 17, 2023

Is this still an issue on Linux?

@ayame113
Copy link
Contributor

#3054 changed the types of errors thrown. This problem may no longer exist?
(And I feel like denoland/fresh#1078 can be merged.)

@iuioiua
Copy link
Contributor

iuioiua commented May 24, 2023

denoland/fresh#1078 is failing on all platforms. The PR only removes the workaround that pertains to this issue. This issue should be re-opened or a new issue created.

iuioiua added a commit to denoland/fresh that referenced this issue May 27, 2023
The issue in question, denoland/std#1310,
has been marked complete. Looks like the underlying issue has been
fixed. So this just removes the comment/unnecessary code.

---------

Co-authored-by: Asher Gomez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants