Release/2.5.3: fix PDF uploads in common-file-uploader

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

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",

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",

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);
+  });
+});

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<void>((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)`

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<any> {
     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;
   }