Code Review

This pull request introduces an EXDEV fallback mechanism to DiskStore to handle cross-device rename failures. When a rename fails with EXDEV, the store can now fallback to writing a temporary file in the target file's own directory before renaming it, ensuring an atomic replacement. This behavior is configurable via the new fallback and fallbackTmpfileName options. The changes also include comprehensive unit tests. The review feedback highlights a critical issue where using a static, shared fallbackTmpfileName for concurrent writes in the same directory can lead to race conditions and data corruption. It is recommended to append a unique identifier, such as a UUID, to the custom fallback file name to prevent these conflicts.

test/index.test.js (1)

142-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using assert.rejects for cleaner async error testing.

Both sites use a verbose try/catch block combined with an intentional assert(false) to validate that a promise rejects. Consider using assert.rejects for a more idiomatic and concise approach to testing async errors.

• test/index.test.js#L142-L147: Simplify the rejection check for the fallback rename error.
• test/index.test.js#L162-L167: Simplify the rejection check for the EXDEV error.

💡 Proposed refactors

For test/index.test.js#L142-L147:

-      try {
-        await diskStore.set('exdev-fail/a', 'v');
-        assert(false, 'should not run here');
-      } catch (err) {
-        assert(err.message === 'mock fallback rename error');
-      }
+      await assert.rejects(
+        diskStore.set('exdev-fail/a', 'v'),
+        err => err.message === 'mock fallback rename error'
+      );

For test/index.test.js#L162-L167:

-      try {
-        await store.set('exdev-off', 'v');
-        assert(false, 'should not run here');
-      } catch (err) {
-        assert(err.code === 'EXDEV');
-      }
+      await assert.rejects(
+        store.set('exdev-off', 'v'),
+        err => err.code === 'EXDEV'
+      );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/index.test.js` around lines 142 - 147, Replace the try/catch plus
intentional assert(false) rejection checks in test/index.test.js at lines
142-147 and 162-167 with assert.rejects assertions. Preserve each test’s
existing promise operation and expected error message: “mock fallback rename
error” at the anchor site and the EXDEV error at the sibling site.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/index.test.js`:
- Around line 142-147: Replace the try/catch plus intentional assert(false)
rejection checks in test/index.test.js at lines 142-147 and 162-167 with
assert.rejects assertions. Preserve each test’s existing promise operation and
expected error message: “mock fallback rename error” at the anchor site and the
EXDEV error at the sibling site.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a8806322-f690-4c4d-9d71-b20815839fa1

Reviewing files that changed from the base of the PR and between fb99247 and 153fd19.

• README.md
• lib/index.js
• test/index.test.js

LGTM