Skip to content

Commit ce5893e

Browse files
matsjfunkeclaude
andauthored
fix: cancel sandbox proxy iframe timeout on effect cleanup to prevent StrictMode unhandled rejection (#190)
* fix: cancel sandbox proxy iframe timeout on effect cleanup to prevent StrictMode unhandled rejection Co-authored-by: claude <noreply@anthropic.com> * test: add Vitest for setupSandboxProxyIframe timeout cancel path and ready message with jsdom contentWindow mock * fix: resolve onReady inside setupSandboxProxyIframe cancel and consolidate SandboxCancelRef documentation --------- Co-authored-by: claude <noreply@anthropic.com>
1 parent 7bfcd93 commit ce5893e

4 files changed

Lines changed: 138 additions & 5 deletions

File tree

sdks/typescript/client/src/components/AppFrame.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
type McpUiAppCapabilities,
1111
} from '@modelcontextprotocol/ext-apps/app-bridge';
1212

13-
import { setupSandboxProxyIframe } from '../utils/app-host-utils';
13+
import { setupSandboxProxyIframe, type SandboxCancelRef } from '../utils/app-host-utils';
1414

1515
/**
1616
* Build sandbox URL with CSP query parameter for HTTP header-based CSP enforcement.
@@ -165,6 +165,11 @@ export const AppFrame = (props: AppFrameProps) => {
165165
setError(null);
166166

167167
let mounted = true;
168+
// cancelRef is populated synchronously inside setupSandboxProxyIframe's Promise
169+
// constructor, before any async yield. This guarantees it is set by the time
170+
// React Strict Mode fires the effect cleanup — allowing the orphaned timeout
171+
// to be cleared even when cleanup runs before the awaited result resolves.
172+
const cancelRef: SandboxCancelRef = {};
168173

169174
const setup = async () => {
170175
try {
@@ -176,7 +181,7 @@ export const AppFrame = (props: AppFrameProps) => {
176181
currentAppBridgeRef.current = null;
177182
}
178183

179-
const { iframe, onReady } = await setupSandboxProxyIframe(sandboxUrl);
184+
const { iframe, onReady } = await setupSandboxProxyIframe(sandboxUrl, cancelRef);
180185

181186
if (!mounted) return;
182187

@@ -237,6 +242,7 @@ export const AppFrame = (props: AppFrameProps) => {
237242

238243
return () => {
239244
mounted = false;
245+
cancelRef.cancel?.();
240246
};
241247
}, [sandbox.url, sandbox.csp, appBridge]);
242248

sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ describe('<AppFrame />', () => {
119119
render(<AppFrame {...getPropsWithBridge()} />);
120120

121121
await waitFor(() => {
122-
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith(defaultProps.sandbox.url);
122+
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith(
123+
defaultProps.sandbox.url,
124+
expect.any(Object),
125+
);
123126
});
124127
});
125128

@@ -279,6 +282,39 @@ describe('<AppFrame />', () => {
279282
});
280283

281284
describe('lifecycle', () => {
285+
it('should cancel sandbox timeout when effect cleanup fires before await resolves (StrictMode)', async () => {
286+
// Simulate the cancelRef being populated synchronously (as the real impl does).
287+
// Capture the cancelRef passed to setupSandboxProxyIframe and verify cancel() is
288+
// called before the mock ever resolves — this is the Strict Mode timing scenario.
289+
let capturedCancelRef: { cancel?: () => void } | undefined;
290+
const cancelSpy = vi.fn();
291+
292+
vi.mocked(appHostUtils.setupSandboxProxyIframe).mockImplementation(
293+
async (_url, cancelRef) => {
294+
capturedCancelRef = cancelRef;
295+
// Synchronously populate the cancelRef, mirroring the real implementation
296+
if (cancelRef) {
297+
cancelRef.cancel = cancelSpy;
298+
}
299+
// Return a promise that never resolves, simulating a slow sandbox
300+
return new Promise(() => {});
301+
},
302+
);
303+
304+
const { unmount } = render(<AppFrame {...getPropsWithBridge()} />);
305+
306+
// Flush microtasks so the mock implementation runs
307+
await act(async () => {});
308+
309+
expect(capturedCancelRef).toBeDefined();
310+
311+
// Unmount (simulates Strict Mode cleanup between effect invocations)
312+
unmount();
313+
314+
// cancel() must have been called to clear the orphaned timeout
315+
expect(cancelSpy).toHaveBeenCalledTimes(1);
316+
});
317+
282318
it('should preserve iframe across re-renders', async () => {
283319
const { rerender } = render(<AppFrame {...getPropsWithBridge()} />);
284320

@@ -329,7 +365,10 @@ describe('<AppFrame />', () => {
329365
// Should call setupSandboxProxyIframe again with new URL
330366
await waitFor(() => {
331367
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledTimes(2);
332-
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenLastCalledWith(newSandboxUrl);
368+
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenLastCalledWith(
369+
newSandboxUrl,
370+
expect.any(Object),
371+
);
333372
});
334373
});
335374

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
3+
import { SANDBOX_PROXY_READY_METHOD } from '@modelcontextprotocol/ext-apps/app-bridge';
4+
5+
import { setupSandboxProxyIframe, type SandboxCancelRef } from './app-host-utils';
6+
7+
describe('setupSandboxProxyIframe', () => {
8+
beforeEach(() => {
9+
vi.useFakeTimers();
10+
});
11+
12+
afterEach(() => {
13+
vi.useRealTimers();
14+
});
15+
16+
it('rejects onReady when sandbox never becomes ready (timeout)', async () => {
17+
const cancelRef: SandboxCancelRef = {};
18+
const { onReady } = await setupSandboxProxyIframe(
19+
new URL('https://example.com/proxy'),
20+
cancelRef,
21+
);
22+
23+
const assertion = expect(onReady).rejects.toThrow(
24+
/Timed out waiting for sandbox proxy iframe to be ready/,
25+
);
26+
await vi.advanceTimersByTimeAsync(10_000);
27+
await assertion;
28+
});
29+
30+
it('resolves onReady when cancel is called before the sandbox handshake completes', async () => {
31+
const cancelRef: SandboxCancelRef = {};
32+
const { onReady } = await setupSandboxProxyIframe(
33+
new URL('https://example.com/proxy'),
34+
cancelRef,
35+
);
36+
37+
expect(cancelRef.cancel).toBeTypeOf('function');
38+
cancelRef.cancel!();
39+
40+
await expect(onReady).resolves.toBeUndefined();
41+
42+
await vi.advanceTimersByTimeAsync(10_000);
43+
});
44+
45+
it('resolves onReady when the sandbox posts the ready message', async () => {
46+
const { iframe, onReady } = await setupSandboxProxyIframe(new URL('https://example.com/proxy'));
47+
48+
// jsdom does not attach contentWindow to programmatic iframes; the handler matches
49+
// event.source === iframe.contentWindow, so expose a stable mock window reference.
50+
const sandboxWindow = {} as Window;
51+
Object.defineProperty(iframe, 'contentWindow', {
52+
get: () => sandboxWindow,
53+
configurable: true,
54+
});
55+
56+
window.dispatchEvent(
57+
new MessageEvent('message', {
58+
source: sandboxWindow,
59+
data: { method: SANDBOX_PROXY_READY_METHOD },
60+
}),
61+
);
62+
63+
await expect(onReady).resolves.toBeUndefined();
64+
});
65+
});

sdks/typescript/client/src/utils/app-host-utils.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,19 @@ import { type Client } from '@modelcontextprotocol/sdk/client/index.js';
77
import { type Tool } from '@modelcontextprotocol/sdk/types.js';
88
const DEFAULT_SANDBOX_TIMEOUT_MS = 10000;
99

10-
export async function setupSandboxProxyIframe(sandboxProxyUrl: URL): Promise<{
10+
/**
11+
* Assigned synchronously inside the onReady executor so callers can invoke
12+
* `cancel()` while still awaiting `setupSandboxProxyIframe` or `onReady`.
13+
* Cancelling clears timers and listeners and resolves `onReady`.
14+
*/
15+
export interface SandboxCancelRef {
16+
cancel?: () => void;
17+
}
18+
19+
export async function setupSandboxProxyIframe(
20+
sandboxProxyUrl: URL,
21+
cancelRef?: SandboxCancelRef,
22+
): Promise<{
1123
iframe: HTMLIFrameElement;
1224
onReady: Promise<void>;
1325
}> {
@@ -34,6 +46,17 @@ export async function setupSandboxProxyIframe(sandboxProxyUrl: URL): Promise<{
3446
}
3547
}, DEFAULT_SANDBOX_TIMEOUT_MS);
3648

49+
if (cancelRef) {
50+
cancelRef.cancel = () => {
51+
if (!settled) {
52+
settled = true;
53+
clearTimeout(timeoutId);
54+
cleanup();
55+
resolve();
56+
}
57+
};
58+
}
59+
3760
const messageListener = (event: MessageEvent) => {
3861
if (event.source === iframe.contentWindow) {
3962
if (event.data && event.data.method === SANDBOX_PROXY_READY_METHOD) {

0 commit comments

Comments
 (0)