diff --git a/.server-changes/fix-transcript-404-empty-reason-phrase.md b/.server-changes/fix-transcript-404-empty-reason-phrase.md new file mode 100644 index 00000000000..3d129cdcac6 --- /dev/null +++ b/.server-changes/fix-transcript-404-empty-reason-phrase.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Fix missing session transcripts returning a download error instead of a not-found response diff --git a/apps/webapp/app/services/realtime/transcriptDownload.server.test.ts b/apps/webapp/app/services/realtime/transcriptDownload.server.test.ts index 6768b6eb0f1..4cd9c1e80da 100644 --- a/apps/webapp/app/services/realtime/transcriptDownload.server.test.ts +++ b/apps/webapp/app/services/realtime/transcriptDownload.server.test.ts @@ -1,4 +1,5 @@ import { createServer, type Server } from "node:http"; +import { createServer as createTcpServer, type Server as TcpServer } from "node:net"; import { once } from "node:events"; import express from "express"; import { sendRemixResponse } from "@remix-run/express/dist/server"; @@ -114,6 +115,34 @@ test("does not treat permission or service failures as an absent transcript", () ).toBe(false); expect(isTranscriptNotFound({ $metadata: { httpStatusCode: 503 } })).toBe(false); expect(isTranscriptNotFound({ name: "NoSuchKey" })).toBe(true); + expect(isTranscriptNotFound({ status: 404, message: "Failed to download from object store: " })).toBe( + true + ); +}); + +test("recognizes a 404 with an empty reason phrase as a missing transcript", async () => { + const server: TcpServer = createTcpServer((socket) => { + socket.once("data", () => { + socket.end("HTTP/1.1 404 \r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + }); + }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Expected TCP listener"); + const baseUrl = `http://127.0.0.1:${address.port}`; + try { + const client = ObjectStoreClient.create({ + baseUrl, + accessKeyId: "test", + secretAccessKey: "test", + service: "s3", + }); + await expect(client.getObjectResponse(key)).rejects.toSatisfy(isTranscriptNotFound); + } finally { + server.close(); + await once(server, "close"); + } }); minioTest( diff --git a/apps/webapp/app/services/realtime/transcriptDownload.server.ts b/apps/webapp/app/services/realtime/transcriptDownload.server.ts index da477a9ca5a..48689d06699 100644 --- a/apps/webapp/app/services/realtime/transcriptDownload.server.ts +++ b/apps/webapp/app/services/realtime/transcriptDownload.server.ts @@ -45,8 +45,16 @@ export function downloadTranscript( export function isTranscriptNotFound(error: unknown): boolean { if (!error || typeof error !== "object") return false; - const { name, $metadata } = error as { name?: string; $metadata?: { httpStatusCode?: number } }; + const { name, $metadata, status } = error as { + name?: string; + $metadata?: { httpStatusCode?: number }; + status?: unknown; + }; if (name === "NoSuchKey" || name === "NotFound" || $metadata?.httpStatusCode === 404) return true; + // The aws4fetch adapter reports the HTTP status on the error. Check it first: + // S3-compatible stores may return an empty reason phrase, so the message alone + // cannot distinguish a 404 from other failures. + if (status === 404) return true; // The aws4fetch adapter currently reports the HTTP status text in its error. return ( error instanceof Error && diff --git a/apps/webapp/app/v3/objectStoreClient.server.ts b/apps/webapp/app/v3/objectStoreClient.server.ts index f7b921be0a5..acd7e039321 100644 --- a/apps/webapp/app/v3/objectStoreClient.server.ts +++ b/apps/webapp/app/v3/objectStoreClient.server.ts @@ -58,6 +58,24 @@ export class ObjectVersionChangedError extends Error { } } +/** + * A failed object-store download that preserves the HTTP status code. + * Callers must not rely on `statusText`: S3-compatible stores may return an + * empty reason phrase (e.g. `HTTP/1.1 404 `), which yields an empty message + * suffix and breaks message-based 404 detection. + */ +export class ObjectStoreDownloadError extends Error { + readonly status: number; + readonly statusText: string; + + constructor(message: string, status: number, statusText: string) { + super(message); + this.name = "ObjectStoreDownloadError"; + this.status = status; + this.statusText = statusText; + } +} + /** `Range` header value for a byte range or a suffix. */ function rangeHeader(range: { suffixLength: number } | { start: number; end: number }): string { return "suffixLength" in range @@ -120,7 +138,11 @@ class Aws4FetchClient implements IObjectStoreClient { async getObject(key: string): Promise { const response = await this.awsClient.fetch(this.buildUrl(key)); if (!response.ok) { - throw new Error(`Failed to download from object store: ${response.statusText}`); + throw new ObjectStoreDownloadError( + `Failed to download from object store: ${response.statusText}`, + response.status, + response.statusText + ); } return response.text(); } @@ -135,7 +157,11 @@ class Aws4FetchClient implements IObjectStoreClient { async getObjectResponse(key: string): Promise { const response = await this.awsClient.fetch(this.buildUrl(key)); if (!response.ok) { - throw new Error(`Failed to download from object store: ${response.statusText}`); + throw new ObjectStoreDownloadError( + `Failed to download from object store: ${response.statusText}`, + response.status, + response.statusText + ); } return response; } @@ -155,7 +181,11 @@ class Aws4FetchClient implements IObjectStoreClient { throw new ObjectVersionChangedError(key); } if (!response.ok) { - throw new Error(`Failed to download range from object store: ${response.statusText}`); + throw new ObjectStoreDownloadError( + `Failed to download range from object store: ${response.statusText}`, + response.status, + response.statusText + ); } const bytes = new Uint8Array(await response.arrayBuffer()); return {