fix: retry interrupted work-folder uploads with verified fresh streams
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
7508f1b6a0
commit
e511a1ce93
|
|
@ -224,6 +224,12 @@ success. Optional SDK streaming checksums are disabled to avoid an unhandled
|
|||
digest rejection when a source file changes during transfer. Work-folder SHA-256
|
||||
verification and complete-checkpoint publication remain required. A changing
|
||||
source must fail its save without stopping the application or another run.
|
||||
Scoped-file and repository-blob uploads retry transient network errors and
|
||||
retryable HTTP responses up to three attempts, using the same object key. Each
|
||||
attempt opens a fresh source and verifies its complete size and SHA-256; the
|
||||
previous request and reader must settle before another attempt starts. Changed
|
||||
content, ownership errors, and authentication failures are not retried. Exhausted
|
||||
retries retain the previous complete checkpoint and the recoverable working copy.
|
||||
|
||||
Deletion moves files to recoverable trash. Restore rejects path collisions.
|
||||
Explicit purge and permanent owner deletion schedule object cleanup through a
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { Readable } from "node:stream";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { eq, inArray, sql } from "drizzle-orm";
|
||||
import fs from "node:fs/promises";
|
||||
|
|
@ -347,6 +348,54 @@ describe("shared sandbox work-folder lifecycle", () => {
|
|||
expect((await svc.list(folder)).files.map((file) => file.path)).toContain("private");
|
||||
}, 120_000);
|
||||
|
||||
it("reopens repository blobs after transient PUT failures and retains the previous checkpoint when retries exhaust", async () => {
|
||||
const leaseId = randomUUID();
|
||||
const run = await prepare(path.join(root, "replayed-repository-upload"), leaseId);
|
||||
await run.flush();
|
||||
const filename = path.join(run.primaryRepo, "retry-upload");
|
||||
const content = "replay the entire repository file";
|
||||
await fs.writeFile(filename, content);
|
||||
const targetHash = createHash("sha256").update(content).digest("hex");
|
||||
const put = storage.putObject.bind(storage);
|
||||
const streams: Readable[] = [];
|
||||
let attempts = 0;
|
||||
const recover = vi.spyOn(storage, "putObject").mockImplementation(async (input) => {
|
||||
if (input.objectKey.endsWith("/" + targetHash)) {
|
||||
streams.push(input.body as Readable);
|
||||
if (++attempts === 1) {
|
||||
await (input.body as Readable).iterator({ destroyOnReturn: false }).next();
|
||||
throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" });
|
||||
}
|
||||
}
|
||||
return put(input);
|
||||
});
|
||||
try { await run.flush(); } finally { recover.mockRestore(); }
|
||||
expect(attempts).toBe(2);
|
||||
expect(new Set(streams).size).toBe(2);
|
||||
expect(streams.every((stream) => stream.destroyed)).toBe(true);
|
||||
const bindingId = run.manifest.repositories[0]!.bindingId;
|
||||
const [before] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId));
|
||||
await fs.writeFile(filename, "retain this unsaved edit");
|
||||
const nextHash = createHash("sha256").update("retain this unsaved edit").digest("hex");
|
||||
let failedAttempts = 0;
|
||||
const fail = vi.spyOn(storage, "putObject").mockImplementation(async (input) => {
|
||||
if (input.objectKey.endsWith("/" + nextHash)) {
|
||||
failedAttempts++;
|
||||
for await (const _chunk of input.body as Readable) { /* Lost response after consuming the body. */ }
|
||||
throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" });
|
||||
}
|
||||
return put(input);
|
||||
});
|
||||
try {
|
||||
await expect(run.stop()).rejects.toThrow("socket hang up");
|
||||
expect(failedAttempts).toBe(3);
|
||||
const [after] = await db.select().from(taskRepositoryBindings).where(eq(taskRepositoryBindings.id, bindingId));
|
||||
expect(after!.checkpointKey).toBe(before!.checkpointKey);
|
||||
expect(await fs.readFile(filename, "utf8")).toBe("retain this unsaved edit");
|
||||
expect(await retainUnsavedWorkFolderLease(db, { id: leaseId, companyId })).toBe(true);
|
||||
} finally { fail.mockRestore(); active.splice(active.indexOf(run), 1); }
|
||||
}, 120_000);
|
||||
|
||||
it("does not publish a partial repository checkpoint and recovers a failed final save in a new run", async () => {
|
||||
const leaseId = randomUUID();
|
||||
const run = await prepare(path.join(root, "interrupted-checkpoint"), leaseId);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,109 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import { Readable } from "node:stream";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { uploadWorkFolderObject } from "../services/work-folder-upload.js";
|
||||
|
||||
const data = Buffer.from("verified file contents\n");
|
||||
const reset = () => Object.assign(new Error("socket hang up"), { name: "TimeoutError", code: "ECONNRESET" });
|
||||
const digest = (bytes: Buffer) => createHash("sha256").update(bytes).digest("hex");
|
||||
const input = { objectKey: "company/repository/blob", contentType: "application/octet-stream", contentLength: data.length, sha256: digest(data) };
|
||||
async function bytes(body: Buffer | Readable) {
|
||||
const chunks: Buffer[] = [];
|
||||
for await (const chunk of body) chunks.push(Buffer.from(chunk));
|
||||
return Buffer.concat(chunks);
|
||||
}
|
||||
|
||||
describe("replayable verified work-folder uploads", () => {
|
||||
it.each(["partial", "complete"])("reopens and verifies the whole source after a reset following %s consumption", async (consumption) => {
|
||||
const sources: Readable[] = [], bodies: Readable[] = [];
|
||||
let attempt = 0;
|
||||
const putObject = vi.fn(async ({ body, objectKey }) => {
|
||||
expect(objectKey).toBe(input.objectKey);
|
||||
bodies.push(body);
|
||||
if (++attempt === 1) {
|
||||
if (consumption === "complete") expect(await bytes(body)).toEqual(data);
|
||||
else await body.iterator({ destroyOnReturn: false }).next();
|
||||
throw reset();
|
||||
}
|
||||
expect(await bytes(body)).toEqual(data);
|
||||
});
|
||||
await uploadWorkFolderObject({ putObject }, { ...input, createSource: () => {
|
||||
const stream = Readable.from([data.subarray(0, 3), data.subarray(3)]); sources.push(stream); return stream;
|
||||
} });
|
||||
expect(putObject).toHaveBeenCalledTimes(2);
|
||||
expect(new Set(bodies).size).toBe(2);
|
||||
expect(sources.every((source) => source.destroyed)).toBe(true);
|
||||
expect(bodies.every((body) => body.destroyed)).toBe(true);
|
||||
});
|
||||
|
||||
it("waits for the previous PUT to settle before reopening a failed source", async () => {
|
||||
let release!: () => void;
|
||||
let failed!: () => void;
|
||||
const held = new Promise<void>((resolve) => { release = resolve; });
|
||||
const sourceFailed = new Promise<void>((resolve) => { failed = resolve; });
|
||||
let attempts = 0;
|
||||
const createSource = vi.fn(() => ++attempts === 1
|
||||
? Readable.from((async function* () { yield data.subarray(0, 3); failed(); throw reset(); })())
|
||||
: Readable.from([data]));
|
||||
const putObject = vi.fn(async ({ body }) => {
|
||||
if (attempts === 1) { await held; throw reset(); }
|
||||
expect(await bytes(body)).toEqual(data);
|
||||
});
|
||||
const upload = uploadWorkFolderObject({ putObject }, { ...input, createSource });
|
||||
await sourceFailed;
|
||||
// Longer than the retry delay: an unsettled old request must still own the key.
|
||||
await new Promise((resolve) => setTimeout(resolve, 300));
|
||||
expect(createSource).toHaveBeenCalledTimes(1);
|
||||
release();
|
||||
await upload;
|
||||
expect(createSource).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("stops after three failed attempts and destroys every source", async () => {
|
||||
const sources: Readable[] = [];
|
||||
const putObject = vi.fn(async ({ body }) => { await bytes(body); throw reset(); });
|
||||
await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => {
|
||||
const source = Readable.from([data]); sources.push(source); return source;
|
||||
} })).rejects.toMatchObject({ code: "ECONNRESET" });
|
||||
expect(putObject).toHaveBeenCalledTimes(3);
|
||||
expect(sources.every((source) => source.destroyed)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects a changed source after a reset without publishing or another retry", async () => {
|
||||
let attempt = 0;
|
||||
const putObject = vi.fn(async ({ body }) => { await bytes(body); throw reset(); });
|
||||
await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => Readable.from([
|
||||
++attempt === 1 ? data : Buffer.alloc(data.length, 120),
|
||||
]) })).rejects.toThrow("Work file changed during upload");
|
||||
expect(putObject).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it.each([Buffer.from("short"), Buffer.alloc(data.length + 1), Buffer.alloc(data.length, 120)])("rejects size or hash changes without retrying", async (body) => {
|
||||
const putObject = vi.fn(async ({ body: stream }) => { await bytes(stream); });
|
||||
await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => Readable.from([body]) }))
|
||||
.rejects.toThrow("Work file changed during upload");
|
||||
expect(putObject).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
Object.assign(new Error("denied"), { $metadata: { httpStatusCode: 403 } }),
|
||||
Object.assign(new Error("missing"), { code: "ENOENT" }),
|
||||
new Error("owner was deleted"),
|
||||
])("does not replay a permanent or unclassified error", async (error) => {
|
||||
const source = Readable.from([data]);
|
||||
const putObject = vi.fn(async () => { throw error; });
|
||||
await expect(uploadWorkFolderObject({ putObject }, { ...input, createSource: () => source })).rejects.toBe(error);
|
||||
expect(putObject).toHaveBeenCalledTimes(1);
|
||||
expect(source.destroyed).toBe(true);
|
||||
});
|
||||
|
||||
it("retries a classified temporary HTTP failure and supports empty files", async () => {
|
||||
let attempt = 0;
|
||||
const putObject = vi.fn(async ({ body }) => {
|
||||
expect(await bytes(body)).toEqual(Buffer.alloc(0));
|
||||
if (++attempt === 1) throw Object.assign(new Error("unavailable"), { $metadata: { httpStatusCode: 503 } });
|
||||
});
|
||||
await uploadWorkFolderObject({ putObject }, { ...input, contentLength: 0, sha256: digest(Buffer.alloc(0)), createSource: () => Readable.from([]) });
|
||||
expect(putObject).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
|
@ -97,6 +97,36 @@ describe("durable work folders", () => {
|
|||
await expect(svc.write(f, { path: "large", body: Buffer.from("large"), maxBytes: 2, operationId: "large" })).rejects.toMatchObject({ status: 413 });
|
||||
expect((await svc.list(f)).files).toHaveLength(0);
|
||||
});
|
||||
it("replays a failed spool upload without publishing a receipt or replacing the previous file", async () => {
|
||||
const f = await folder();
|
||||
await svc.write(f, { path: "note", body: Buffer.from("old"), operationId: "old" });
|
||||
const old = await svc.get(f, "note");
|
||||
const put = storage.putObject.bind(storage);
|
||||
const bodies: Readable[] = [];
|
||||
const failed = vi.spyOn(storage, "putObject").mockImplementation(async (input) => {
|
||||
bodies.push(input.body as Readable);
|
||||
for await (const _chunk of input.body as Readable) { /* Consume the uncertain request. */ }
|
||||
throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" });
|
||||
});
|
||||
try {
|
||||
await expect(svc.write(f, { path: "note", body: Buffer.from("new"), operationId: "new" })).rejects.toThrow("socket hang up");
|
||||
expect(failed).toHaveBeenCalledTimes(3);
|
||||
expect(new Set(bodies).size).toBe(3);
|
||||
expect(bodies.every((body) => body.destroyed)).toBe(true);
|
||||
expect((await svc.get(f, "note")).objectKey).toBe(old.objectKey);
|
||||
expect(await textContent(f, "note")).toBe("old");
|
||||
} finally { failed.mockRestore(); }
|
||||
let attempts = 0;
|
||||
const recovered = vi.spyOn(storage, "putObject").mockImplementation(async (input) => {
|
||||
await put(input);
|
||||
if (++attempts === 1) throw Object.assign(new Error("lost response"), { code: "ECONNRESET" });
|
||||
});
|
||||
try {
|
||||
expect(await svc.write(f, { path: "note", body: Buffer.from("new"), operationId: "new" })).toEqual({ applied: true });
|
||||
expect(recovered).toHaveBeenCalledTimes(2);
|
||||
expect(await textContent(f, "note")).toBe("new");
|
||||
} finally { recovered.mockRestore(); }
|
||||
});
|
||||
it("serializes conflicting parent/file creation", async () => {
|
||||
const f = await folder();
|
||||
const results = await Promise.allSettled([
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { WORK_FOLDER_SCOPES, type SandboxWorkFolderManifest, type WorkFolderScop
|
|||
import type { AdapterSandboxExecutionTarget } from "@paperclipai/adapter-utils/execution-target";
|
||||
import type { StorageProvider } from "../storage/types.js";
|
||||
import { loadConfig } from "../config.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { createStorageProviderFromConfig } from "../storage/provider-registry.js";
|
||||
import { resolveDefaultAgentWorkspaceDir } from "../home-paths.js";
|
||||
import { createGitRemoteAuthProvider } from "./git-credentials.js";
|
||||
|
|
@ -319,7 +320,15 @@ export async function prepareSandboxWorkFolders(input: {
|
|||
for (const { binding, root } of bindings) await repositories.checkpoint(binding, root);
|
||||
await saveState("saved");
|
||||
},
|
||||
async onError() { await saveState("failed", "Files could not be saved; the sandbox must be retained for recovery"); },
|
||||
async onError(error) {
|
||||
const failure = error as { name?: unknown; code?: unknown; $metadata?: { httpStatusCode?: unknown } } | null;
|
||||
// Do not log SDK request objects, headers, file contents or credentials.
|
||||
const label = (value: unknown) => typeof value === "string" && /^[A-Za-z0-9_]{1,80}$/.test(value) ? value : null;
|
||||
logger.warn({ runId: input.runId, errorName: label(failure?.name), errorCode: label(failure?.code),
|
||||
httpStatus: typeof failure?.$metadata?.httpStatusCode === "number" ? failure.$metadata.httpStatusCode : null },
|
||||
"Work folder checkpoint failed; retaining sandbox for recovery");
|
||||
await saveState("failed", "Files could not be saved; the sandbox must be retained for recovery");
|
||||
},
|
||||
});
|
||||
let completion: Promise<void> | null = null;
|
||||
function stop(beforeCompletion?: () => Promise<void>) {
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { Readable, Transform } from "node:stream";
|
||||
import { and, eq, inArray, isNull } from "drizzle-orm";
|
||||
import { taskRepositoryBindings, workFolderObjects, type Db } from "@paperclipai/db";
|
||||
import { validateWorkFilePath } from "@paperclipai/shared";
|
||||
import { z } from "zod";
|
||||
import { registerWorkFolderObject } from "./work-folder-garbage.js";
|
||||
import { uploadWorkFolderObject } from "./work-folder-upload.js";
|
||||
import type { StorageProvider } from "../storage/types.js";
|
||||
import type { WorkFolderTransport, WorkTreeEntry } from "./work-folder-transport.js";
|
||||
|
||||
|
|
@ -40,14 +40,9 @@ export function workFolderRepositoryService(db: Db, storage: StorageProvider, tr
|
|||
const objectKey = entry.kind === "file" && !entry.linkTarget ? `${prefix}blobs/${entry.sha256}` : null;
|
||||
if (objectKey && !known.has(objectKey)) await registerWorkFolderObject(db, storage, { objectKey, companyId: binding.companyId, repositoryBindingId: binding.id });
|
||||
if (objectKey && !known.has(objectKey) && !(await storage.headObject({ objectKey })).exists) {
|
||||
const hash = createHash("sha256");
|
||||
const verify = new Transform({ transform(chunk: Buffer, _encoding, callback) { hash.update(chunk); callback(null, chunk); },
|
||||
flush(callback) { callback(hash.digest("hex") === entry.sha256 ? undefined : new Error("Repository changed during checkpoint")); } });
|
||||
const source = transport.read(root, entry.path, entry.byteSize);
|
||||
source.on("error", (error) => verify.destroy(error));
|
||||
try {
|
||||
await storage.putObject({ objectKey, body: source.pipe(verify), contentType: "application/octet-stream", contentLength: entry.byteSize });
|
||||
} finally { source.destroy(); verify.destroy(); }
|
||||
await uploadWorkFolderObject(storage, { objectKey, contentType: "application/octet-stream",
|
||||
contentLength: entry.byteSize, sha256: entry.sha256!,
|
||||
createSource: () => transport.read(root, entry.path, entry.byteSize) });
|
||||
}
|
||||
files.push({ ...entry, objectKey });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,62 @@
|
|||
import { createHash } from "node:crypto";
|
||||
import { Readable, Transform } from "node:stream";
|
||||
import { pipeline } from "node:stream/promises";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import type { StorageProvider } from "../storage/types.js";
|
||||
|
||||
function transientUploadFailure(error: unknown) {
|
||||
if (!(error instanceof Error)) return false;
|
||||
const failure = error as Error & { code?: string; $metadata?: { httpStatusCode?: number } };
|
||||
// Authentication, validation and ownership failures must never be retried.
|
||||
const status = failure.$metadata?.httpStatusCode;
|
||||
if (status !== undefined) return [408, 429, 500, 502, 503, 504].includes(status);
|
||||
return ["ECONNRESET", "EPIPE", "ETIMEDOUT", "ECONNABORTED", "EAI_AGAIN"].includes(failure.code ?? "");
|
||||
}
|
||||
|
||||
/** Retry an uncertain object PUT with a fresh, fully verified stream and the same key. */
|
||||
export async function uploadWorkFolderObject(storage: Pick<StorageProvider, "putObject">, input: {
|
||||
objectKey: string;
|
||||
contentType: string;
|
||||
contentLength: number;
|
||||
sha256: string;
|
||||
createSource: () => Readable;
|
||||
}) {
|
||||
for (let attempt = 0; attempt < 3; attempt++) {
|
||||
const source = input.createSource();
|
||||
const hash = createHash("sha256");
|
||||
let bytes = 0;
|
||||
let validationError: Error | undefined;
|
||||
const changed = () => validationError ??= new Error("Work file changed during upload; retry the checkpoint");
|
||||
const verify = new Transform({
|
||||
transform(chunk: Buffer, _encoding, callback) {
|
||||
bytes += chunk.length;
|
||||
if (bytes > input.contentLength) return callback(changed());
|
||||
hash.update(chunk);
|
||||
callback(null, chunk);
|
||||
},
|
||||
flush(callback) {
|
||||
callback(bytes === input.contentLength && hash.digest("hex") === input.sha256 ? undefined : changed());
|
||||
},
|
||||
});
|
||||
const transferred = pipeline(source, verify);
|
||||
const uploaded = Promise.resolve().then(() => storage.putObject({
|
||||
objectKey: input.objectKey, contentType: input.contentType,
|
||||
contentLength: input.contentLength, body: verify,
|
||||
}));
|
||||
let failure: unknown;
|
||||
try {
|
||||
// Observe both promises immediately. An early HTTP success cannot publish
|
||||
// unvalidated content, and a failed request must release its source reader.
|
||||
await Promise.all([transferred, uploaded]);
|
||||
return;
|
||||
} catch (error) {
|
||||
failure = validationError ?? error;
|
||||
} finally {
|
||||
source.destroy();
|
||||
verify.destroy();
|
||||
await Promise.allSettled([transferred, uploaded]);
|
||||
}
|
||||
if (validationError || attempt === 2 || !transientUploadFailure(failure)) throw failure;
|
||||
await delay(250 * (attempt + 1));
|
||||
}
|
||||
}
|
||||
|
|
@ -9,6 +9,7 @@ import { and, asc, eq, gt, isNull, isNotNull, max, sql } from "drizzle-orm";
|
|||
import { workFolders, workFiles, workFileOperations, workFolderObjects, type Db } from "@paperclipai/db";
|
||||
import { validateWorkFilePath, type WorkFile, type WorkFolderOwner } from "@paperclipai/shared";
|
||||
import { registerWorkFolderObject } from "./work-folder-garbage.js";
|
||||
import { uploadWorkFolderObject } from "./work-folder-upload.js";
|
||||
import type { StorageProvider } from "../storage/types.js";
|
||||
import { badRequest, conflict, notFound, payloadTooLarge } from "../errors.js";
|
||||
|
||||
|
|
@ -92,6 +93,7 @@ export function workFolderService(db: Db, storage: StorageProvider) {
|
|||
const spool = path.join(directory, "content");
|
||||
const hash = createHash("sha256");
|
||||
let byteSize = 0;
|
||||
let sha256: string | null = null;
|
||||
let objectKey: string | null = null;
|
||||
let discardUpload = false;
|
||||
try {
|
||||
|
|
@ -103,12 +105,13 @@ export function workFolderService(db: Db, storage: StorageProvider) {
|
|||
hash.update(chunk);
|
||||
callback(null, chunk);
|
||||
} }), createWriteStream(spool, { mode: 0o600 }));
|
||||
sha256 = hash.digest("hex");
|
||||
if (input.expectedSha256 && input.expectedSha256 !== sha256) throw conflict("File changed during transfer; retry the checkpoint");
|
||||
objectKey = `${folder.companyId}/work-folders/${folder.id}/${randomUUID()}`;
|
||||
await registerWorkFolderObject(db, storage, { objectKey, companyId: folder.companyId, folderId: folder.id });
|
||||
await storage.putObject({ objectKey, body: createReadStream(spool), contentLength: byteSize,
|
||||
contentType: input.contentType ?? "application/octet-stream" });
|
||||
await uploadWorkFolderObject(storage, { objectKey, createSource: () => createReadStream(spool),
|
||||
contentLength: byteSize, sha256, contentType: input.contentType ?? "application/octet-stream" });
|
||||
}
|
||||
const sha256 = kind === "file" ? hash.digest("hex") : null;
|
||||
if (input.expectedSha256 && input.expectedSha256 !== sha256) throw conflict("File changed during transfer; retry the checkpoint");
|
||||
const value = { kind, objectKey, byteSize, sha256, executable: input.executable ?? false,
|
||||
contentType: input.contentType ?? "application/octet-stream", updatedAt: new Date() };
|
||||
|
|
|
|||
Loading…
Reference in New Issue