Skip to content

Commit 06d309d

Browse files
fix(driver-utils): prevent unhandled rejections in prefetch (#26151)
## Summary - Fix unhandled promise rejections in `PrefetchDocumentStorageService` when parallel prefetch requests fail - Add unit tests for prefetch error handling and caching behavior ## Problem The `PrefetchDocumentStorageService` fires prefetch requests using fire-and-forget pattern (`void this.cachedRead(blob)`). When these requests failed, the `.catch()` handler was re-throwing errors, creating rejected promises stored in cache that no one awaited - causing `uncaughtException` errors. Evidence from telemetry showed 98 `uncaughtException` errors from a single process within ~70ms, all originating from `readBlob` via prefetch. ## Solution Store the original promise in the cache and attach `.catch()` for side effects only (cache cleanup on retryable errors). Do not re-throw - callers who await the cached promise still receive the rejection properly. Also added a race-condition fix to avoid clearing a newer cached promise when an older one fails. ## Test plan - [x] New unit tests for `PrefetchDocumentStorageService` (4 tests covering error propagation, retry behavior, prefetch caching, and fire-and-forget failure handling) - [x] driver-utils tests pass (60 tests) - [x] Lint passes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: anthony-murphy <[email protected]> fixes [AB#47951](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/47951) --------- Co-authored-by: anthony-murphy-agent <[email protected]> Co-authored-by: anthony-murphy <[email protected]>
1 parent fb50906 commit 06d309d

File tree

2 files changed

+174
-7
lines changed

2 files changed

+174
-7
lines changed

packages/loader/driver-utils/src/prefetchDocumentStorageService.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,20 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy
5757
return prefetchedBlobP;
5858
}
5959
const prefetchedBlobPFromStorage = this.internalStorageService.readBlob(blobId);
60-
this.prefetchCache.set(
61-
blobId,
62-
prefetchedBlobPFromStorage.catch((error) => {
63-
if (canRetryOnError(error)) {
60+
// Attach error handler for side effects only:
61+
// 1. Clear cache on retryable errors so next read retries
62+
// 2. Prevent unhandled rejection warning for fire-and-forget prefetch
63+
// Note: Callers who await the cached promise will still see the rejection
64+
prefetchedBlobPFromStorage.catch((error) => {
65+
if (canRetryOnError(error)) {
66+
// Only clear cache if our promise is still the cached one
67+
// (avoids race condition with concurrent requests)
68+
if (this.prefetchCache.get(blobId) === prefetchedBlobPFromStorage) {
6469
this.prefetchCache.delete(blobId);
6570
}
66-
throw error;
67-
}),
68-
);
71+
}
72+
});
73+
this.prefetchCache.set(blobId, prefetchedBlobPFromStorage);
6974
return prefetchedBlobPFromStorage;
7075
}
7176
return this.internalStorageService.readBlob(blobId);
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*!
2+
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
3+
* Licensed under the MIT License.
4+
*/
5+
6+
import { strict as assert } from "assert";
7+
8+
import type {
9+
IDocumentStorageService,
10+
ISnapshotTree,
11+
} from "@fluidframework/driver-definitions/internal";
12+
13+
import { PrefetchDocumentStorageService } from "../prefetchDocumentStorageService.js";
14+
15+
/**
16+
* Helper to wait for a condition with timeout
17+
*/
18+
async function waitForCondition(
19+
condition: () => boolean,
20+
timeoutMs: number = 1000,
21+
intervalMs: number = 5,
22+
): Promise<void> {
23+
const startTime = Date.now();
24+
while (!condition()) {
25+
if (Date.now() - startTime > timeoutMs) {
26+
throw new Error("Condition not met within timeout");
27+
}
28+
await new Promise((resolve) => setTimeout(resolve, intervalMs));
29+
}
30+
}
31+
32+
/**
33+
* Mock storage service for testing
34+
*/
35+
class MockStorageService implements Partial<IDocumentStorageService> {
36+
public readBlobCalls: string[] = [];
37+
public shouldFail = false;
38+
public failureError = new Error("Mock read failure");
39+
40+
public async readBlob(blobId: string): Promise<ArrayBufferLike> {
41+
this.readBlobCalls.push(blobId);
42+
if (this.shouldFail) {
43+
throw this.failureError;
44+
}
45+
return new Uint8Array([1, 2, 3]).buffer;
46+
}
47+
48+
public async getSnapshotTree(): Promise<ISnapshotTree | null> {
49+
return {
50+
blobs: {
51+
".metadata": "blob1",
52+
"header": "blob2",
53+
"quorumMembers": "blob3",
54+
"other": "blob4",
55+
},
56+
trees: {},
57+
};
58+
}
59+
60+
public get policies() {
61+
return undefined;
62+
}
63+
}
64+
65+
describe("PrefetchDocumentStorageService", () => {
66+
let mockStorage: MockStorageService;
67+
let prefetchService: PrefetchDocumentStorageService;
68+
69+
beforeEach(() => {
70+
mockStorage = new MockStorageService();
71+
prefetchService = new PrefetchDocumentStorageService(
72+
mockStorage as unknown as IDocumentStorageService,
73+
);
74+
});
75+
76+
afterEach(() => {
77+
prefetchService.stopPrefetch();
78+
});
79+
80+
it("should propagate errors to callers who await readBlob", async () => {
81+
mockStorage.shouldFail = true;
82+
const testError = new Error("Network failure");
83+
mockStorage.failureError = testError;
84+
85+
// Direct readBlob call should receive the error
86+
await assert.rejects(
87+
async () => prefetchService.readBlob("someBlob"),
88+
(error: Error) => error.message === "Network failure",
89+
);
90+
});
91+
92+
it("should clear cache on retryable errors allowing retry", async () => {
93+
const retryableError = new Error("Retryable error");
94+
(retryableError as any).canRetry = true;
95+
mockStorage.failureError = retryableError;
96+
mockStorage.shouldFail = true;
97+
98+
// First call fails
99+
await assert.rejects(async () => prefetchService.readBlob("blob1"));
100+
101+
// Reset mock to succeed
102+
mockStorage.shouldFail = false;
103+
mockStorage.readBlobCalls = [];
104+
105+
// Second call should retry (not use cached error)
106+
const result = await prefetchService.readBlob("blob1");
107+
assert.strictEqual(result.byteLength, 3, "Should return blob data after retry");
108+
assert.strictEqual(
109+
mockStorage.readBlobCalls.length,
110+
1,
111+
"Should perform exactly one new underlying read after cache is cleared",
112+
);
113+
});
114+
115+
it("should successfully prefetch blobs", async () => {
116+
// Trigger prefetch
117+
await prefetchService.getSnapshotTree();
118+
119+
// Wait for prefetch to complete using polling instead of fixed timeout
120+
await waitForCondition(() => mockStorage.readBlobCalls.length > 0);
121+
122+
// Verify blobs were prefetched
123+
assert.ok(
124+
mockStorage.readBlobCalls.length > 0,
125+
"Prefetch should have triggered blob reads",
126+
);
127+
128+
// Clear call tracking
129+
mockStorage.readBlobCalls = [];
130+
131+
// Reading a prefetched blob should use cache (no new call)
132+
await prefetchService.readBlob("blob1");
133+
assert.strictEqual(mockStorage.readBlobCalls.length, 0, "Should use cached prefetch");
134+
});
135+
136+
it("should not cause unhandled rejections on fire-and-forget prefetch failures", async () => {
137+
// Set up to fail all blob reads
138+
const prefetchError = new Error("Prefetch network failure");
139+
(prefetchError as any).canRetry = true;
140+
mockStorage.failureError = prefetchError;
141+
mockStorage.shouldFail = true;
142+
143+
// Trigger prefetch via getSnapshotTree (fire-and-forget pattern)
144+
// The prefetch will fail, but should NOT cause unhandled rejection
145+
await prefetchService.getSnapshotTree();
146+
147+
// Wait for prefetch attempts to occur
148+
await waitForCondition(() => mockStorage.readBlobCalls.length > 0);
149+
150+
// Allow microtask queue to flush (for catch handlers to execute)
151+
await Promise.resolve();
152+
153+
// If we reach here without unhandled rejection, the test passes
154+
// Now verify that explicit readBlob calls still receive the error properly
155+
mockStorage.readBlobCalls = [];
156+
await assert.rejects(
157+
async () => prefetchService.readBlob("blob1"),
158+
(error: Error) => error.message === "Prefetch network failure",
159+
"Explicit readBlob should still receive the error",
160+
);
161+
});
162+
});

0 commit comments

Comments
 (0)