From 6141622d7f9714dd9370f4e2303df7ecac277865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Thu, 11 Jun 2026 11:22:36 +0200 Subject: [PATCH 1/3] feat(core): surface S3 error details on upload failure Read the XML error body S3 returns on a failed upload and include the Code/Message in the thrown error so users see the actual reason (e.g. "AccessDenied: Request has expired") instead of a bare HTTP status. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/s3.ts | 61 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/core/src/s3.ts b/packages/core/src/s3.ts index eb4d04b8..d0707e8e 100644 --- a/packages/core/src/s3.ts +++ b/packages/core/src/s3.ts @@ -11,6 +11,52 @@ interface PresignedPostUploadInput extends UploadInput { fields: Record; } +/** + * On failure, S3 responds with an XML body describing the error, e.g.: + * + * + * + * AccessDenied + * Request has expired + * ... + * + * + * Extract the `Code` and `Message` so the user gets the actual reason for the + * failure instead of a generic HTTP status. Returns `null` when the body + * cannot be read or does not look like an S3 error document. + */ +async function readS3ErrorMessage(response: Response): Promise { + try { + const body = await response.text(); + const code = body.match(/(.*?)<\/Code>/)?.[1]; + const message = body.match(/(.*?)<\/Message>/)?.[1]; + if (code && message) { + return `${code}: ${message}`; + } + return message ?? code ?? null; + } catch { + return null; + } +} + +/** + * Build an error for a failed upload, including the S3 error details when + * available so the user can understand why the upload was rejected. + */ +async function createUploadError( + url: string, + response: Response, +): Promise { + const detail = await readS3ErrorMessage(response); + const status = `${response.status} ${response.statusText}`; + return new Error( + `Failed to upload file to ${url}: ${status}${detail ? ` — ${detail}` : ""}`, + ); +} + +/** + * Upload a file to S3 using a presigned PUT URL. + */ export async function uploadFile(input: UploadInput): Promise { const file = await readFile(input.path); const response = await fetch(input.url, { @@ -22,17 +68,22 @@ export async function uploadFile(input: UploadInput): Promise { body: new Uint8Array(file), }); if (!response.ok) { - throw new Error( - `Failed to upload file to ${input.url}: ${response.status} ${response.statusText}`, - ); + throw await createUploadError(input.url, response); } } +/** + * Upload a file to S3 using a presigned POST. Unlike a presigned PUT, this + * sends the file as multipart form data alongside the policy `fields` + * provided by S3. + */ export async function uploadFileWithPresignedPost( input: PresignedPostUploadInput, ): Promise { const file = await readFile(input.path); const formData = new FormData(); + // The presigned policy fields (key, policy, signature, etc.) must be + // appended before the file part for S3 to accept the upload. for (const [key, value] of Object.entries(input.fields)) { formData.append(key, value); } @@ -48,8 +99,6 @@ export async function uploadFileWithPresignedPost( body: formData, }); if (!response.ok) { - throw new Error( - `Failed to upload file to ${input.url}: ${response.status} ${response.statusText}`, - ); + throw await createUploadError(input.url, response); } } From 04a9fefffdb7b0e8f348d842f312978b8b3f9504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Thu, 11 Jun 2026 11:28:22 +0200 Subject: [PATCH 2/3] test(cli): raise upload e2e timeout to avoid flakes The upload e2e globs the whole __fixtures__ dir, including a 10MB PNG that is sharp-optimized, hashed, and uploaded to S3 over the network. That work sits right at the previous 15s limit and timed out in CI. Raise it to 30s, consistent with the sibling skip test's 20s budget. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/e2e/upload.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/e2e/upload.test.js b/packages/cli/e2e/upload.test.js index 9d092755..1fd784bd 100644 --- a/packages/cli/e2e/upload.test.js +++ b/packages/cli/e2e/upload.test.js @@ -4,7 +4,10 @@ import { getRequiredEnv, run } from "./utils.js"; getRequiredEnv("ARGOS_TOKEN"); -test("upload returns a full build URL", { timeout: 15_000 }, () => { +// This test uploads the full __fixtures__ directory, which includes a 10MB PNG +// stress fixture. That file is sharp-optimized, hashed, and uploaded to S3 over +// a real network connection, so a generous timeout is required to avoid flakes. +test("upload returns a full build URL", { timeout: 30_000 }, () => { const buildName = `argos-cli-e2e-node-${process.env.NODE_VERSION}-${process.env.OS}`; const uploadResult = run([ "upload", From aede7f49718b8fbe0a45bdbce34238ddcd65ff79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Thu, 11 Jun 2026 11:37:23 +0200 Subject: [PATCH 3/3] fix(core): trim S3 error captures and handle empty statusText Address review feedback: trim the Code/Message inner text (pretty-printed XML leaves surrounding whitespace) and only append response.statusText when present, since it's often empty in Node and would yield "403 ". Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/s3.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/core/src/s3.ts b/packages/core/src/s3.ts index d0707e8e..28837e04 100644 --- a/packages/core/src/s3.ts +++ b/packages/core/src/s3.ts @@ -28,12 +28,14 @@ interface PresignedPostUploadInput extends UploadInput { async function readS3ErrorMessage(response: Response): Promise { try { const body = await response.text(); - const code = body.match(/(.*?)<\/Code>/)?.[1]; - const message = body.match(/(.*?)<\/Message>/)?.[1]; + // Trim captures: pretty-printed XML can leave whitespace/newlines around + // the inner text. `[\s\S]` matches across lines in case the value wraps. + const code = body.match(/([\s\S]*?)<\/Code>/)?.[1]?.trim(); + const message = body.match(/([\s\S]*?)<\/Message>/)?.[1]?.trim(); if (code && message) { return `${code}: ${message}`; } - return message ?? code ?? null; + return message || code || null; } catch { return null; } @@ -48,7 +50,11 @@ async function createUploadError( response: Response, ): Promise { const detail = await readS3ErrorMessage(response); - const status = `${response.status} ${response.statusText}`; + // `statusText` is often empty in Node (e.g. HTTP/2), so only append it when + // present to avoid a trailing space like "403 ". + const status = response.statusText + ? `${response.status} ${response.statusText}` + : `${response.status}`; return new Error( `Failed to upload file to ${url}: ${status}${detail ? ` — ${detail}` : ""}`, );