diff --git a/src/lib/documents/__tests__/native-canvas-support.test.ts b/src/lib/documents/__tests__/native-canvas-support.test.ts index df978c965..df282df1d 100644 --- a/src/lib/documents/__tests__/native-canvas-support.test.ts +++ b/src/lib/documents/__tests__/native-canvas-support.test.ts @@ -115,6 +115,6 @@ describe("generateThumbnail behind a closed gate", () => { Buffer.from([0xff, 0xd8, 0xff]), "image/jpeg", ); - expect(result).toEqual({ ok: false }); + expect(result).toEqual({ ok: false, reason: "native-canvas-unsupported" }); }); }); diff --git a/src/lib/documents/__tests__/thumbnail.test.ts b/src/lib/documents/__tests__/thumbnail.test.ts index 59a03ab64..05a2630d6 100644 --- a/src/lib/documents/__tests__/thumbnail.test.ts +++ b/src/lib/documents/__tests__/thumbnail.test.ts @@ -126,19 +126,19 @@ describe("generateThumbnail", () => { it("returns { ok: false } for an unsupported MIME type", async () => { await expect( generateThumbnail(Buffer.from("plain text"), "text/plain"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "unsupported-type" }); }); it("returns { ok: false } and never throws on malformed image bytes", async () => { await expect( generateThumbnail(Buffer.from("not a real image"), "image/png"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "error" }); }); it("returns { ok: false } on a malformed PDF", async () => { await expect( generateThumbnail(Buffer.from("not a pdf"), "application/pdf"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "raster-failed" }); }); it("refuses a decompression bomb before decoding (pixel cap)", async () => { @@ -153,6 +153,7 @@ describe("generateThumbnail", () => { bomb.writeUInt32BE(30_000, 20); // height await expect(generateThumbnail(bomb, "image/png")).resolves.toEqual({ ok: false, + reason: "pixel-cap", }); }); @@ -164,6 +165,6 @@ describe("generateThumbnail", () => { loadImageMock.mockResolvedValueOnce({ width: 30_000, height: 30_000 }); await expect( generateThumbnail(Buffer.from("desynced jpeg bytes"), "image/jpeg"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "pixel-cap" }); }); }); diff --git a/src/lib/documents/thumbnail.ts b/src/lib/documents/thumbnail.ts index 8d975954f..26ed69c1b 100644 --- a/src/lib/documents/thumbnail.ts +++ b/src/lib/documents/thumbnail.ts @@ -157,9 +157,23 @@ export interface Thumbnail { height: number; } +/** + * Why no preview was made. `unsupported-type` is the expected answer for an + * Office file or a text document; every other reason means a preview should + * have existed, so the thumbnail job logs it as a warning. + */ +export type ThumbnailSkipReason = + | "unsupported-type" + | "native-canvas-unsupported" + | "pixel-cap" + | "raster-failed" + | "empty-render" + | "error"; + /** Best-effort generation outcome — never an exception. */ export type ThumbnailResult = - { ok: true; thumbnail: Thumbnail } | { ok: false }; + | { ok: true; thumbnail: Thumbnail } + | { ok: false; reason: ThumbnailSkipReason }; // Minimal structural types for the slice of `@napi-rs/canvas` this module uses. // A type-only shape (erased at build) so we never eagerly evaluate the module @@ -229,7 +243,7 @@ export async function generateThumbnail( action: { name: "documents.thumbnail.rejected" }, meta: { reason: "native_canvas_unsupported" }, }); - return { ok: false }; + return { ok: false, reason: "native-canvas-unsupported" }; } // Lazy import: keeps @napi-rs/canvas (+ transitively pdfjs on the PDF path) // out of the eager server chunk. Resolved on first generation, then cached. @@ -244,7 +258,7 @@ export async function generateThumbnail( action: { name: "documents.thumbnail.rejected" }, meta: { source: "image", reason: "pixel_cap", pixels }, }); - return { ok: false }; + return { ok: false, reason: "pixel-cap" }; } const image = await napi.loadImage(bytes); // Post-decode backstop: the header sniff returns null on any marker @@ -260,10 +274,10 @@ export async function generateThumbnail( pixels: image.width * image.height, }, }); - return { ok: false }; + return { ok: false, reason: "pixel-cap" }; } const thumb = drawScaledJpeg(napi, image); - if (!thumb) return { ok: false }; + if (!thumb) return { ok: false, reason: "empty-render" }; annotate({ action: { name: "documents.thumbnail.ok" }, meta: { source: "image", width: thumb.width, height: thumb.height }, @@ -274,11 +288,13 @@ export async function generateThumbnail( if (mimeType === "application/pdf") { // Page 1 only — a preview never renders the whole document. const raster = await rasterizePdf(bytes, 1); - if (!raster.ok || raster.images.length === 0) return { ok: false }; + if (!raster.ok || raster.images.length === 0) { + return { ok: false, reason: "raster-failed" }; + } const pageJpeg = Buffer.from(raster.images[0]!.dataBase64, "base64"); const image = await napi.loadImage(pageJpeg); const thumb = drawScaledJpeg(napi, image); - if (!thumb) return { ok: false }; + if (!thumb) return { ok: false, reason: "empty-render" }; annotate({ action: { name: "documents.thumbnail.ok" }, meta: { source: "pdf", width: thumb.width, height: thumb.height }, @@ -287,7 +303,7 @@ export async function generateThumbnail( } // Every other MIME (Office/text/TIFF/HEIC/XML/JSON) — no preview. - return { ok: false }; + return { ok: false, reason: "unsupported-type" }; } catch (err) { annotate({ action: { name: "documents.thumbnail.failed" }, @@ -297,6 +313,6 @@ export async function generateThumbnail( message: err instanceof Error ? err.message.slice(0, 300) : String(err), }, }); - return { ok: false }; + return { ok: false, reason: "error" }; } } diff --git a/src/lib/jobs/__tests__/document-thumbnail.test.ts b/src/lib/jobs/__tests__/document-thumbnail.test.ts new file mode 100644 index 000000000..4270507d1 --- /dev/null +++ b/src/lib/jobs/__tests__/document-thumbnail.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * GitHub #1124 — the thumbnail job ran 355 times on one instance, left + * sixteen PDFs without a preview, and the log held not one line about it: + * the job opened no wide event, so every annotation went nowhere. Pins: each + * run is a `job.document_thumbnail` background event; a run that should have + * made a preview and did not is raised to `warn` with the reason; a type that + * never has a preview stays at info. + */ + +const { evt, findFirst, upsert, generateThumbnail, withBackgroundEvent } = + vi.hoisted(() => { + const evt = { addMeta: vi.fn(), elevateLevel: vi.fn() }; + return { + evt, + findFirst: vi.fn(), + upsert: vi.fn(), + generateThumbnail: vi.fn(), + withBackgroundEvent: vi.fn( + async (_name: string, fn: (e: typeof evt) => Promise) => + fn(evt), + ), + }; + }); + +vi.mock("@/lib/db", () => ({ + prisma: { + inboundDocument: { findFirst }, + documentThumbnail: { upsert }, + }, +})); +vi.mock("@/lib/documents/store", () => ({ + decryptDocumentContent: vi.fn(() => Buffer.from("%PDF-1.7")), + encryptThumbnail: vi.fn(() => new Uint8Array([1])), +})); +vi.mock("@/lib/documents/thumbnail", () => ({ generateThumbnail })); +vi.mock("@/lib/jobs/boss-instance", () => ({ getGlobalBoss: vi.fn() })); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/logging/background", () => ({ withBackgroundEvent })); + +import { runDocumentThumbnail } from "../document-thumbnail"; + +const DOC = { + id: "doc-1", + mimeType: "application/pdf", + contentEncrypted: new Uint8Array([1]), + contentCodec: "binary2", + thumbnail: null, +}; + +beforeEach(() => { + vi.clearAllMocks(); + findFirst.mockResolvedValue(DOC); +}); + +describe("runDocumentThumbnail wide event", () => { + it("runs inside a background event named for the job", async () => { + generateThumbnail.mockResolvedValue({ ok: false, reason: "raster-failed" }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(withBackgroundEvent).toHaveBeenCalledWith( + "job.document_thumbnail", + expect.any(Function), + ); + expect(evt.addMeta).toHaveBeenCalledWith("document_id", "doc-1"); + }); + + it("warns with the reason when a PDF gets no preview", async () => { + generateThumbnail.mockResolvedValue({ ok: false, reason: "error" }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(evt.elevateLevel).toHaveBeenCalledWith("warn"); + expect(evt.addMeta).toHaveBeenCalledWith( + "thumbnail_missing_reason", + "error", + ); + expect(upsert).not.toHaveBeenCalled(); + }); + + it("stays at info for a type that never has a preview", async () => { + findFirst.mockResolvedValue({ ...DOC, mimeType: "text/plain" }); + generateThumbnail.mockResolvedValue({ + ok: false, + reason: "unsupported-type", + }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(evt.elevateLevel).not.toHaveBeenCalled(); + }); + + it("stores the preview and stays at info when one is made", async () => { + generateThumbnail.mockResolvedValue({ + ok: true, + thumbnail: { jpeg: Buffer.from([0xff, 0xd8]), width: 2, height: 3 }, + }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(upsert).toHaveBeenCalledOnce(); + expect(evt.elevateLevel).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/jobs/document-thumbnail.ts b/src/lib/jobs/document-thumbnail.ts index 91a8d44ed..1b0a1a91d 100644 --- a/src/lib/jobs/document-thumbnail.ts +++ b/src/lib/jobs/document-thumbnail.ts @@ -15,6 +15,12 @@ * kind icon), and `runDocumentThumbnail` returns cleanly so pg-boss does not * retry it. Only an unexpected throw (a DB hiccup) propagates for retry. * + * Each run is one background wide event (`job.document_thumbnail`). A run + * that should have produced a preview and did not is raised to `warn` with + * the reason, so a document left with its kind icon is explained in the log. + * Without the event every annotation below went nowhere: a whole class of + * PDFs went without previews for weeks and the log never said so. + * * The queue MUST be registered in the maintenance registrar * (`src/lib/jobs/reminder/register-maintenance.ts`) so pg-boss provisions it. */ @@ -25,6 +31,7 @@ import { } from "@/lib/documents/store"; import { generateThumbnail } from "@/lib/documents/thumbnail"; import { getGlobalBoss } from "@/lib/jobs/boss-instance"; +import { withBackgroundEvent } from "@/lib/logging/background"; import { annotate } from "@/lib/logging/context"; export const DOCUMENT_THUMBNAIL_QUEUE = "document-thumbnail"; @@ -49,7 +56,20 @@ export async function runDocumentThumbnail( ): Promise { const { userId, documentId } = payload; if (!userId || !documentId) return; + await withBackgroundEvent("job.document_thumbnail", async (evt) => { + evt.addMeta("document_id", documentId); + await renderThumbnail(userId, documentId, (reason) => { + evt.addMeta("thumbnail_missing_reason", reason); + evt.elevateLevel("warn"); + }); + }); +} +async function renderThumbnail( + userId: string, + documentId: string, + warn: (reason: string) => void, +): Promise { // Owner-scoped load; the blob column is selected ONLY here (the job that // renders the preview), never in the list/detail queries. Skip a document // that already has a thumbnail so a re-enqueue is a cheap no-op. @@ -84,6 +104,7 @@ export async function runDocumentThumbnail( action: { name: "documents.thumbnail.decryptFailed" }, meta: { documentId }, }); + warn("decrypt-failed"); return; } @@ -91,8 +112,11 @@ export async function runDocumentThumbnail( if (!result.ok) { annotate({ action: { name: "documents.thumbnail.run" }, - meta: { documentId, generated: false }, + meta: { documentId, generated: false, mimeType: document.mimeType }, }); + // An Office or text document has no preview by design; anything else + // was meant to get one. + if (result.reason !== "unsupported-type") warn(result.reason); return; }