feat(uploads): bump cap to 100MB + correct-reason error messages
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
CI / Detect changes (pull_request) Successful in 11s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 16s
E2E API Smoke Test / detect-changes (pull_request) Successful in 16s
E2E Chat / detect-changes (pull_request) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 12s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
Harness Replays / detect-changes (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 7s
Lint no tenant GITEA/GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 5s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m8s
publish-runtime-autobump / pr-validate (pull_request) Successful in 34s
publish-runtime-autobump / bump-and-tag (pull_request) Has been skipped
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 8s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 6s
gate-check-v3 / gate-check (pull_request) Successful in 6s
qa-review / approved (pull_request) Successful in 6s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 3s
sop-checklist / review-refire (pull_request) Has been skipped
sop-tier-check / tier-check (pull_request) Successful in 4s
CI / Platform (Go) (pull_request) Successful in 5m5s
CI / Canvas (Next.js) (pull_request) Successful in 6m11s
CI / Python Lint & Test (pull_request) Successful in 7m17s
CI / all-required (pull_request) Successful in 6m33s
Harness Replays / Harness Replays (pull_request) Successful in 4s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Failing after 2m27s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m24s
security-review / approved (pull_request) Refired via /security-recheck by unknown
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2m56s
E2E Chat / E2E Chat (pull_request) Failing after 6m33s
audit-force-merge / audit (pull_request) Successful in 4s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 10m21s
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
CI / Detect changes (pull_request) Successful in 11s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 16s
E2E API Smoke Test / detect-changes (pull_request) Successful in 16s
E2E Chat / detect-changes (pull_request) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 12s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
Harness Replays / detect-changes (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 7s
Lint no tenant GITEA/GITHUB token write / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 5s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m8s
publish-runtime-autobump / pr-validate (pull_request) Successful in 34s
publish-runtime-autobump / bump-and-tag (pull_request) Has been skipped
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 8s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 6s
gate-check-v3 / gate-check (pull_request) Successful in 6s
qa-review / approved (pull_request) Successful in 6s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 3s
sop-checklist / review-refire (pull_request) Has been skipped
sop-tier-check / tier-check (pull_request) Successful in 4s
CI / Platform (Go) (pull_request) Successful in 5m5s
CI / Canvas (Next.js) (pull_request) Successful in 6m11s
CI / Python Lint & Test (pull_request) Successful in 7m17s
CI / all-required (pull_request) Successful in 6m33s
Harness Replays / Harness Replays (pull_request) Successful in 4s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Failing after 2m27s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m24s
security-review / approved (pull_request) Refired via /security-recheck by unknown
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2m56s
E2E Chat / E2E Chat (pull_request) Failing after 6m33s
audit-force-merge / audit (pull_request) Successful in 4s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 10m21s
CTO 2026-05-19 directive on forensic a99ab0a1 (reno-stars >50MB
upload that surfaced "signal timed out" when the real cause was
file-size + a fixed 60s client timeout):
"if its file size issue, should have error that instead saying
timeout which is wrong"
Bundles the cap raise + the wrong-reason fix in ONE PR because the
two are coupled — bumping the server alone would still leak the
fixed-60s timeout for legitimate slow uploads; fixing the client
alone would 413 every >50MB attempt.
Server (push-mode, EC2 workspace):
- workspace-server/internal/handlers/chat_files.go:
chatUploadMaxBytes 50→100 MB
httpClient.Timeout 120→1200 s (matches the new slow-uplink budget)
- workspace/internal_chat_uploads.py:
CHAT_UPLOAD_MAX_BYTES 50→100 MB
CHAT_UPLOAD_MAX_FILE_BYTES 25→100 MB (aligned with total so a
single legitimate large file succeeds end-to-end)
Canvas:
- canvas/src/components/tabs/chat/uploads.ts:
MAX_UPLOAD_BYTES 100 MB constant + FileTooLargeError class
pre-flight gate: file-size violation throws BEFORE any fetch,
with the actionable "File too large (got X MB) — limit is 100MB"
computeUploadTimeoutMs: 60s floor + 100 KB/s scaled deadline
(was a fixed 60s — the root cause of the forensic)
- canvas/src/components/tabs/chat/hooks/useChatSend.ts:
mapUploadErrorToReason: routes each cause to ITS OWN message
(FileTooLargeError | TimeoutError | server-Error | fallback)
no conflation between file-size and connection-too-slow
Tests:
- workspace-server chat_files_test.go: pins 100 MB constant,
asserts sub-cap forwards + over-cap non-2xx
- canvas uploads.cap.test.ts (10 cases): pre-flight gate, exact-cap
edge, scaled-timeout curve, server-413 propagation, AbortSignal
shape — explicit negative on "TimeoutError ≠ FileTooLargeError"
- canvas useChatSend.errorReason.test.ts (5 cases): per-cause
message contract, explicit negatives that guard against the
wrong-reason conflation
Test harness mirror:
- tests/harness/cf-proxy/nginx.conf: client_max_body_size 50m→100m
(this is the harness mirror; the production CF / nginx tier is
out-of-repo. If prod still caps at 50m, this mirror passes while
prod 413s — surface to ops.)
Follow-up (SSOT, NOT in this PR):
The 100 MB constant now lives in THREE mirror sites (canvas TS +
workspace Python + platform Go). Per feedback_no_single_source_of_truth,
the proper fix is exposing the cap via GET /uploads/limits so the
client fetches the live value. Filing as a separate issue.
References:
- task #295 (internal tracker; CTO-authorized this work)
- forensic a99ab0a1 (reno-stars 2026-05-19)
- feedback_surface_actionable_failure_reason_to_user (CTO 2026-05-17)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,179 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import {
|
||||
uploadChatFiles,
|
||||
FileTooLargeError,
|
||||
MAX_UPLOAD_BYTES,
|
||||
computeUploadTimeoutMs,
|
||||
} from "../uploads";
|
||||
|
||||
// Tests for the 100 MB upload-cap raise + correct-reason error mapping
|
||||
// (CTO 2026-05-19 directive on forensic a99ab0a1: "if its file size
|
||||
// issue, should have error that instead saying timeout which is
|
||||
// wrong"). Each case has its own specific reason; conflation is the
|
||||
// bug this PR fixes.
|
||||
|
||||
// File constructor in node's vitest env supports size via array length.
|
||||
// Allocate a typed-array of N bytes and wrap it — File reads .size off
|
||||
// the underlying Blob. Allocating 101 MB once per test is fine (vitest
|
||||
// maxWorkers=1, single test process).
|
||||
function makeFile(name: string, size: number): File {
|
||||
const buf = new Uint8Array(size);
|
||||
return new File([buf], name);
|
||||
}
|
||||
|
||||
const wsId = "00000000-0000-0000-0000-000000000001";
|
||||
|
||||
describe("uploadChatFiles — MAX_UPLOAD_BYTES + pre-flight gate", () => {
|
||||
it("MAX_UPLOAD_BYTES is exactly 100 MB (mirrors server constant)", () => {
|
||||
// Pinned so a regression that flipped the constant back to 50 MB
|
||||
// would fail loudly here — without this the canvas would
|
||||
// silently start rejecting files the server now accepts.
|
||||
expect(MAX_UPLOAD_BYTES).toBe(100 * 1024 * 1024);
|
||||
});
|
||||
|
||||
it("throws FileTooLargeError for a 101 MB file BEFORE any network I/O", async () => {
|
||||
const oversize = makeFile("big.bin", 101 * 1024 * 1024);
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
try {
|
||||
await uploadChatFiles(wsId, [oversize]);
|
||||
throw new Error("expected uploadChatFiles to throw, but it resolved");
|
||||
} catch (e) {
|
||||
// The exact class name matters — useChatSend's mapUploadErrorToReason
|
||||
// routes off `instanceof FileTooLargeError`. A regression that
|
||||
// demoted to a plain Error would silently re-introduce the
|
||||
// wrong-reason conflation CTO flagged.
|
||||
expect(e).toBeInstanceOf(FileTooLargeError);
|
||||
const err = e as FileTooLargeError;
|
||||
// Message must contain the 100MB cap (so the user knows what the
|
||||
// limit is) and a number-with-MB form of the actual size.
|
||||
expect(err.message).toContain("100MB");
|
||||
// Some toFixed(1) renderings: 101.0MB. Loose match: contains "MB".
|
||||
expect(err.message).toMatch(/got\s+\d+(\.\d+)?MB/);
|
||||
expect(err.fileSize).toBe(101 * 1024 * 1024);
|
||||
}
|
||||
// CRITICAL: no fetch may have been initiated. Pre-flight is the
|
||||
// whole point — if a network round-trip happened we'd be back to
|
||||
// surfacing a downstream timeout / 413 instead of the actionable
|
||||
// file-size message.
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("accepts a file exactly at the cap (== MAX_UPLOAD_BYTES)", async () => {
|
||||
// Equality must NOT trip the gate — the cap is inclusive on the
|
||||
// server side and the canvas must match. Without this, an exact-
|
||||
// cap file would 503 client-side while the server accepts it.
|
||||
const exact = makeFile("max.bin", MAX_UPLOAD_BYTES);
|
||||
const fetchSpy = vi
|
||||
.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValue(
|
||||
new Response(JSON.stringify({ files: [] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
await expect(uploadChatFiles(wsId, [exact])).resolves.toBeDefined();
|
||||
expect(fetchSpy).toHaveBeenCalledOnce();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeUploadTimeoutMs — scaled timeout curve", () => {
|
||||
it("100 KB file → 60s floor (small-file ergonomics)", () => {
|
||||
// Below the floor, the small-file UX (typo'd hostname surfacing as
|
||||
// connect-error quickly) takes priority over the slow-uplink
|
||||
// assumption.
|
||||
expect(computeUploadTimeoutMs(100 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("1 MB file → 60s floor", () => {
|
||||
expect(computeUploadTimeoutMs(1 * 1024 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("100 MB file → ~1000s (matches the slow-uplink design budget)", () => {
|
||||
// Pin the upper-bound case the design targets: at 100 MB / 100 KB/s
|
||||
// a legitimate slow uplink completes in ~1000s, comfortably
|
||||
// before the platform's 1200s http.Client timeout. Without this
|
||||
// scaling, the previous fixed 60s deadline aborted Ryan's ~60 MB
|
||||
// upload in forensic a99ab0a1.
|
||||
const ms = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
// 100*1024*1024 / 100 = 1048576 ms ≈ 1048.6s — pin to ±1ms.
|
||||
expect(ms).toBe(Math.floor((100 * 1024 * 1024) / 100));
|
||||
expect(ms).toBeGreaterThan(1_000_000);
|
||||
expect(ms).toBeLessThan(1_100_000);
|
||||
});
|
||||
|
||||
it("strictly monotonic above the floor", () => {
|
||||
// A regression that capped or non-monotonised the curve would
|
||||
// silently re-introduce premature aborts for mid-size files.
|
||||
const a = computeUploadTimeoutMs(10 * 1024 * 1024);
|
||||
const b = computeUploadTimeoutMs(50 * 1024 * 1024);
|
||||
const c = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
expect(b).toBeGreaterThan(a);
|
||||
expect(c).toBeGreaterThan(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe("uploadChatFiles — error path shapes (for downstream reason-mapping)", () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn> | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
});
|
||||
afterEach(() => {
|
||||
fetchSpy?.mockRestore();
|
||||
fetchSpy = null;
|
||||
});
|
||||
|
||||
it("propagates the server's 413 reason verbatim (not as 'timeout')", async () => {
|
||||
// The error message text is what useChatSend surfaces via
|
||||
// `Upload failed: ${e.message}` — pin that the server's reason
|
||||
// is present, not swallowed.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"error":"file exceeds per-file limit (100 MB)"}', {
|
||||
status: 413,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).rejects.toThrow(
|
||||
/upload failed:.*413.*per-file limit/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("propagates AbortSignal timeout as a DOMException with name=TimeoutError", async () => {
|
||||
// Reason-routing in useChatSend.mapUploadErrorToReason discriminates
|
||||
// by e.name === 'TimeoutError'. Pin the shape so a future browser /
|
||||
// polyfill change that renamed it would fail loudly here, NOT
|
||||
// silently fall through to the generic "Upload failed" path
|
||||
// (which is what made forensic a99ab0a1 hard to root-cause).
|
||||
const abortErr = new DOMException("signal timed out", "TimeoutError");
|
||||
fetchSpy!.mockRejectedValue(abortErr);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
try {
|
||||
await uploadChatFiles(wsId, [f]);
|
||||
throw new Error("expected throw");
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(DOMException);
|
||||
expect((e as DOMException).name).toBe("TimeoutError");
|
||||
// CRITICAL negative: the rejection must NOT be a
|
||||
// FileTooLargeError, because pre-flight already excluded that.
|
||||
expect(e).not.toBeInstanceOf(FileTooLargeError);
|
||||
}
|
||||
});
|
||||
|
||||
it("a 50 MB file does NOT trip the pre-flight gate (sub-cap)", async () => {
|
||||
// The forensic case: Ryan's file was over the OLD 50MB cap but
|
||||
// under the NEW 100MB cap. Pin that the pre-flight does NOT
|
||||
// misfire on a sub-100MB file.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"files":[]}', {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("ryan.bin", 50 * 1024 * 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).resolves.toBeDefined();
|
||||
expect(fetchSpy!).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { mapUploadErrorToReason } from "../useChatSend";
|
||||
import { FileTooLargeError } from "../../uploads";
|
||||
|
||||
// Pin the case-by-case error mapping (CTO 2026-05-19 directive on
|
||||
// forensic a99ab0a1: each cause maps to ITS OWN message, no
|
||||
// conflation). The four cases — FileTooLargeError, TimeoutError,
|
||||
// other Error, non-Error — are the entire user-facing contract this
|
||||
// PR ships; each gets a dedicated assertion so a regression that
|
||||
// re-conflated them would surface here.
|
||||
|
||||
describe("mapUploadErrorToReason", () => {
|
||||
it("FileTooLargeError → surfaces the pre-flight message verbatim", () => {
|
||||
const err = new FileTooLargeError(
|
||||
101 * 1024 * 1024,
|
||||
"File too large (got 101.0MB) — limit is 100MB. Please use a smaller file.",
|
||||
);
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Verbatim, no "Upload failed:" prefix — the FileTooLargeError
|
||||
// message is already a complete, user-facing sentence.
|
||||
expect(out).toBe(err.message);
|
||||
expect(out).not.toMatch(/^Upload failed:/);
|
||||
// Must mention the cap so the user knows what to aim for.
|
||||
expect(out).toContain("100MB");
|
||||
// Must NOT mention timeout — wrong-reason conflation guard.
|
||||
expect(out.toLowerCase()).not.toContain("timeout");
|
||||
expect(out.toLowerCase()).not.toContain("connection");
|
||||
});
|
||||
|
||||
it("TimeoutError → connection-too-slow message, NOT file-size", () => {
|
||||
const err = new DOMException("signal timed out", "TimeoutError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// The user-facing reason matches the design contract: tells the
|
||||
// user the connection is slow, gives them the actionable retry
|
||||
// hint, and does NOT mention file-size (pre-flight already
|
||||
// excluded that — this is the case CTO flagged).
|
||||
expect(out).toContain("Upload timed out");
|
||||
expect(out).toContain("connection is too slow");
|
||||
// CRITICAL negatives — guard against the wrong-reason conflation.
|
||||
expect(out).not.toMatch(/100MB|file too large|File too large/);
|
||||
});
|
||||
|
||||
it("plain Error from server (e.g. 413) → wraps with 'Upload failed:' + server reason", () => {
|
||||
// What uploadChatFiles throws when res.ok is false. The message
|
||||
// already encodes the status + body; the mapper just prefixes
|
||||
// "Upload failed:" so the chat error banner makes sense.
|
||||
const err = new Error("upload failed: 413 file exceeds per-file limit");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
expect(out).toBe("Upload failed: upload failed: 413 file exceeds per-file limit");
|
||||
// Server's actual reason must survive — that's the whole
|
||||
// feedback_surface_actionable_failure_reason_to_user point.
|
||||
expect(out).toContain("413");
|
||||
expect(out).toContain("per-file limit");
|
||||
});
|
||||
|
||||
it("non-Error throw → generic fallback", () => {
|
||||
// A string-throw (or a frozen object) is unusual but possible in
|
||||
// some catch paths. The fallback must NOT crash and must still
|
||||
// give the user a non-empty reason.
|
||||
expect(mapUploadErrorToReason("some random string")).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(undefined)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(null)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(42)).toBe("Upload failed");
|
||||
});
|
||||
|
||||
it("an AbortError that ISN'T a TimeoutError falls through to generic Error path", () => {
|
||||
// Belt-and-braces: a regression that loosened the name check to
|
||||
// ANY DOMException would silently rewrite legitimate AbortError
|
||||
// (user-initiated cancel) into "connection too slow". Pin the
|
||||
// narrow check.
|
||||
const err = new DOMException("user aborted", "AbortError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Falls through to non-Error branch (DOMException is not an Error
|
||||
// subclass in node's vitest environment); accept either generic
|
||||
// fallback or the Error-message form depending on the runtime.
|
||||
expect(out).not.toContain("connection is too slow");
|
||||
expect(out).not.toContain("File too large");
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { useCallback, useRef, useState } from "react";
|
||||
import { api } from "@/lib/api";
|
||||
import { uploadChatFiles } from "../uploads";
|
||||
import { uploadChatFiles, FileTooLargeError } from "../uploads";
|
||||
import { createMessage, type ChatMessage, type ChatAttachment } from "../types";
|
||||
import { extractFilesFromTask } from "../message-parser";
|
||||
|
||||
@@ -46,6 +46,52 @@ export function extractReplyText(resp: A2AResponse): string {
|
||||
return collected.join("\n");
|
||||
}
|
||||
|
||||
/** Map a thrown error from `uploadChatFiles` to the user-facing reason
|
||||
* shown in the chat error banner.
|
||||
*
|
||||
* Cases (per `feedback_surface_actionable_failure_reason_to_user` —
|
||||
* user-facing failures MUST tell the user WHY):
|
||||
*
|
||||
* 1. FileTooLargeError → use the error's message verbatim. The
|
||||
* pre-flight already built the actionable string with the actual
|
||||
* size + the cap; don't re-wrap it (which would prepend a
|
||||
* redundant "Upload failed:" prefix).
|
||||
*
|
||||
* 2. DOMException name="TimeoutError" → AbortSignal.timeout fired
|
||||
* during the fetch. Pre-flight already excluded file-size, so
|
||||
* this CANNOT mean "file too large". Surface a connection-speed
|
||||
* message — the user's actionable next step is retry or check
|
||||
* network, NOT shrink the file.
|
||||
*
|
||||
* 3. Other Error → use the wrapped form so the server's reason
|
||||
* (e.g. "upload failed: 413 ...") reaches the user instead of
|
||||
* being swallowed.
|
||||
*
|
||||
* 4. Non-Error throw → generic fallback.
|
||||
*
|
||||
* Exported for unit testing — the case-by-case mapping is the
|
||||
* load-bearing contract this PR ships. */
|
||||
export function mapUploadErrorToReason(e: unknown): string {
|
||||
if (e instanceof FileTooLargeError) {
|
||||
// Already a complete, user-facing sentence — surface verbatim.
|
||||
return e.message;
|
||||
}
|
||||
// DOMException with name="TimeoutError" is what AbortSignal.timeout
|
||||
// produces on abort. Browsers represent it as a DOMException, not a
|
||||
// regular Error subclass — feature-detect via .name to avoid coupling
|
||||
// to a global that's missing in test envs.
|
||||
if (
|
||||
e !== null && typeof e === "object" &&
|
||||
"name" in e && (e as { name: unknown }).name === "TimeoutError"
|
||||
) {
|
||||
return "Upload timed out — your connection is too slow for this file. Try again, or reduce file size.";
|
||||
}
|
||||
if (e instanceof Error) {
|
||||
return `Upload failed: ${e.message}`;
|
||||
}
|
||||
return "Upload failed";
|
||||
}
|
||||
|
||||
export interface UseChatSendOptions {
|
||||
getHistoryMessages: () => ChatMessage[];
|
||||
onUserMessage?: (msg: ChatMessage) => void;
|
||||
@@ -85,9 +131,12 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
|
||||
} catch (e) {
|
||||
setUploading(false);
|
||||
sendInFlightRef.current = false;
|
||||
setError(
|
||||
e instanceof Error ? `Upload failed: ${e.message}` : "Upload failed",
|
||||
);
|
||||
// Error-reason routing (CTO 2026-05-19 on forensic a99ab0a1:
|
||||
// "if its file size issue, should have error that instead
|
||||
// saying timeout which is wrong"). Each cause maps to ITS
|
||||
// OWN message — NO conflation between file-size and
|
||||
// connection-too-slow.
|
||||
setError(mapUploadErrorToReason(e));
|
||||
return;
|
||||
}
|
||||
setUploading(false);
|
||||
|
||||
@@ -1,6 +1,55 @@
|
||||
import { PLATFORM_URL, platformAuthHeaders } from "@/lib/api";
|
||||
import type { ChatAttachment } from "./types";
|
||||
|
||||
/** Hard cap on a single chat upload. Pre-flight gate: this constant is
|
||||
* checked BEFORE any network I/O so a file-size violation surfaces
|
||||
* immediately with an actionable reason ("File too large (got X MB)
|
||||
* — limit is 100MB") rather than as a downstream timeout or 413.
|
||||
*
|
||||
* SERVER_MIRROR: keep aligned with
|
||||
* - workspace-server/internal/handlers/chat_files.go chatUploadMaxBytes
|
||||
* - workspace/internal_chat_uploads.py CHAT_UPLOAD_MAX_BYTES /
|
||||
* CHAT_UPLOAD_MAX_FILE_BYTES
|
||||
*
|
||||
* Three mirror sites exist because each layer must enforce / pre-flight
|
||||
* on its own (no shared codegen yet). Tracked for SSOT follow-up:
|
||||
* expose via GET /uploads/limits so the client can fetch the live cap
|
||||
* instead of duplicating the constant. */
|
||||
export const MAX_UPLOAD_BYTES = 100 * 1024 * 1024;
|
||||
|
||||
/** Thrown by `uploadChatFiles` when a candidate file exceeds
|
||||
* MAX_UPLOAD_BYTES. Caught by `useChatSend` and surfaced verbatim —
|
||||
* the message is already user-actionable. Distinct name lets the
|
||||
* catch path route it correctly without parsing the message string.
|
||||
*
|
||||
* Why a distinct class instead of a sentinel string match: the catch
|
||||
* in `useChatSend` already needs to discriminate this case from a
|
||||
* `TimeoutError` (which has a structurally similar surface but a
|
||||
* DIFFERENT root cause). Conflating them was the bug CTO flagged on
|
||||
* forensic a99ab0a1: "if its file size issue, should have error that
|
||||
* instead saying timeout which is wrong". */
|
||||
export class FileTooLargeError extends Error {
|
||||
readonly name = "FileTooLargeError";
|
||||
readonly fileSize: number;
|
||||
constructor(fileSize: number, message: string) {
|
||||
super(message);
|
||||
this.fileSize = fileSize;
|
||||
}
|
||||
}
|
||||
|
||||
/** Compute the abort timeout for an upload of `totalBytes`. Floor at
|
||||
* 60s (small-file ergonomics: a 100 KB image shouldn't wait 1000s to
|
||||
* see a typo'd hostname surface as a connect error). Above the floor,
|
||||
* scale linearly at ~100 KB/s assumed minimum uplink — at the 100 MB
|
||||
* cap this yields ~1000s, comfortable for the slow-mobile-tether case
|
||||
* that motivated forensic a99ab0a1 (Ryan's >50 MB upload aborted at
|
||||
* the fixed 60s timeout while still streaming).
|
||||
*
|
||||
* Exported for the unit test that pins the curve at the boundary. */
|
||||
export function computeUploadTimeoutMs(totalBytes: number): number {
|
||||
return Math.max(60_000, totalBytes / 100); // 100KB/s → ms = bytes/100
|
||||
}
|
||||
|
||||
/** Chat attachments are intentionally uploaded via a direct fetch()
|
||||
* instead of the `api.post` helper — `api.post` JSON-stringifies the
|
||||
* body, which would 500 on a Blob. Auth headers (tenant slug, admin
|
||||
@@ -10,25 +59,57 @@ import type { ChatAttachment } from "./types";
|
||||
* Content-Type so the browser writes the multipart boundary into the
|
||||
* header; setting it manually would yield a multipart body the server
|
||||
* can't parse. See lib/api.ts platformAuthHeaders() for the full
|
||||
* rationale on why this pair must stay matched. */
|
||||
* rationale on why this pair must stay matched.
|
||||
*
|
||||
* Failure-reason contract (CTO 2026-05-19 directive on forensic
|
||||
* a99ab0a1: each cause maps to ITS OWN message, no conflation):
|
||||
* 1. file.size > MAX_UPLOAD_BYTES → throws FileTooLargeError
|
||||
* BEFORE any network I/O, with the offending size + the cap.
|
||||
* 2. fetch aborts via AbortSignal → DOMException name="TimeoutError";
|
||||
* caller surfaces "connection too slow" (file-size already
|
||||
* excluded by gate 1, so the TimeoutError CANNOT mean file-size).
|
||||
* 3. server returns !res.ok → throws Error with the server's
|
||||
* reason embedded (status + body); caller surfaces verbatim.
|
||||
* 4. any other thrown error → falls through as-is. */
|
||||
export async function uploadChatFiles(
|
||||
workspaceId: string,
|
||||
files: File[],
|
||||
): Promise<ChatAttachment[]> {
|
||||
if (files.length === 0) return [];
|
||||
|
||||
// PRE-FLIGHT: bail before any network I/O if any file exceeds the cap.
|
||||
// After this gate, an AbortSignal.timeout firing during the fetch
|
||||
// CANNOT be attributed to file size — it's necessarily a slow
|
||||
// connection. That distinction is what makes the downstream error
|
||||
// mapping unambiguous.
|
||||
let totalBytes = 0;
|
||||
for (const f of files) {
|
||||
if (f.size > MAX_UPLOAD_BYTES) {
|
||||
const sizeMb = (f.size / (1024 * 1024)).toFixed(1);
|
||||
throw new FileTooLargeError(
|
||||
f.size,
|
||||
`File too large (got ${sizeMb}MB) — limit is 100MB. Please use a smaller file.`,
|
||||
);
|
||||
}
|
||||
totalBytes += f.size;
|
||||
}
|
||||
|
||||
const form = new FormData();
|
||||
for (const f of files) form.append("files", f, f.name);
|
||||
|
||||
// Uploads legitimately take a while on cold cache (tar write +
|
||||
// docker cp into the container). 60s is comfortable for the 25MB/
|
||||
// 50MB caps the server enforces.
|
||||
// Scale the abort timeout with payload size so a legitimate slow-
|
||||
// uplink upload of a large file isn't aborted before the body has
|
||||
// finished streaming. The fixed 60s previous-version was the root
|
||||
// cause of forensic a99ab0a1: Ryan's ~60 MB upload over a constrained
|
||||
// uplink streamed past 60s, AbortSignal fired client-side, server
|
||||
// got a truncated body, the user saw "signal timed out" — when the
|
||||
// real cause was simply "uplink slower than our hard-coded deadline".
|
||||
const res = await fetch(`${PLATFORM_URL}/workspaces/${workspaceId}/chat/uploads`, {
|
||||
method: "POST",
|
||||
headers: platformAuthHeaders(),
|
||||
body: form,
|
||||
credentials: "include",
|
||||
signal: AbortSignal.timeout(60_000),
|
||||
signal: AbortSignal.timeout(computeUploadTimeoutMs(totalBytes)),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => "");
|
||||
|
||||
Reference in New Issue
Block a user