Core: Retry `writeFile` cache when EBUSY error occurs #32981

reduckted · 2025-11-07T11:49:00Z

What I did

When an EBUSY error occurs in FileSystemCache.set, wait for 100ms and try again. Try up to five times (including the original attempt). If the EBUSY error still occurs, re-throw it.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

This is very difficult for anyone to verify that it solves the original problem from #23131, because that error only seems to happen in certain environments.

If you can repeatedly reproduce the EBUSY errors, then manual testing is quite easy:

Without this change:

Run Storybook and verify that an EBUSY error occurs.
Repeat step 1 a lot to verify that the EBUSY error continues to occur.

With this change:

Run Storybook and verify that no errors occur.
Repeat step 1 a lot to verify that no errors occur.

I also manually tested this change with some logging:

I added a console.log statement when an EBUSY error occurred in the set function.
I added a console.log statement when the cache file is successfully written to after an EBUSY error occurred.

Every time the EBUSY error occurred, the first message was logged. The very next attempt to write (after waiting 100ms) would succeed and the second message would be logged.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

Improvements
- File cache writes are now asynchronous and more resilient to transient file-system busy errors via automatic retry, reducing failed saves and improving responsiveness and stability.
Tests
- Added comprehensive tests covering retry scenarios, timing behavior, and error handling to ensure reliable write operations across success and failure paths.

reduckted · 2025-11-07T11:54:40Z

I've marked this as a draft PR because I'm having problems with the tests. All five new tests pass, but I end up with an uncaught error at the end of the last test. I can't work out how that's happening, because the last test verifies that the promise is rejected. 😕

Hopefully someone can point me in the right direction.

 ✓  storybook  src/common/utils/file-cache.test.ts (5 tests) 15ms
   ✓ FileSystemCache > set > can set values 5ms
   ✓ FileSystemCache > set > does not retry for errors without a code 1ms
   ✓ FileSystemCache > set > does not retry for non-EBUSY errors 1ms
   ✓ FileSystemCache > set > retries for EBUSY errors 2ms
   ✓ FileSystemCache > set > throws when EBUSY errors constantly occur 5ms
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are 
not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: locked
 ❯ core/src/common/utils/file-cache.test.ts:85:35
 ❯ node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
 ❯ node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
 ❯ node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
 ❯ runWithTimeout node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
 ❯ runTest node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12      
 ❯ runSuite node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8      
 ❯ runSuite node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8      
 ❯ runSuite node_modules/vitest/node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8      

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'EBUSY' }
This error originated in "core/src/common/utils/file-cache.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "throws when EBUSY errors constantly occur". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 Test Files  1 passed (1)
      Tests  5 passed (5)
Type Errors  no errors
     Errors  1 error

ndelangen · 2025-11-12T10:16:28Z

code/core/src/common/utils/file-cache.ts

+    for (let attempt = 1; attempt <= SAFE_WRITE_ATTEMPTS; attempt++) {
+      try {
+        await writeFile(fileName, fileData, fileOptions);
+        return;
+      } catch (err) {
+        // If an EBUSY error occurs on any attempt except
+        // the last, then wait for a bit and try again.
+        // https://github.com/storybookjs/storybook/issues/23131
+        if (
+          attempt < SAFE_WRITE_ATTEMPTS &&
+          err instanceof Error &&
+          'code' in err &&
+          err.code === 'EBUSY'
+        ) {
+          await new Promise((resolve) => setTimeout(resolve, 100));
+        } else {
+          throw err;
+        }
+      }
+    }


This retry a write mechanism, seems something that belongs in a common helper function, not implemented in the file-cache mechanism directly.

Do you agree?

Then we can make the unit test purely test the retry-mechanism.

That sounds OK. Did you have a preference for where to create the helper function? Is there an existing file that contains similar helper functions that it could be added to?

I've moved it to code/core/src/common/utils/write-file-with-retry.ts, but feel free to move it elsewhere if there's somewhere better. 😄

nx-cloud · 2025-11-12T10:17:01Z

View your CI Pipeline Execution ↗ for commit 283917b

Command	Status	Duration	Result
`nx run-many -t build --parallel=3`	✅ Succeeded	53s	View ↗

☁️ Nx Cloud last updated this comment at 2025-11-24 09:41:13 UTC

storybook-app-bot · 2025-11-12T11:54:45Z

Package Benchmarks

^{Commit: 283917b, ran on 24 November 2025 at 09:32:54 UTC}

No significant changes detected, all good. 👏

reduckted · 2025-11-13T10:43:20Z

All five new tests pass, but I end up with an uncaught error at the end of the last test. I can't work out how that's happening, because the last test verifies that the promise is rejected.

This appears to be a bug in vitest:
vitest-dev/vitest#9024

ndelangen · 2025-11-13T14:59:46Z

I think there's a bug in the unit test @reduckted

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

code/core/src/common/utils/write-file-with-retry.test.ts (1)
7-14: Critical: Apply the previously suggested fix for mock state persistence.

The past review comment correctly identified the root cause of the unhandled rejection you're seeing after test 5. The fix has not yet been applied:

Line 7 is missing { spy: true }

Line 12-14 beforeEach is missing mock cleanup

As noted in the previous review, test 5 (line 84) uses mockRejectedValue (not ...Once), which makes the mock reject on ALL subsequent calls. Without proper mock cleanup, this state persists and causes the unhandled rejection.

Apply the previously suggested fix:
-vi.mock('node:fs/promises');
+vi.mock('node:fs/promises', { spy: true });
 beforeEach(async () => {
   vi.useFakeTimers();
+  vi.clearAllMocks();
 });

🧹 Nitpick comments (1)

code/core/src/common/utils/write-file-with-retry.test.ts (1)
82-104: Consider using multiple mockRejectedValueOnce calls for safer test isolation.

While the mock cleanup fix will resolve the unhandled rejection, this test could be made more robust by using five mockRejectedValueOnce calls instead of mockRejectedValue:
-    vi.mocked(fs.writeFile).mockRejectedValue(error);
+    vi.mocked(fs.writeFile)
+      .mockRejectedValueOnce(error)
+      .mockRejectedValueOnce(error)
+      .mockRejectedValueOnce(error)
+      .mockRejectedValueOnce(error)
+      .mockRejectedValueOnce(error);
This approach mirrors the pattern in the test at lines 50-55 and ensures the mock behavior is scoped to exactly this test's 5 attempts, making the test more self-contained and less reliant on cleanup.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19c731b and dad75f3.

📒 Files selected for processing (1)

code/core/src/common/utils/write-file-with-retry.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

code/core/src/common/utils/write-file-with-retry.test.ts (1)

code/core/src/common/utils/write-file-with-retry.ts (1)

writeFileWithRetry (7-25)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Core Unit Tests, windows-latest
GitHub Check: normal

🔇 Additional comments (1)

code/core/src/common/utils/write-file-with-retry.test.ts (1)

20-104: Test coverage and logic are solid.

The test suite comprehensively covers all code paths:

Success on first attempt

Non-retryable errors (missing code, non-EBUSY)

EBUSY with eventual success after retries

EBUSY persisting through all attempts

The fake timer assertions correctly validate the 100ms delay and 5-attempt limit. Once the mock cleanup issue is resolved, this test suite will properly validate the retry behavior without unhandled rejections.

reduckted · 2025-11-21T11:02:21Z

Any chance of this making it into 10.1? It looks like the test failures are unrelated and happening in other PRs too.

ndelangen · 2025-11-24T09:33:29Z

@reduckted QA for 10.1 is going on right now.

I'll ping @yannbf and @valentinpalkovic here, to decide if it's something we want to add to the release.

reduckted · 2025-11-24T09:42:19Z

Thank you!

reduckted · 2025-12-03T11:35:19Z

I see the alpha versions of 10.2 have started being released. Any chance of this making it into 10.2?

Retry when EBUSY error occurs while writing to the cache

956c7de

reduckted mentioned this pull request Nov 7, 2025

[Bug]: EBUSY: Resource busy or locked #23131

Closed

Merge branch 'next' into feature/file-system-cache-set-retry

4314a6e

ndelangen self-assigned this Nov 12, 2025

ndelangen added bug core ci:normal labels Nov 12, 2025

ndelangen reviewed Nov 12, 2025

View reviewed changes

Moved write retries to a helper function.

d2f0bec

reduckted marked this pull request as ready for review November 12, 2025 11:40

This comment was marked as spam.

Sign in to view

ndelangen added 2 commits November 12, 2025 13:31

Merge branch 'next' into feature/file-system-cache-set-retry

b89a7e4

Merge branch 'next' into feature/file-system-cache-set-retry

66b19fe

ndelangen changed the title ~~Retry when EBUSY error occurs while writing to the cache~~ Core: Retry writeFile cache when EBUSY error occurs Nov 12, 2025

reduckted mentioned this pull request Nov 13, 2025

Unhandled error after asserting that promise throws an error vitest-dev/vitest#9024

Open

6 tasks

Merge branch 'next' into feature/file-system-cache-set-retry

1dbd1bc

reduckted added 2 commits November 14, 2025 20:35

Fixed bug in test.

1e872a9

Assert that the promise was rejected with the expected error.

19c731b

reduckted requested a review from ndelangen November 15, 2025 01:38

ndelangen added 2 commits November 18, 2025 10:05

Merge branch 'next' into feature/file-system-cache-set-retry

c8e375b

improve mocks

dad75f3

coderabbitai bot reviewed Nov 18, 2025

View reviewed changes

Merge branch 'next' into feature/file-system-cache-set-retry

98f908f

Merge branch 'next' into feature/file-system-cache-set-retry

283917b

ndelangen approved these changes Nov 24, 2025

View reviewed changes

ndelangen merged commit 9e7805b into storybookjs:next Dec 5, 2025
57 of 58 checks passed

reduckted deleted the feature/file-system-cache-set-retry branch December 5, 2025 10:32

github-actions bot mentioned this pull request Dec 5, 2025

Release: Prerelease alpha 10.2.0-alpha.4 #33288

Merged

15 tasks

Uh oh!

Core: Retry writeFile cache when EBUSY error occurs #32981

Core: Retry writeFile cache when EBUSY error occurs #32981

Uh oh!

Conversation

reduckted commented Nov 7, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Summary by CodeRabbit

Uh oh!

reduckted commented Nov 7, 2025

Uh oh!

ndelangen Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

reduckted Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

reduckted Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

nx-cloud bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

Uh oh!

storybook-app-bot bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package Benchmarks

Uh oh!

reduckted commented Nov 13, 2025

Uh oh!

ndelangen commented Nov 13, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

reduckted commented Nov 21, 2025

Uh oh!

ndelangen commented Nov 24, 2025

Uh oh!

reduckted commented Nov 24, 2025

Uh oh!

reduckted commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Core: Retry `writeFile` cache when EBUSY error occurs #32981

Core: Retry `writeFile` cache when EBUSY error occurs #32981

reduckted commented Nov 7, 2025 •

edited by coderabbitai bot

Loading

nx-cloud bot commented Nov 12, 2025 •

edited

Loading

storybook-app-bot bot commented Nov 12, 2025 •

edited

Loading