fix(server): retry idempotent sandbox root preparation
Retry transient transport failures while ensuring a work-folder root exists. Preserve bounded attempts, directory confinement, and single-attempt handling for other mutations. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
6444ddb87e
commit
97231edb81
|
|
@ -1,7 +1,7 @@
|
|||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { mkdtemp, mkdir, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { mkdtemp, mkdir, readFile, realpath, rm, stat, symlink, writeFile } from "node:fs/promises";
|
||||
import { Readable } from "node:stream";
|
||||
import { createHash } from "node:crypto";
|
||||
import os from "node:os";
|
||||
|
|
@ -116,6 +116,64 @@ describe("sandbox work folder transport with real Node and Git", () => {
|
|||
await expect(retrying.scan("/home/daytona/task")).rejects.toThrow("symlink_not_allowed");
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
it.each(["before", "after"])("recovers a root-directory transport failure %s creation without replacing files", async (when) => {
|
||||
const dir = path.join(await root(), "task");
|
||||
let failed = false;
|
||||
const execute = vi.fn(async (input: Parameters<typeof localTestWorkFolderRunner.execute>[0]) => {
|
||||
if (!failed) {
|
||||
failed = true;
|
||||
if (when === "after") {
|
||||
expect((await localTestWorkFolderRunner.execute(input)).exitCode).toBe(0);
|
||||
await writeFile(path.join(dir, "retained"), "unsaved work", { mode: 0o751 });
|
||||
}
|
||||
throw Object.assign(new Error("Request failed with status code 502: Sandbox command requested here"), {
|
||||
name: "JsonRpcCallError", code: -32002,
|
||||
});
|
||||
}
|
||||
return localTestWorkFolderRunner.execute(input);
|
||||
});
|
||||
await workFolderTransport({ execute }).mkdirRoot(dir);
|
||||
expect(execute).toHaveBeenCalledTimes(2);
|
||||
expect(execute.mock.calls[1]![0].args).toEqual(execute.mock.calls[0]![0].args);
|
||||
expect((await stat(dir)).isDirectory()).toBe(true);
|
||||
if (when === "after") {
|
||||
expect(await readFile(path.join(dir, "retained"), "utf8")).toBe("unsaved work");
|
||||
expect((await stat(path.join(dir, "retained"))).mode & 0o777).toBe(0o751);
|
||||
}
|
||||
});
|
||||
it("revalidates a root replaced by a symlink after a lost creation response", async () => {
|
||||
const dir = path.join(await root(), "task"), outside = await root();
|
||||
await writeFile(path.join(outside, "private"), "untouched");
|
||||
let failed = false;
|
||||
const execute = vi.fn(async (input: Parameters<typeof localTestWorkFolderRunner.execute>[0]) => {
|
||||
const result = await localTestWorkFolderRunner.execute(input);
|
||||
if (!failed) {
|
||||
failed = true;
|
||||
expect(result.exitCode).toBe(0);
|
||||
await rm(dir, { recursive: true });
|
||||
await symlink(outside, dir);
|
||||
throw Object.assign(new Error("upstream unavailable"), { status: 502 });
|
||||
}
|
||||
return result;
|
||||
});
|
||||
await expect(workFolderTransport({ execute }).mkdirRoot(dir)).rejects.toThrow("symlink_not_allowed");
|
||||
expect(execute).toHaveBeenCalledTimes(2);
|
||||
expect(await readFile(path.join(outside, "private"), "utf8")).toBe("untouched");
|
||||
});
|
||||
it("bounds root-directory retries and keeps non-idempotent mutations single-attempt", async () => {
|
||||
const execute = vi.fn().mockRejectedValue(Object.assign(new Error("upstream unavailable"), { status: 503 }));
|
||||
const retrying = workFolderTransport({ execute });
|
||||
await expect(retrying.mkdirRoot("/home/daytona/task")).rejects.toThrow("upstream unavailable");
|
||||
expect(execute).toHaveBeenCalledTimes(3);
|
||||
for (const mutate of [() => retrying.moveRoot("/old", "/new"), () => retrying.remove("/root", "file")]) {
|
||||
execute.mockClear();
|
||||
await expect(mutate()).rejects.toThrow("upstream unavailable");
|
||||
expect(execute).toHaveBeenCalledOnce();
|
||||
}
|
||||
execute.mockReset().mockResolvedValue({ exitCode: 1, stdout: "", stderr: "symlink_not_allowed", timedOut: false });
|
||||
await expect(retrying.mkdirRoot("/home/daytona/task")).rejects.toThrow("symlink_not_allowed");
|
||||
expect(execute).toHaveBeenCalledOnce();
|
||||
});
|
||||
it("streams and atomically publishes files larger than a transfer chunk", async () => {
|
||||
const dir = await root();
|
||||
const staging = await root();
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ export function workFolderTransport(runner: CommandManagedRuntimeRunner) {
|
|||
span.set({ bytes: Buffer.byteLength(encoded), inputBytes: stdin === undefined ? 0 : Buffer.byteLength(stdin) });
|
||||
return ["--input-type=module", "-e", await source, encoded];
|
||||
});
|
||||
const readOnly = ["home", "scan", "read", "read-batch", "batch-status"].includes(operation);
|
||||
const retrySafe = ["home", "scan", "read", "read-batch", "batch-status", "mkdir-root"].includes(operation);
|
||||
for (let attempt = 0; ; attempt++) {
|
||||
if (Date.now() >= deadline) throw new Error("Work folder transfer deadline exceeded");
|
||||
let executionReturned = false;
|
||||
|
|
@ -89,9 +89,12 @@ export function workFolderTransport(runner: CommandManagedRuntimeRunner) {
|
|||
});
|
||||
});
|
||||
} catch (error) {
|
||||
// Mutations may already have happened. Only reads repeat here.
|
||||
// Reads and ensuring a root directory exists are safe after a lost
|
||||
// response. mkdir-root rechecks confinement and leaves existing
|
||||
// directories, permissions, and contents untouched. Other mutations
|
||||
// may already have happened and must not repeat here.
|
||||
const waitMs = 250 * (attempt + 1);
|
||||
if (executionReturned || !readOnly || attempt >= 2 || !transientTransportFailure(error) || Date.now() + waitMs >= deadline) throw error;
|
||||
if (executionReturned || !retrySafe || attempt >= 2 || !transientTransportFailure(error) || Date.now() + waitMs >= deadline) throw error;
|
||||
await measureSandboxOperation("work_folder.transport.backoff", { operation, attempt: attempt + 1, waitMs }, () => delay(waitMs));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue