Skip to content

fs: fix fs.promises.mkdir to also report ENOSPC correctly - #56918

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
jackyzha0:jackyzha0/enoent-mkdir-async
Jun 16, 2025
Merged

fs: fix fs.promises.mkdir to also report ENOSPC correctly#56918
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
jackyzha0:jackyzha0/enoent-mkdir-async

Conversation

@jackyzha0

Copy link
Copy Markdown
Contributor

Continuation of #42811 which fixes#42808 for the async case

Happy to try to write a test but left as is for now seeing as the previous PR also didn't: #42811 (review)

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Feb 5, 2025
@codecov

codecovBot commented Feb 5, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.16%. Comparing base (316ac8c) to head (008577c).
Report is 1017 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #56918 +/- ##
==========================================
- Coverage 89.18% 89.16% -0.03% 
==========================================
Files 665 665 Lines 192543 192574 +31 Branches 37045 37085 +40 ==========================================
- Hits 171726 171702 -24 - Misses 13636 13658 +22 - Partials 7181 7214 +33 
Files with missing linesCoverage Δ
src/node_file.cc77.19% <ø> (-0.05%)⬇️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lpincalpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 7, 2025

@lpincalpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 7, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@jackyzha0

Copy link
Copy Markdown
ContributorAuthor

looks like two test failures for the test runner itself, i think its unrelated to the changeset, are those two known to flake?

@jackyzha0

Copy link
Copy Markdown
ContributorAuthor

@addaleax@lpinca sorry for the tag but let me know if theres anything else I need to do here on my end, I can't seem to be able to re-run CI on my own

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65159/

@jackyzha0

Copy link
Copy Markdown
ContributorAuthor

hey! sorry to bump on this again, was also curious about the process of backporting fixes like these to previous Node.js releases (we are currently on v20.9 but can also upgrade to LTS)

@jackyzha0

Copy link
Copy Markdown
ContributorAuthor

@addaleax@lpinca sorry but just a quick bump on the above, we're still seeing this in production and having this be reported properly would be really helpful ❤️

@lpincalpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 16, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 16, 2025
@nodejs-github-bot
nodejs-github-bot merged commit 908f9f1 into nodejs:mainJun 16, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 908f9f1

@lpinca

Copy link
Copy Markdown
Member

Sorry for the long delay @jackyzha0 😞

@jackyzha0

Copy link
Copy Markdown
ContributorAuthor

its ok, just happy it got in and thanks for the review :)

RafaelGSS pushed a commit that referenced this pull request Jun 23, 2025
PR-URL: #56918
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Jul 21, 2025
PR-URL: #56918
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
aduh95 pushed a commit that referenced this pull request Jul 24, 2025
PR-URL: #56918
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.fsIssues and PRs related to the fs subsystem / file system.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.mkdirSync with recursive: true throws ENOENT instead of ENOSPC when disk is full

4 participants

@jackyzha0@nodejs-github-bot@lpinca@addaleax