diff --git a/src/classifier/coerce-workflow-data.ts b/src/classifier/coerce-workflow-data.ts index 7c3c146a..168e6f13 100644 --- a/src/classifier/coerce-workflow-data.ts +++ b/src/classifier/coerce-workflow-data.ts @@ -256,7 +256,7 @@ function coerceEnumValue(raw: unknown, enumValues: Array<{ value: string }>): st * Formats for human-readable date parsing (first match wins after ISO). * Slash-separated numeric formats are explicitly excluded — they are ambiguous. */ -const DATE_FORMATS_WITH_YEAR = [ +const BASE_DATE_FORMATS_WITH_YEAR = [ "d MMMM yyyy", "MMMM d, yyyy", "d MMM yyyy", @@ -264,13 +264,39 @@ const DATE_FORMATS_WITH_YEAR = [ "dd.MM.yyyy", ]; -const DATE_FORMATS_YEARFREE = [ +const BASE_DATE_FORMATS_YEARFREE = [ "d MMMM", "MMMM d", "d MMM", "MMM d", ]; +/** + * Expands a format list with a leading-weekday variant of each entry, using luxon's + * own "ccc"/"cccc" weekday tokens rather than an enumerated word list — those tokens + * resolve locale-specific weekday names via Intl (same mechanism already relied on for + * MMM/MMMM month names below), so this covers "Fri, ...", "Friday, ...", and their + * equivalents in any locale hint without us hardcoding weekday names per language. + */ +function withWeekdayPrefix(formats: string[]): string[] { + return formats.flatMap(fmt => [fmt, `ccc, ${fmt}`, `cccc, ${fmt}`, `ccc ${fmt}`, `cccc ${fmt}`]); +} + +/** + * Expands a format list with a trailing-period variant for any bare "MMM" token (not + * "MMMM"), e.g. "MMM d, yyyy" → also try "MMM. d, yyyy". Many locales abbreviate months + * with a trailing period (English "Nov.", French "janv."); luxon's MMM token already + * resolves the locale-specific abbreviation itself, so adding the period as a literal + * in the format string covers it without us enumerating month abbreviations. + */ +function withAbbrevMonthPeriod(formats: string[]): string[] { + const bareMmm = /(? (bareMmm.test(fmt) ? [fmt, fmt.replace(bareMmm, "MMM.")] : [fmt])); +} + +const DATE_FORMATS_WITH_YEAR = withWeekdayPrefix(withAbbrevMonthPeriod(BASE_DATE_FORMATS_WITH_YEAR)); +const DATE_FORMATS_YEARFREE = withWeekdayPrefix(withAbbrevMonthPeriod(BASE_DATE_FORMATS_YEARFREE)); + const TIME_SUFFIXES = ["", " HH:mm", " h:mm a", " 'at' HH:mm", " 'at' h:mm a"]; /** Pattern to detect slash-separated numeric dates (e.g. 01/02/2025, 1/2/25). */ diff --git a/src/isolated/content-sanitizer.ts b/src/isolated/content-sanitizer.ts index 8453b004..ac8e025a 100644 --- a/src/isolated/content-sanitizer.ts +++ b/src/isolated/content-sanitizer.ts @@ -36,6 +36,29 @@ interface DroppedAttachment { mimeType: string; sizeBytes: number; reason: "too_large" | "upload_failed"; + /** Populated for "upload_failed" — the HTTP status or caught error that made the S3 upload fail. */ + detail?: string; +} + +/** + * Builds a human-readable reason summary for the TRACK log title — not just the payload. + * Reasons carrying a `detail` (currently only "upload_failed") are listed individually + * with their full detail text, since that's the actual "why" and different attachments + * can fail for different reasons; reasons with no further detail (e.g. "too_large") are + * collapsed into a single count. + */ +function summarizeDroppedReasons(dropped: { reason: string; detail?: string }[]): string { + const parts: string[] = []; + const countsByReason = new Map(); + for (const d of dropped) { + if (d.detail) { + parts.push(`${d.reason}: ${d.detail}`); + } else { + countsByReason.set(d.reason, (countsByReason.get(d.reason) ?? 0) + 1); + } + } + for (const [reason, count] of countsByReason) parts.push(`${count} ${reason}`); + return parts.join("; "); } interface InlineImageRef { @@ -155,7 +178,7 @@ async function uploadViaPresignedPost( content: Buffer | Uint8Array, contentType: string, retentionTag: "365" | "3650" | null, -): Promise { +): Promise<{ ok: true } | { ok: false; detail: string }> { const formData = new FormData(); // Add all pre-signed fields @@ -179,9 +202,19 @@ async function uploadViaPresignedPost( method: "POST", body: formData, }); - return response.ok || response.status === 204; - } catch { - return false; + if (response.ok || response.status === 204) return { ok: true }; + // S3 presigned POST failures are almost always an expired/mismatched policy + // (clock skew, a key/condition that no longer matches what the URL was signed + // for) — the body carries the actual S3 error code, worth surfacing. + let body = ""; + try { + body = await response.text(); + } catch { + // best-effort — fall back to the status alone below + } + return { ok: false, detail: `HTTP ${response.status}${body ? `: ${body.slice(0, 300)}` : ""}` }; + } catch (e) { + return { ok: false, detail: e instanceof Error ? e.message : "unknown fetch error" }; } } @@ -255,9 +288,9 @@ async function processEmail(event: ContentSanitizeRequest, logger?: Logger): Pro try { const displayRaw = buildDisplayRawEmail(rawMime.toString("latin1")); const key = `${event.keyPrefix}raw-display.eml`; - const uploaded = await uploadViaPresignedPost(event.presignedPost, key, Buffer.from(displayRaw, "latin1"), "message/rfc822", event.retentionTag); - if (uploaded) displayRawS3Key = key; - logger?.trackPoint("display_raw_build_complete", { uploaded }); + const uploadResult = await uploadViaPresignedPost(event.presignedPost, key, Buffer.from(displayRaw, "latin1"), "message/rfc822", event.retentionTag); + if (uploadResult.ok) displayRawS3Key = key; + logger?.trackPoint("display_raw_build_complete", { uploaded: uploadResult.ok, ...(uploadResult.ok ? {} : { detail: uploadResult.detail }) }); } catch { logger?.trackPoint("display_raw_build_failed"); } @@ -354,17 +387,17 @@ async function processEmail(event: ContentSanitizeRequest, logger?: Logger): Pro // attachment instead of embedding. Its `cid:` reference is deliberately left // unresolved below; the API layer resolves it to a CDN url at read time. const s3Key = `${event.keyPrefix}${uploadIndex}`; - const uploaded = await uploadViaPresignedPost(event.presignedPost, s3Key, attachment.content, contentType, event.retentionTag); + const uploadResult = await uploadViaPresignedPost(event.presignedPost, s3Key, attachment.content, contentType, event.retentionTag); const filename = attachment.filename ?? `inline-${uploadIndex}`; - if (uploaded) { + if (uploadResult.ok) { inlineImageRefs.push({ contentId: cid, mimeType: contentType, sizeBytes: attachment.size, s3Key }); // Still eligible for QR scanning via the attachment-image path in extractAssets if (contentType.startsWith("image/")) { attachmentsWithBytes.push({ filename, mimeType: contentType, content: attachment.content, s3Key }); } } else { - droppedAttachments.push({ filename, mimeType: contentType, sizeBytes: attachment.size, reason: "upload_failed" }); + droppedAttachments.push({ filename, mimeType: contentType, sizeBytes: attachment.size, reason: "upload_failed", detail: uploadResult.detail }); } uploadIndex++; } @@ -372,25 +405,26 @@ async function processEmail(event: ContentSanitizeRequest, logger?: Logger): Pro } const s3Key = `${event.keyPrefix}${uploadIndex}`; - const uploaded = await uploadViaPresignedPost(event.presignedPost, s3Key, attachment.content, contentType, event.retentionTag); + const uploadResult = await uploadViaPresignedPost(event.presignedPost, s3Key, attachment.content, contentType, event.retentionTag); const filename = attachment.filename ?? `attachment-${uploadIndex}`; - if (uploaded) { + if (uploadResult.ok) { attachmentRefs.push({ filename, mimeType: contentType, sizeBytes: attachment.size, s3Key }); attachmentsWithBytes.push({ filename, mimeType: contentType, content: attachment.content, s3Key }); } else { - droppedAttachments.push({ filename, mimeType: contentType, sizeBytes: attachment.size, reason: "upload_failed" }); + droppedAttachments.push({ filename, mimeType: contentType, sizeBytes: attachment.size, reason: "upload_failed", detail: uploadResult.detail }); } uploadIndex++; } logger?.trackPoint("attachments_processed", { attachmentRefCount: attachmentRefs.length, inlineImageCount: inlineImages.length, inlineImageRefCount: inlineImageRefs.length, droppedCount: droppedAttachments.length }); if (droppedAttachments.length > 0) { - logger?.track("Attachment(s) dropped from message.", { + const reasonSummary = summarizeDroppedReasons(droppedAttachments); + logger?.track(`Attachment(s) dropped from message: ${reasonSummary}`, { code: "content_sanitizer.attachments_dropped", accountId: event.accountId, droppedCount: droppedAttachments.length, - dropped: droppedAttachments.map(d => ({ mimeType: d.mimeType, sizeBytes: d.sizeBytes, reason: d.reason })), + dropped: droppedAttachments.map(d => ({ mimeType: d.mimeType, sizeBytes: d.sizeBytes, reason: d.reason, detail: d.detail })), }); } diff --git a/src/processor/content-sanitizer-client.ts b/src/processor/content-sanitizer-client.ts index a6886d95..24ddeb4d 100644 --- a/src/processor/content-sanitizer-client.ts +++ b/src/processor/content-sanitizer-client.ts @@ -25,6 +25,8 @@ export interface DroppedAttachment { mimeType: string; sizeBytes: number; reason: "too_large" | "upload_failed"; + /** Populated for "upload_failed" — the HTTP status or error that made the S3 upload fail. */ + detail?: string; } export interface InlineImageRef { diff --git a/src/processor/processor.ts b/src/processor/processor.ts index 3b14b58d..03a1fd67 100644 --- a/src/processor/processor.ts +++ b/src/processor/processor.ts @@ -153,6 +153,27 @@ interface ProcessingOutcome { doPong: boolean; } +/** + * Builds a human-readable reason summary for the dropped-attachments log title — not + * just the payload. Reasons carrying a `detail` (currently only "upload_failed") are + * listed individually with their full detail text, since that's the actual "why" and + * different attachments can fail for different reasons; reasons with no further detail + * (e.g. "too_large") are collapsed into a single count. + */ +function summarizeDroppedReasons(dropped: { reason: string; detail?: string }[]): string { + const parts: string[] = []; + const countsByReason = new Map(); + for (const d of dropped) { + if (d.detail) { + parts.push(`${d.reason}: ${d.detail}`); + } else { + countsByReason.set(d.reason, (countsByReason.get(d.reason) ?? 0) + 1); + } + } + for (const [reason, count] of countsByReason) parts.push(`${count} ${reason}`); + return parts.join("; "); +} + function emptyOutcome(): ProcessingOutcome { return { blockDisposition: null, @@ -888,11 +909,12 @@ export class SignalProcessor { const sanitizerAssets = sanitizedParsed.assets ?? []; if (sanitizedParsed.droppedAttachments && sanitizedParsed.droppedAttachments.length > 0) { - this.logger.warn("Message had attachment(s) dropped by content sanitizer", { + const reasonSummary = summarizeDroppedReasons(sanitizedParsed.droppedAttachments); + this.logger.warn(`Message had attachment(s) dropped by content sanitizer: ${reasonSummary}`, { code: "processor.attachments_dropped", accountId, droppedCount: sanitizedParsed.droppedAttachments.length, - dropped: sanitizedParsed.droppedAttachments.map(d => ({ mimeType: d.mimeType, sizeBytes: d.sizeBytes, reason: d.reason })), + dropped: sanitizedParsed.droppedAttachments.map(d => ({ mimeType: d.mimeType, sizeBytes: d.sizeBytes, reason: d.reason, detail: d.detail })), }); } diff --git a/tests/classifier/coerce-date.spec.ts b/tests/classifier/coerce-date.spec.ts index 7c0cfdb6..e042387a 100644 --- a/tests/classifier/coerce-date.spec.ts +++ b/tests/classifier/coerce-date.spec.ts @@ -322,3 +322,38 @@ describe("coerceDate — noise stripping safety", () => { expect(coerceDate("15/03/2025", RECEIVED_AT)).toBeNull(); }); }); + +// --------------------------------------------------------------------------- +// Weekday prefix + abbreviated-month period stripping +// --------------------------------------------------------------------------- + +describe("coerceDate — weekday prefix and abbreviated month period stripping", () => { + it("strips weekday prefix and period after abbreviated month with time", () => { + expect(coerceDate("Fri, Nov. 20, 2026 07:30", RECEIVED_AT)).toBe("2026-11-20T07:30"); + }); + + it("strips full weekday name prefix", () => { + expect(coerceDate("Friday, Nov. 20, 2026", RECEIVED_AT)).toBe("2026-11-20"); + }); + + it("strips period after abbreviated month without weekday prefix", () => { + expect(coerceDate("Nov. 20, 2026", RECEIVED_AT)).toBe("2026-11-20"); + }); + + it("strips weekday prefix without abbreviated month period", () => { + expect(coerceDate("Fri, Nov 20, 2026", RECEIVED_AT)).toBe("2026-11-20"); + }); + + it("handles weekday prefix on d MMM yyyy form", () => { + expect(coerceDate("Fri, 20 Nov. 2026", RECEIVED_AT)).toBe("2026-11-20"); + }); + + it("does not strip a leading month name mistaken for a weekday", () => { + expect(coerceDate("March 15, 2025", RECEIVED_AT)).toBe("2025-03-15"); + }); + + it("handles a French weekday prefix via locale hint — generic, not English-only", () => { + // "ven." (vendredi) — proves the weekday token is locale-resolved, not a hardcoded English list + expect(coerceDate("ven. 20 novembre 2026", RECEIVED_AT, ["fr"])).toBe("2026-11-20"); + }); +}); diff --git a/tests/isolated/content-sanitizer-attachments.spec.ts b/tests/isolated/content-sanitizer-attachments.spec.ts index 6c982264..ae12efb4 100644 --- a/tests/isolated/content-sanitizer-attachments.spec.ts +++ b/tests/isolated/content-sanitizer-attachments.spec.ts @@ -121,3 +121,63 @@ describe("content-sanitizer — oversized attachments", () => { expect(result.parsed.droppedAttachments).toBeUndefined(); }); }); + +describe("content-sanitizer — upload failures", () => { + function mockFetchWithFailingUpload(raw: string) { + const buf = Buffer.from(raw, "utf-8"); + vi.stubGlobal("fetch", vi.fn(async (url: string) => { + if (url === "https://example.com/get") { + return { + ok: true, + status: 200, + arrayBuffer: async () => buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength), + }; + } + // presigned POST upload fails with a realistic S3 error body + return { + ok: false, + status: 403, + text: async () => "AccessDeniedRequest has expired", + }; + })); + } + + it("captures the S3 failure detail and surfaces it in the log title, not just 'upload_failed'", async () => { + mockFetchWithFailingUpload(buildEmailWithOversizedAttachment(1024)); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const result = await handler({ + presignedGetUrl: "https://example.com/get", + presignedPost: { url: "https://example.com/post", fields: {} }, + accountId: "acct-test", + senderEtld1: "example.com", + keyPrefix: "emails/msg-upload-failed/", + retentionTag: null, + invocationId: "inv-upload-failed", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + expect(result.parsed.droppedAttachments).toEqual([ + { + filename: "huge.bin", + mimeType: "application/octet-stream", + sizeBytes: 1024, + reason: "upload_failed", + detail: "HTTP 403: AccessDeniedRequest has expired", + }, + ]); + + const trackEntry = logSpy.mock.calls + .map(call => call[0]) + .find((entry): entry is Record => + typeof entry === "object" && entry !== null && (entry as Record).code === "content_sanitizer.attachments_dropped"); + + expect(trackEntry).toBeDefined(); + const title = (trackEntry as Record).title; + expect(typeof title).toBe("string"); + expect(title as string).toContain("AccessDenied"); + expect(title as string).toContain("Request has expired"); + }); +});