Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/selfhost/s3-blob-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export type S3BlobStoreConfig = {
// review pipeline over a persistently misconfigured or down bucket.
const S3_CLIENT_RETRIES = 3;

// Per-request ceiling for every S3 REST call (matches the bounded-fetch convention already used by the
// other self-host adapters, e.g. qdrant-vectorize.ts's QDRANT_FETCH_TIMEOUT_MS). aws4fetch's `retries`
// option above only bounds retry COUNT, not per-attempt hang time -- a misconfigured or unreachable
// S3-compatible endpoint could otherwise hang get/put/delete indefinitely.
const S3_FETCH_TIMEOUT_MS = 15_000;

/** Build an S3-compatible-bucket-backed REVIEW_AUDIT store. Keys are app-generated
* (`loopover/shots/<hash>.png`, already validated by the /loopover/shot serve route's own prefix +
* traversal check) and passed straight through as the S3 object key -- no additional encoding beyond the
Expand All @@ -56,7 +62,7 @@ export function createS3BlobStore(config: S3BlobStoreConfig): R2Bucket {
/** Stream a stored object's bytes, or null on a miss (404) or any request failure. */
async get(key: string): Promise<R2ObjectBody | null> {
try {
const response = await client.fetch(urlFor(key), { method: "GET" });
const response = await client.fetch(urlFor(key), { method: "GET", signal: AbortSignal.timeout(S3_FETCH_TIMEOUT_MS) });
if (!response.ok) return null;
return { body: response.body } as unknown as R2ObjectBody;
} catch {
Expand All @@ -74,14 +80,14 @@ export function createS3BlobStore(config: S3BlobStoreConfig): R2Bucket {
const body = await new Response(value ?? "").arrayBuffer();
const headers: Record<string, string> = {};
if (options?.httpMetadata?.contentType) headers["content-type"] = options.httpMetadata.contentType;
const response = await client.fetch(urlFor(key), { method: "PUT", headers, body });
const response = await client.fetch(urlFor(key), { method: "PUT", headers, body, signal: AbortSignal.timeout(S3_FETCH_TIMEOUT_MS) });
if (!response.ok) throw new Error(`S3 put failed: ${response.status} ${await response.text().catch(() => "")}`);
return { key } as unknown as R2Object;
},
/** Delete a stored object. Best-effort semantics live with the caller (see actions-fallback.ts's dispatch
* marker cleanup) -- this itself just reports whether the DELETE request succeeded. */
async delete(key: string): Promise<void> {
const response = await client.fetch(urlFor(key), { method: "DELETE" });
const response = await client.fetch(urlFor(key), { method: "DELETE", signal: AbortSignal.timeout(S3_FETCH_TIMEOUT_MS) });
if (!response.ok && response.status !== 404) {
throw new Error(`S3 delete failed: ${response.status} ${await response.text().catch(() => "")}`);
}
Expand Down
37 changes: 37 additions & 0 deletions test/unit/selfhost-s3-blob-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,40 @@ describe("createS3BlobStore (self-host visual screenshot persistence, S3-compati
expect(request.headers.get("authorization")).toContain("/us-east-1/s3/aws4_request");
});
});

describe("S3 REST calls are bounded by an AbortSignal timeout (#8362)", () => {
let fetchMock: ReturnType<typeof vi.fn>;

beforeEach(() => {
fetchMock = vi.fn();
vi.stubGlobal("fetch", fetchMock);
});

afterEach(() => {
vi.unstubAllGlobals();
});

it("get, put, and delete all pass an AbortSignal to the underlying fetch", async () => {
fetchMock.mockResolvedValue(new Response(new Uint8Array([1]), { status: 200 }));
const store = createS3BlobStore(CONFIG);
await store.get("loopover/shots/x.png");
await store.put("loopover/shots/x.png", new Uint8Array([1]));
await store.delete("loopover/shots/x.png");

expect(fetchMock.mock.calls.length).toBe(3);
for (const call of fetchMock.mock.calls) {
const [request] = call as [Request];
expect(request.signal).toBeInstanceOf(AbortSignal);
}
});

it("a get() request that aborts (simulating a timed-out fetch) degrades to null (never throws), matching the network-failure fail-safe", async () => {
fetchMock.mockRejectedValueOnce(new DOMException("The operation was aborted.", "TimeoutError"));
await expect(createS3BlobStore(CONFIG).get("loopover/shots/x.png")).resolves.toBeNull();
});

it("a put() request that aborts (simulating a timed-out fetch) rejects, matching the network-failure fail-safe contract of put/delete", async () => {
fetchMock.mockRejectedValueOnce(new DOMException("The operation was aborted.", "TimeoutError"));
await expect(createS3BlobStore(CONFIG).put("loopover/shots/x.png", new Uint8Array([1]))).rejects.toThrow(/aborted/);
});
});