Skip to content

lib: add chainErrors API to avoid error swallowing#37460

Merged
aduh95 merged 1 commit intonodejs:masterfrom
aduh95:fs-close-swallow
Mar 19, 2021
Merged

lib: add chainErrors API to avoid error swallowing#37460
aduh95 merged 1 commit intonodejs:masterfrom
aduh95:fs-close-swallow

Conversation

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Feb 20, 2021

I think the current behaviour is wrong:

  • it waits for the file to be closed, but there's no need for this, the error could be passed directly to the user.
  • It swallows the error on closing. Failing to close a file is fairly rare in practice, but I think it would make sense to use the default behaviour added in fs: use a default callback for fs.close() #37174 (rethrow the error if one occurs).

Conservatively marking this as semver-major as it might raise an error where it didn't before.
EDIT: I've repurposed this PR to aggregate errors instead.

/cc @nodejs/fs

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fs Issues and PRs related to the fs subsystem / file system. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants