BREAKING(fs): throw Deno.errors.NotFound instead of WalkError in walk[Sync]()

[iuioiua]

What's changed

walk() and walkSync() now naturally throw Deno.errors.NotFound instead of throwing WalkError. The root property has also been removed and the message property no longer includes the normalized path.

Motivation

This change was made to allow these functions to delegate error throwing functionality to the Deno runtime, which provides sufficient context naturally. This also removes a dependency on @std/path/normalize and slightly reduces the API surface area of @std/fs/walk.

Migration guide

To migrate, compare against Deno.errors.NotFound instead of WalkError when error-handling walk().

- import { walk, WalkError } from "@std/fs/walk";
+ import { walk } from "@std/fs/walk";

const root = "../my-dir";

try {
  await Array.fromAsync(
    walk(root),
    async () => await Deno.remove(root, { recursive: true }),
  );
} catch (error) {
- if (error instanceof WalkError) {
+ if (error instanceof Deno.errors.NotFound) {
    // ...
  }
}

Related

Towards #5476

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.40%. Comparing base (2e542ff) to head (3f75c00).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5477      +/-   ##
==========================================
+ Coverage   96.39%   96.40%   +0.01%     
==========================================
  Files         465      465              
  Lines       37757    37727      -30     
  Branches     5579     5574       -5     
==========================================
- Hits        36397    36372      -25     
+ Misses       1318     1313       -5     
  Partials       42       42              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kt3k]

WalkError.path WalkError.root seems to have a usage #3023 (comment)

[iuioiua]

That suggestion was made as a passing remark rather than being triggered from a reported need from a user. After recent work on errors in the codebase, I'm becoming more convinced that throwing naturally is sufficient in almost all cases. If it's good enough for the runtime, I think it's good enough for the Standard Library.

[iuioiua]

Furthermore, Deno.errors.NotFound naturally provides clearer context for the error.

[kt3k]

Furthermore, Deno.errors.NotFound naturally provides clearer context for the error.

WalkError.prototype.root gives the root path of walk. On the other hand, Deno.error.NotFound only shows the exact path, which was not found. The same information is not available with this change.

Also the user needs to parse the path from the error message of Deno.errors.NotFound to get the path. The feels like degraded DX.

[kt3k]

@lowlighter and @lambdalisue are the users of WalkError in open source https://github.com/search?q=WalkError+std%2Ffs&type=code

What do you think about this change?

[kt3k]

Also I think @binyamin uses this in some projects #3023 (comment)

@binyamin Do you still use WalkError in your projects? What do you think about this change?

[lambdalisue]

If the change is clearly mentioned in the release notes, I'm fine with it. In fact, I prefer Deno.errors.NotFound over WalkError because users of walk() can wrap the error themselves if they need root information. I believe that certain responsibilities should lie with the users rather than the providers.

[marvinhagemeister]

+1 on going with Deno.errors.NotFound. It feels more consistent with the rest of Deno to me as other file system APIs throw that too.

[lowlighter]

I concur with what have been already said

[kt3k]

Thanks for the feedbacks/comments!

LGTM