Skip to content

Commit a3eec5d

Browse files
sweetmantechclaude
andauthored
fix: skip expired snapshots and fallback to fresh sandbox (#412)
* fix: skip expired snapshots and fallback to fresh sandbox on failure Accounts with expired Vercel snapshots caused Sandbox.create() to throw "Status code 400 is not ok" with no recovery path. Changes: - Extract getValidSnapshotId to its own lib file (SRP) - Both processCreateSandbox and createSandboxFromSnapshot use it - Add fallback: if snapshot creation fails, create a fresh sandbox - DRY: processCreateSandbox uses createSandboxWithFallback helper so createSandbox({}) is only called once - Add tests for getValidSnapshotId (5 tests) - Add snapshot expiry and fallback tests to existing test files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: DRY createSandbox({}) in createSandboxFromSnapshot Single createSandbox({}) call path instead of duplicating it in both the catch block and the else branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: extract createSandboxWithFallback to own lib file, add error logging - Extract createSandboxWithFallback to lib/sandbox/createSandboxWithFallback.ts (SRP) - processCreateSandbox and createSandboxFromSnapshot use it / follow same pattern - Log snapshot creation errors before falling back to fresh sandbox - Add 4 unit tests for createSandboxWithFallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: DRY createSandboxFromSnapshot via shared createSandboxWithFallback - createSandboxFromSnapshot now delegates to createSandboxWithFallback instead of duplicating snapshot/fresh fallback logic - createSandboxWithFallback returns full SandboxCreateResult + fromSnapshot - Update tests to match new delegation pattern Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: consolidate processCreateSandbox to use createSandboxFromSnapshot processCreateSandbox now delegates sandbox creation to createSandboxFromSnapshot instead of calling getValidSnapshotId, createSandboxWithFallback, and insertAccountSandbox directly. This eliminates the duplicated snapshot lookup + fallback + DB insert logic between the two functions. processCreateSandbox adds only the prompt-triggering behavior on top. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: treat exact-expiry and unparsable timestamps as invalid snapshots Use <= instead of < so snapshots expiring exactly now are treated as expired. Add NaN check so unparsable expires_at values don't slip through as valid. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f010076 commit a3eec5d

9 files changed

+328
-514
lines changed

lib/sandbox/__tests__/createSandboxFromSnapshot.test.ts

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,16 @@ import type { Sandbox } from "@vercel/sandbox";
33

44
import { createSandboxFromSnapshot } from "../createSandboxFromSnapshot";
55

6-
const mockSelectAccountSnapshots = vi.fn();
6+
const mockGetValidSnapshotId = vi.fn();
77
const mockInsertAccountSandbox = vi.fn();
8-
const mockCreateSandbox = vi.fn();
8+
const mockCreateSandboxWithFallback = vi.fn();
99

10-
vi.mock("@/lib/sandbox/createSandbox", () => ({
11-
createSandbox: (...args: unknown[]) => mockCreateSandbox(...args),
10+
vi.mock("@/lib/sandbox/createSandboxWithFallback", () => ({
11+
createSandboxWithFallback: (...args: unknown[]) => mockCreateSandboxWithFallback(...args),
1212
}));
1313

14-
vi.mock("@/lib/supabase/account_snapshots/selectAccountSnapshots", () => ({
15-
selectAccountSnapshots: (...args: unknown[]) => mockSelectAccountSnapshots(...args),
14+
vi.mock("@/lib/sandbox/getValidSnapshotId", () => ({
15+
getValidSnapshotId: (...args: unknown[]) => mockGetValidSnapshotId(...args),
1616
}));
1717

1818
vi.mock("@/lib/supabase/account_sandboxes/insertAccountSandbox", () => ({
@@ -28,43 +28,40 @@ describe("createSandboxFromSnapshot", () => {
2828

2929
beforeEach(() => {
3030
vi.clearAllMocks();
31-
mockCreateSandbox.mockResolvedValue({
31+
mockCreateSandboxWithFallback.mockResolvedValue({
3232
sandbox: mockSandbox,
3333
response: {
3434
sandboxId: "sbx_new",
3535
sandboxStatus: "running",
3636
timeout: 1800000,
3737
createdAt: "2024-01-01T00:00:00.000Z",
3838
},
39+
fromSnapshot: false,
3940
});
4041
mockInsertAccountSandbox.mockResolvedValue({
4142
data: { account_id: "acc_1", sandbox_id: "sbx_new" },
4243
error: null,
4344
});
4445
});
4546

46-
it("creates from snapshot when available", async () => {
47-
mockSelectAccountSnapshots.mockResolvedValue([
48-
{ snapshot_id: "snap_abc", account_id: "acc_1" },
49-
]);
47+
it("passes snapshotId to createSandboxWithFallback", async () => {
48+
mockGetValidSnapshotId.mockResolvedValue("snap_abc");
5049

5150
await createSandboxFromSnapshot("acc_1");
5251

53-
expect(mockCreateSandbox).toHaveBeenCalledWith({
54-
source: { type: "snapshot", snapshotId: "snap_abc" },
55-
});
52+
expect(mockCreateSandboxWithFallback).toHaveBeenCalledWith("snap_abc");
5653
});
5754

58-
it("creates fresh sandbox when no snapshot exists", async () => {
59-
mockSelectAccountSnapshots.mockResolvedValue([]);
55+
it("passes undefined when no snapshot exists", async () => {
56+
mockGetValidSnapshotId.mockResolvedValue(undefined);
6057

6158
await createSandboxFromSnapshot("acc_1");
6259

63-
expect(mockCreateSandbox).toHaveBeenCalledWith({});
60+
expect(mockCreateSandboxWithFallback).toHaveBeenCalledWith(undefined);
6461
});
6562

6663
it("inserts account_sandbox record", async () => {
67-
mockSelectAccountSnapshots.mockResolvedValue([]);
64+
mockGetValidSnapshotId.mockResolvedValue(undefined);
6865

6966
await createSandboxFromSnapshot("acc_1");
7067

@@ -74,18 +71,53 @@ describe("createSandboxFromSnapshot", () => {
7471
});
7572
});
7673

77-
it("returns { sandbox, fromSnapshot: true } when snapshot exists", async () => {
78-
mockSelectAccountSnapshots.mockResolvedValue([
79-
{ snapshot_id: "snap_abc", account_id: "acc_1" },
80-
]);
74+
it("returns { sandbox, fromSnapshot: true } when snapshot used", async () => {
75+
mockGetValidSnapshotId.mockResolvedValue("snap_abc");
76+
mockCreateSandboxWithFallback.mockResolvedValue({
77+
sandbox: mockSandbox,
78+
response: {
79+
sandboxId: "sbx_new",
80+
sandboxStatus: "running",
81+
timeout: 1800000,
82+
createdAt: "2024-01-01T00:00:00.000Z",
83+
},
84+
fromSnapshot: true,
85+
});
8186

8287
const result = await createSandboxFromSnapshot("acc_1");
8388

8489
expect(result).toEqual({ sandbox: mockSandbox, fromSnapshot: true });
8590
});
8691

8792
it("returns { sandbox, fromSnapshot: false } when no snapshot", async () => {
88-
mockSelectAccountSnapshots.mockResolvedValue([]);
93+
mockGetValidSnapshotId.mockResolvedValue(undefined);
94+
95+
const result = await createSandboxFromSnapshot("acc_1");
96+
97+
expect(result).toEqual({ sandbox: mockSandbox, fromSnapshot: false });
98+
});
99+
100+
it("returns { sandbox, fromSnapshot: false } for expired snapshot", async () => {
101+
mockGetValidSnapshotId.mockResolvedValue(undefined);
102+
103+
const result = await createSandboxFromSnapshot("acc_1");
104+
105+
expect(mockCreateSandboxWithFallback).toHaveBeenCalledWith(undefined);
106+
expect(result).toEqual({ sandbox: mockSandbox, fromSnapshot: false });
107+
});
108+
109+
it("returns { sandbox, fromSnapshot: false } when snapshot creation fails", async () => {
110+
mockGetValidSnapshotId.mockResolvedValue("snap_bad");
111+
mockCreateSandboxWithFallback.mockResolvedValue({
112+
sandbox: mockSandbox,
113+
response: {
114+
sandboxId: "sbx_new",
115+
sandboxStatus: "running",
116+
timeout: 1800000,
117+
createdAt: "2024-01-01T00:00:00.000Z",
118+
},
119+
fromSnapshot: false,
120+
});
89121

90122
const result = await createSandboxFromSnapshot("acc_1");
91123

0 commit comments

Comments
 (0)