From 5eea00abaddafda50df726969218328b9cdb8547 Mon Sep 17 00:00:00 2001 From: Isaac Stevens Date: Tue, 13 Oct 2026 14:13:13 -0700 Subject: [PATCH 1/3] fix(file-uploader): register the pdfjs worker when no workerSrc is set pdfjs-dist 4.x has no main-thread fallback without a worker: getDocument() throws 'No "GlobalWorkerOptions.workerSrc" specified.' unless globalThis.pdfjsWorker is set, so every PDF upload failed with CannotOpenPDF in apps that never set PdfService.workerSrc. Import the legacy worker module and register it in that case. Keep both imports behind .catch() so pdfjs-dist stays an optional peer, and cache pdfjs only after the worker is registered so concurrent and retried calls work. --- .../file-uploader/pdf.service.spec.ts | 113 +++++++++++++++++- .../components/file-uploader/pdf.service.ts | 30 +++-- 2 files changed, 132 insertions(+), 11 deletions(-) diff --git a/src/lib/components/file-uploader/pdf.service.spec.ts b/src/lib/components/file-uploader/pdf.service.spec.ts index c5232d4..a3154fb 100644 --- a/src/lib/components/file-uploader/pdf.service.spec.ts +++ b/src/lib/components/file-uploader/pdf.service.spec.ts @@ -8,9 +8,44 @@ const pdfjsMock = { getDocument: jest.fn(), }; -jest.mock('pdfjs-dist/legacy/build/pdf.mjs', () => pdfjsMock, { - virtual: true, -}); +// Hooks called from inside the module factories, so a test can act while load() +// is part-way through its imports. +let onPdfjsImport: (() => void) | undefined; +let onWorkerImport: (() => unknown) | undefined; + +jest.mock( + 'pdfjs-dist/legacy/build/pdf.mjs', + () => { + onPdfjsImport?.(); + return pdfjsMock; + }, + { virtual: true } +); + +const globalWithWorker = () => + globalThis as { pdfjsWorker?: { WorkerMessageHandler?: unknown } }; + +const pdfjsWorkerMock = { WorkerMessageHandler: {} }; + +jest.mock( + 'pdfjs-dist/legacy/build/pdf.worker.mjs', + () => onWorkerImport?.() ?? pdfjsWorkerMock, + { virtual: true } +); + +const noWorkerMessage = 'No "GlobalWorkerOptions.workerSrc" specified.'; + +// Mirrors the check in pdfjs-dist 4.10.38 legacy/build/pdf.mjs (lines 17493-17505): +// getDocument() throws unless a worker handler is on globalThis or workerSrc is set. +const pdfjsLikeGetDocument = () => { + if ( + !globalWithWorker().pdfjsWorker?.WorkerMessageHandler && + !pdfjsMock.GlobalWorkerOptions.workerSrc + ) { + throw new Error(noWorkerMessage); + } + return { promise: Promise.resolve('doc') }; +}; describe('PdfService', () => { let service: PdfService; @@ -19,6 +54,12 @@ describe('PdfService', () => { TestBed.configureTestingModule({}); service = TestBed.inject(PdfService); PdfService.workerSrc = undefined; + onPdfjsImport = undefined; + onWorkerImport = undefined; + // Fresh registry so the module factories (and their hooks) run again in each + // test. + jest.resetModules(); + delete globalWithWorker().pdfjsWorker; pdfjsMock.GlobalWorkerOptions.workerSrc = ''; pdfjsMock.getDocument.mockReset(); pdfjsMock.getDocument.mockReturnValue({ promise: Promise.resolve('doc') }); @@ -50,12 +91,76 @@ describe('PdfService', () => { ); }); - it('should leave pdfjs alone when no workerSrc is configured', async () => { + it('should not register the bundled worker when a workerSrc is configured', async () => { + PdfService.workerSrc = 'https://example.test/pdf.worker.mjs'; + + await service.getDocument(new ArrayBuffer(8)); + + expect(globalWithWorker().pdfjsWorker).toBeUndefined(); + }); + + it('should register the bundled worker for pdfjs when no workerSrc is configured', async () => { pdfjsMock.GlobalWorkerOptions.workerSrc = 'untouched'; await service.getDocument(new ArrayBuffer(8)); expect(pdfjsMock.GlobalWorkerOptions.workerSrc).toBe('untouched'); + expect(globalWithWorker().pdfjsWorker?.WorkerMessageHandler).toBe( + pdfjsWorkerMock.WorkerMessageHandler + ); + }); + + it('should not call pdfjs getDocument before the worker is registered, even for concurrent first calls', async () => { + pdfjsMock.getDocument.mockImplementation(pdfjsLikeGetDocument); + // Holds the worker import open, like a worker chunk still downloading. The + // gate matters: with the second call made inside the factory the race window + // is a couple of microtasks, and an early-cache mutant passes once + // getDocument() gains two awaits. + let releaseWorker!: () => void; + const workerFetched = new Promise((r) => (releaseWorker = r)); + onWorkerImport = () => ({ + __esModule: true, + then: (resolve: (m: unknown) => void) => + workerFetched.then(() => resolve(pdfjsWorkerMock)), + }); + const settle = () => new Promise((r) => setTimeout(r)); + + const first = service.getDocument(new ArrayBuffer(8)); + await settle(); + const second = service.getDocument(new ArrayBuffer(8)); + await settle(); + releaseWorker(); + + await expect(Promise.all([first, second])).resolves.toEqual([ + 'doc', + 'doc', + ]); + }); + + it('should retry the worker import on a later call after it failed', async () => { + onWorkerImport = () => { + onWorkerImport = undefined; + throw new Error('worker chunk failed'); + }; + + await expect(service.getDocument(new ArrayBuffer(8))).rejects.toThrow( + 'uploading a PDF needs pdfjs-dist' + ); + + await service.getDocument(new ArrayBuffer(8)); + expect(globalWithWorker().pdfjsWorker?.WorkerMessageHandler).toBe( + pdfjsWorkerMock.WorkerMessageHandler + ); + }); + + it('should report a missing pdfjs-dist when the pdfjs import fails', async () => { + onPdfjsImport = () => { + throw new Error('Cannot find module'); + }; + + await expect(service.getDocument(new ArrayBuffer(8))).rejects.toThrow( + 'moh-common-lib-angular: uploading a PDF needs pdfjs-dist.' + ); }); // Caching is observable: load() only touches pdfjs behind its `if (!this.pdfjs)` diff --git a/src/lib/components/file-uploader/pdf.service.ts b/src/lib/components/file-uploader/pdf.service.ts index 06aeacd..8669650 100644 --- a/src/lib/components/file-uploader/pdf.service.ts +++ b/src/lib/components/file-uploader/pdf.service.ts @@ -18,25 +18,41 @@ export class PdfService { /** * Where pdfjs should load its worker from. Set this before the first PDF is - * processed. Left unset, pdfjs falls back to running on the main thread, - * which works but blocks while rendering. + * processed. Left unset, the worker is bundled with the app and pdfjs runs it + * on the main thread, which works but blocks while rendering. */ static workerSrc: string | undefined; /** Loads pdfjs on first use, so it is not pulled in unless a PDF is opened. */ private async load(): Promise { if (!this.pdfjs) { - // The .catch() is what keeps this optional. Without it a bundler resolves - // the specifier at build time and fails when pdfjs-dist is absent. - this.pdfjs = await import('pdfjs-dist/legacy/build/pdf.mjs').catch(() => { + // The .catch() on each import is what keeps pdfjs-dist optional. Without + // it a bundler resolves the specifier at build time and fails when + // pdfjs-dist is absent. + const missing = () => { throw new Error( 'moh-common-lib-angular: uploading a PDF needs pdfjs-dist. ' + 'Install it, or restrict common-file-uploader to images.' ); - }); + }; + const pdfjs = await import('pdfjs-dist/legacy/build/pdf.mjs').catch( + missing + ); if (PdfService.workerSrc) { - this.pdfjs.GlobalWorkerOptions.workerSrc = PdfService.workerSrc; + pdfjs.GlobalWorkerOptions.workerSrc = PdfService.workerSrc; + } else { + // With no workerSrc, pdfjs reads globalThis.pdfjsWorker and runs that + // handler on the main thread; with neither, getDocument() throws + // 'No "GlobalWorkerOptions.workerSrc" specified.' + const worker = await import( + // @ts-expect-error pdfjs-dist ships no type declarations for the worker + 'pdfjs-dist/legacy/build/pdf.worker.mjs' + ).catch(missing); + (globalThis as { pdfjsWorker?: unknown }).pdfjsWorker = worker; } + // Cached last, so a concurrent or retried call never sees pdfjs without + // its worker. + this.pdfjs = pdfjs; } return this.pdfjs; } From 009015533a2660f7c9a6742c6339822fecf5b2d0 Mon Sep 17 00:00:00 2001 From: Isaac Stevens Date: Tue, 13 Oct 2026 14:13:56 -0700 Subject: [PATCH 2/3] chore(release): 2.5.3 --- CHANGELOG.md | 37 ++++++++++++++++++++++++++++++++ package-lock.json | 4 ++-- package.json | 2 +- projects/common-lib/package.json | 2 +- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92ddbe6..c3d2d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +## 2.5.3 (2026-10-13) + +Fixes PDF uploads in `common-file-uploader`. In an app that never sets +`PdfService.workerSrc`, every PDF, valid or not, was rejected with the +`CannotOpenPDF` error ("That PDF cannot be opened, please upload a different +attachment." in MSP). Affects 2.1.0 through 2.5.2, every release that has +shipped `PdfService`. Image uploads were not affected. + +### Breaking + +- None. No input, output or public API changes, and the `pdfjs-dist` peer + range is unchanged. + +### Added + +- None. + +### Fixed + +- `PdfService` (`common-file-uploader`): with `workerSrc` unset, pdfjs-dist 4.x + has no main-thread fallback of its own. `getDocument()` threw + `No "GlobalWorkerOptions.workerSrc" specified.`, which the uploader reports + as `CannotOpenPDF`. The doc comment said pdfjs would fall back to the main + thread; it did not. The service now imports + `pdfjs-dist/legacy/build/pdf.worker.mjs` and registers it as + `globalThis.pdfjsWorker`, so pdfjs runs the worker on the main thread. The + import keeps its `.catch()`, so `pdfjs-dist` stays an optional peer. pdfjs is + cached only after the worker is registered, so selecting several PDFs at once + or retrying after a failed load works. +- Apps that set `PdfService.workerSrc` are unchanged. + +### Consumer action required + +- None to adopt. Apps on 2.1.0 through 2.5.2 that accept PDFs get the fix by + upgrading to 2.5.3. An app pinned by its lockfile must update the lockfile + (`npm install moh-common-lib-angular@2.5.3`). + ## 2.5.2 (2026-10-12) Fixes the error text a required `common-date` shows for an impossible day. With diff --git a/package-lock.json b/package-lock.json index 9876e3b..466252f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "moh-common-lib-angular", - "version": "2.5.2", + "version": "2.5.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "moh-common-lib-angular", - "version": "2.5.2", + "version": "2.5.3", "dependencies": { "@angular/common": "^19.2.20", "@angular/compiler": "^19.2.20", diff --git a/package.json b/package.json index 0301efc..cfddfc0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "moh-common-lib-angular", - "version": "2.5.2", + "version": "2.5.3", "scripts": { "ng": "ng", "start": "ng serve", diff --git a/projects/common-lib/package.json b/projects/common-lib/package.json index 71be8c0..28a4eee 100644 --- a/projects/common-lib/package.json +++ b/projects/common-lib/package.json @@ -1,6 +1,6 @@ { "name": "moh-common-lib-angular", - "version": "2.5.2", + "version": "2.5.3", "description": "Shared Angular component library for BC Ministry of Health applications.", "keywords": [ "bcgov", From b68f533092a60602ec763faa21975679b3dc5104 Mon Sep 17 00:00:00 2001 From: Isaac Stevens Date: Tue, 13 Oct 2026 14:40:43 -0700 Subject: [PATCH 3/3] test(file-uploader): open a real PDF through pdfjs's browser path with no workerSrc --- .../file-uploader/pdf.service.real.spec.ts | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/lib/components/file-uploader/pdf.service.real.spec.ts diff --git a/src/lib/components/file-uploader/pdf.service.real.spec.ts b/src/lib/components/file-uploader/pdf.service.real.spec.ts new file mode 100644 index 0000000..ce8cdaa --- /dev/null +++ b/src/lib/components/file-uploader/pdf.service.real.spec.ts @@ -0,0 +1,56 @@ +import { deserialize, serialize } from 'v8'; +import { PdfService } from './pdf.service'; + +// Minimal one-page PDF with a correct xref table. +const minimalPdf = (): Uint8Array => { + const objs = [ + '<< /Type /Catalog /Pages 2 0 R >>', + '<< /Type /Pages /Kids [3 0 R] /Count 1 >>', + '<< /Type /Page /Parent 2 0 R /MediaBox [0 0 10 10] >>', + ]; + let out = '%PDF-1.4\n'; + const offsets = objs.map((body, i) => { + const at = out.length; + out += `${i + 1} 0 obj\n${body}\nendobj\n`; + return at; + }); + const xref = out.length; + out += `xref\n0 ${objs.length + 1}\n0000000000 65535 f \n`; + offsets.forEach((o) => (out += `${String(o).padStart(10, '0')} 00000 n \n`)); + out += `trailer\n<< /Size ${objs.length + 1} /Root 1 0 R >>\nstartxref\n${xref}\n%%EOF\n`; + return new TextEncoder().encode(out); +}; + +// Real pdfjs-dist, no mocks. pdfjs decides at module load whether it is in +// Node (pdf.mjs:4967) and if so sets its own workerSrc and loads its own worker +// (pdf.mjs:17315-17317), which hides the browser-only workerSrc error. Jest +// reports a Node-like process, so process.toString is overridden while pdfjs +// first evaluates, forcing the browser path (pdf.mjs:17493-17513). +describe('PdfService with the real pdfjs-dist', () => { + it('opens a PDF in the browser path with workerSrc unset', async () => { + PdfService.workerSrc = undefined; + delete (globalThis as { pdfjsWorker?: unknown }).pdfjsWorker; + const own = Object.getOwnPropertyDescriptor(process, 'toString'); + Object.defineProperty(process, 'toString', { + value: () => '[object browser]', + configurable: true, + }); + // jsdom lacks structuredClone, which pdfjs's main-thread worker port calls. + const g = globalThis as { structuredClone?: unknown }; + const hadClone = g.structuredClone; + g.structuredClone = (v: unknown) => deserialize(serialize(v)); + const service = new PdfService(); + let doc: { numPages: number }; + try { + doc = await service.getDocument(minimalPdf()); + } finally { + g.structuredClone = hadClone; + if (own) { + Object.defineProperty(process, 'toString', own); + } else { + delete (process as any).toString; + } + } + expect(doc.numPages).toBe(1); + }); +});