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
17 changes: 17 additions & 0 deletions src/client/http-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,23 @@ describe("ApiClient path guard + envelope unwrap", () => {
const c = new ApiClient(cfg({ token: "t" }));
await expect(c.get("/cms/x")).rejects.toMatchObject({ statusCode: 422, apiMessage: "boom" });
});

it("extracts the message from a structured error envelope (not [object Object])", async () => {
// Backend shape: { success:false, error:{ id, code, status, message }, message }
mockFetch(
{ success: false, error: { id: "payload_too_large", code: 413, message: "media: payload too large" }, message: "media: payload too large" },
false,
413
);
const c = new ApiClient(cfg({ token: "t" }));
await expect(c.get("/cms/x")).rejects.toMatchObject({ statusCode: 413, apiMessage: "media: payload too large" });
});

it("falls back to the top-level message when error is not a string", async () => {
mockFetch({ error: {}, message: "top-level detail" }, false, 400);
const c = new ApiClient(cfg({ token: "t" }));
await expect(c.get("/cms/x")).rejects.toMatchObject({ statusCode: 400, apiMessage: "top-level detail" });
});
});

describe("putAbsoluteBytes", () => {
Expand Down
41 changes: 32 additions & 9 deletions src/client/http-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,31 @@ function isMcpPath(path: string): boolean {
return path === "/mcp" || path.startsWith("/mcp/");
}

// Pull a human-readable message out of an error response body. The backend
// wraps errors as { success:false, error:{ id, code, status, message }, message }
// so a naive `data.error` yields the object itself (rendered "[object Object]").
// Prefer error.message, then the top-level message, then a bare string body.
function extractApiErrorMessage(data: unknown, statusText: string): string {
if (data && typeof data === "object") {
const d = data as Record<string, unknown>;
const err = d.error;
if (typeof err === "string" && err) return err;
if (err && typeof err === "object") {
const m = (err as Record<string, unknown>).message;
if (typeof m === "string" && m) return m;
}
if (typeof d.message === "string" && d.message) return d.message;
}
if (typeof data === "string" && data) return data;
return statusText;
}

// Per-request overrides. `timeoutMs` lets callers widen the abort window for
// requests known to run long server-side (e.g. media commit content-probing).
export interface RequestOptions {
timeoutMs?: number;
}

// HTTP client for AgentBrain API with auth, error handling, and SSE streaming
export class ApiClient {
private baseUrl: string;
Expand All @@ -35,8 +60,8 @@ export class ApiClient {
return this.request<T>("GET", path, undefined, params);
}

async post<T>(path: string, body?: unknown): Promise<T> {
return this.request<T>("POST", path, body);
async post<T>(path: string, body?: unknown, opts?: RequestOptions): Promise<T> {
return this.request<T>("POST", path, body, undefined, opts);
}

async put<T>(path: string, body?: unknown): Promise<T> {
Expand Down Expand Up @@ -190,17 +215,18 @@ export class ApiClient {
return url.toString();
}

private async request<T>(method: string, path: string, body?: unknown, params?: Record<string, string>): Promise<T> {
private async request<T>(method: string, path: string, body?: unknown, params?: Record<string, string>, opts?: RequestOptions): Promise<T> {
const url = this.buildUrl(path, params);
const headers = { ...this.baseHeaders(), ...this.authHeaders(path) };
const start = Date.now();
const timeout = opts?.timeoutMs ?? this.timeout;

if (this.verbose) {
console.error(`${method} ${url}`);
}

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), this.timeout);
const timeoutId = setTimeout(() => controller.abort(), timeout);

try {
const response = await fetch(url, {
Expand All @@ -225,10 +251,7 @@ export class ApiClient {
}

if (!response.ok) {
const msg = (data as Record<string, unknown>)?.error ??
(data as Record<string, unknown>)?.message ??
response.statusText;
throw new ApiError(response.status, String(msg), data);
throw new ApiError(response.status, extractApiErrorMessage(data, response.statusText), data);
}

// Unwrap envelope if present: { data: T } -> T
Expand All @@ -239,7 +262,7 @@ export class ApiClient {
} catch (err) {
if (err instanceof ApiError) throw err;
if ((err as Error).name === "AbortError") {
throw new ApiError(408, `Request timed out after ${this.timeout}ms`);
throw new ApiError(408, `Request timed out after ${timeout}ms`);
}
throw err;
} finally {
Expand Down
50 changes: 49 additions & 1 deletion src/utils/media-upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { describe, it, expect, beforeAll, afterAll } from "vitest";
import { writeFile, rm } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { detectContentType, detectKind, uploadMedia, MAX_UPLOAD_BYTES } from "./media-upload.js";
import { detectContentType, detectKind, uploadMedia, MAX_UPLOAD_BYTES, commitTimeoutMs } from "./media-upload.js";
import type { ApiClient } from "../client/http-client.js";
import { ApiError } from "../client/api-error.js";

describe("detectContentType", () => {
it("maps known extensions", () => {
Expand Down Expand Up @@ -107,6 +108,53 @@ describe("uploadMedia orchestration", () => {
const fakeClient = { post: async () => ({}), putAbsoluteBytes: async () => {} } as unknown as ApiClient;
await expect(uploadMedia(fakeClient, { filePath: "/no/such/file.pdf" })).rejects.toThrow(/File not found/);
});

it("passes a size-scaled timeout to the commit request", async () => {
let commitOpts: { timeoutMs?: number } | undefined;
const fakeClient = {
post: async (path: string, _body: unknown, opts?: { timeoutMs?: number }) => {
if (path.endsWith("/presign")) {
return { assetId: "a", putUrl: "https://x/y", expiresAt: "", fields: {} };
}
commitOpts = opts;
return { assetId: "a", status: "ready", objectKey: "k" };
},
putAbsoluteBytes: async () => {},
} as unknown as ApiClient;

await uploadMedia(fakeClient, { filePath, computeSha256: false });
// Small fixture → floor applies; must be well above the default 30s request timeout.
expect(commitOpts?.timeoutMs).toBe(commitTimeoutMs(Buffer.byteLength(fileBody)));
expect(commitOpts?.timeoutMs).toBeGreaterThanOrEqual(60_000);
});

it("treats a 409 commit as success, reusing the echoed asset payload", async () => {
const fakeClient = {
post: async (path: string) => {
if (path.endsWith("/presign")) {
return { assetId: "dup", putUrl: "https://x/y", expiresAt: "", fields: {} };
}
throw new ApiError(409, "already committed", {
data: { assetId: "dup", status: "ready", objectKey: "org/dup.pdf" },
});
},
putAbsoluteBytes: async () => {},
} as unknown as ApiClient;

const result = await uploadMedia(fakeClient, { filePath, computeSha256: false });
expect(result.status).toBe("ready");
expect(result.objectKey).toBe("org/dup.pdf");
});
});

describe("commitTimeoutMs", () => {
it("applies a floor for small files", () => {
expect(commitTimeoutMs(1_000)).toBe(60_000);
});
it("scales with size for large media", () => {
// 300MB → 300 * 1000ms = 300s, comfortably above the observed ~50s probe time.
expect(commitTimeoutMs(300_000_000)).toBe(300_000);
});
});

describe("limits", () => {
Expand Down
33 changes: 26 additions & 7 deletions src/utils/media-upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ export const MAX_UPLOAD_BYTES = 500_000_000;
// backend (which HEAD-validates size authoritatively) is rarely worth the I/O.
export const SHA256_DEFAULT_MAX_BYTES = 100_000_000;

// Floor for the commit timeout so even small uploads tolerate a slow probe.
const COMMIT_TIMEOUT_FLOOR_MS = 60_000;
// Extra window granted per megabyte to cover backend content-probing that
// streams the object from storage (observed ~50s for a 300MB media asset).
const COMMIT_TIMEOUT_MS_PER_MB = 1_000;

// Size-scaled abort window for the commit step. Kept generous because it only
// caps a request that returns as soon as the backend finishes probing.
export function commitTimeoutMs(sizeBytes: number): number {
return Math.max(COMMIT_TIMEOUT_FLOOR_MS, Math.ceil(sizeBytes / 1_000_000) * COMMIT_TIMEOUT_MS_PER_MB);
}

// Media asset kind accepted by the presign endpoint.
export type MediaKind = "audio" | "video" | "image" | "raw_doc";

Expand Down Expand Up @@ -150,15 +162,22 @@ export async function uploadMedia(client: ApiClient, opts: UploadOptions): Promi

let commit: CommitResponse;
try {
commit = await client.post<CommitResponse>("/cms/media/uploads/commit", {
assetId: presign.assetId,
sha256,
contentType,
});
commit = await client.post<CommitResponse>(
"/cms/media/uploads/commit",
{ assetId: presign.assetId, sha256, contentType },
// Commit runs backend-side content probing (media kinds stream the whole
// object from storage through ffprobe), which scales with file size and
// routinely exceeds the default 30s request timeout for large media.
// Give commit a size-scaled window so a slow-but-succeeding commit is not
// aborted (leaving the asset stuck in "uploaded").
{ timeoutMs: commitTimeoutMs(sizeBytes) }
);
} catch (err) {
// 409 = asset already committed (e.g. a retried commit). Treat as success.
// 409 = asset already committed (e.g. a retried commit). Treat as success,
// reusing the asset payload the backend echoes in the 409 body when present.
if (err instanceof ApiError && err.statusCode === 409) {
commit = { assetId: presign.assetId, status: "ready", objectKey: "" };
const echoed = (err.details as { data?: CommitResponse } | undefined)?.data;
commit = echoed ?? { assetId: presign.assetId, status: "ready", objectKey: "" };
} else {
throw err;
}
Expand Down