fix(export): cancel PDF work when requesting document retires (#34907)

* fix(export): cancel PDF work when requesting document retires

* test(export): assert success on non-retired repeated export cycles

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

src/main/ipc/export.test.ts

@@ -0,0 +1,362 @@
+import { EventEmitter } from 'node:events'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { ExportHtmlToPdfArgs, ExportHtmlToPdfResult } from './export'
+
+type ExportHandler = (
+  event: { sender: EventEmitter },
+  args: ExportHtmlToPdfArgs
+) => Promise<ExportHtmlToPdfResult>
+
+const fake = vi.hoisted(() => ({
+  handlers: new Map<string, ExportHandler>(),
+  tempFiles: new Set<string>(),
+  savedFiles: new Map<string, Uint8Array>(),
+  windows: new Array<{ events: EventEmitter; destroyed: boolean }>(),
+  uuid: 0,
+  write:
+    vi.fn<
+      (path: string, data: string | Uint8Array, options?: { signal?: AbortSignal }) => Promise<void>
+    >(),
+  load: vi.fn<(events: EventEmitter, path: string) => Promise<void>>(),
+  images: vi.fn<() => Promise<void>>(),
+  print: vi.fn<() => Promise<Buffer>>(),
+  dialog: vi.fn<() => Promise<{ canceled: boolean; filePath?: string }>>()
+}))
+
+vi.mock('electron', () => ({
+  app: { getPath: () => '/mock-export-temp' },
+  ipcMain: {
+    removeHandler: () => {
+      fake.handlers.delete('export:html-to-pdf')
+    },
+    handle: (_channel: string, handler: ExportHandler) => {
+      fake.handlers.set('export:html-to-pdf', handler)
+    }
+  },
+  dialog: { showSaveDialog: fake.dialog },
+  BrowserWindow: class {
+    static fromWebContents() {
+      return { id: 1 }
+    }
+    events = new EventEmitter()
+    destroyed = false
+    webContents = Object.assign(this.events, {
+      executeJavaScript: fake.images,
+      printToPDF: fake.print
+    })
+    constructor() {
+      fake.windows.push(this)
+    }
+    loadFile(path: string) {
+      return fake.load(this.events, path)
+    }
+    isDestroyed() {
+      return this.destroyed
+    }
+    destroy() {
+      this.destroyed = true
+    }
+  }
+}))
+vi.mock('node:fs/promises', () => ({
+  writeFile: fake.write,
+  unlink: async (path: string) => {
+    fake.tempFiles.delete(path)
+  }
+}))
+vi.mock('node:crypto', () => ({ randomUUID: () => `export-${++fake.uuid}` }))
+
+import { registerExportHandlers } from './export'
+
+const PDF = Buffer.from('exported PDF snapshot')
+const ARGS = { html: '<p>captured markdown</p>', title: 'draft' }
+const GONE_EVENTS = ['render-process-gone', 'did-navigate', 'destroyed'] as const
+
+function exportFrom(sender: EventEmitter): Promise<ExportHtmlToPdfResult> {
+  const handler = fake.handlers.get('export:html-to-pdf')
+  if (!handler) {
+    throw new Error('Export handler was not registered')
+  }
+  return handler({ sender }, ARGS)
+}
+
+async function flush(): Promise<void> {
+  await vi.advanceTimersByTimeAsync(0)
+}
+
+function expectReleased(sender: EventEmitter): void {
+  expect(fake.tempFiles.size).toBe(0)
+  expect(fake.windows.every((window) => window.destroyed)).toBe(true)
+  expect(vi.getTimerCount()).toBe(0)
+  for (const event of GONE_EVENTS) {
+    expect(sender.listenerCount(event)).toBe(0)
+  }
+  for (const window of fake.windows) {
+    expect(window.events.listenerCount('did-finish-load')).toBe(0)
+    expect(window.events.listenerCount('did-fail-load')).toBe(0)
+  }
+}
+
+beforeEach(() => {
+  vi.useFakeTimers()
+  vi.resetAllMocks()
+  fake.tempFiles.clear()
+  fake.savedFiles.clear()
+  fake.windows.length = 0
+  fake.write.mockImplementation(async (path, data) => {
+    if (typeof data === 'string') {
+      fake.tempFiles.add(path)
+    } else {
+      fake.savedFiles.set(path, data)
+    }
+  })
+  fake.load.mockImplementation(async (events) => {
+    events.emit('did-finish-load')
+  })
+  fake.images.mockResolvedValue()
+  fake.print.mockResolvedValue(PDF)
+  fake.dialog.mockResolvedValue({ canceled: true })
+  registerExportHandlers()
+})
+
+afterEach(() => {
+  vi.useRealTimers()
+})
+
+describe('PDF export requesting document lifetime', () => {
+  it.each(GONE_EVENTS)('releases a hidden export on %s before images settle', async (event) => {
+    const images = Promise.withResolvers<void>()
+    fake.images.mockReturnValueOnce(images.promise)
+    const sender = new EventEmitter()
+    const result = exportFrom(sender)
+    await flush()
+    expect(fake.tempFiles.size).toBe(1)
+    expect(fake.windows).toHaveLength(1)
+
+    sender.emit(event)
+    await flush()
+    expectReleased(sender)
+    await expect(result).resolves.toEqual({ success: false, cancelled: true })
+    expectReleased(sender)
+    images.resolve()
+    await flush()
+    expect(fake.print).not.toHaveBeenCalled()
+    expect(fake.dialog).not.toHaveBeenCalled()
+  })
+
+  it.each(['load', 'load-event', 'images', 'print'] as const)(
+    'cleans up pending %s and observes its late rejection',
+    async (stage) => {
+      const pending = Promise.withResolvers<void>()
+      const printing = Promise.withResolvers<Buffer>()
+      if (stage === 'load') {
+        fake.load.mockReturnValueOnce(pending.promise)
+      }
+      if (stage === 'load-event') {
+        fake.load.mockResolvedValueOnce()
+      }
+      if (stage === 'images') {
+        fake.images.mockReturnValueOnce(pending.promise)
+      }
+      if (stage === 'print') {
+        fake.print.mockReturnValueOnce(printing.promise)
+      }
+      const sender = new EventEmitter()
+      const result = exportFrom(sender)
+      await flush()
+      sender.emit('render-process-gone')
+      await flush()
+      expectReleased(sender)
+      await expect(result).resolves.toEqual({ success: false, cancelled: true })
+      expectReleased(sender)
+
+      if (stage === 'print') {
+        printing.reject(new Error('late print failure'))
+      } else if (stage === 'load-event') {
+        fake.windows[0]?.events.emit('did-fail-load', {}, -3, 'late failure')
+      } else {
+        pending.reject(new Error('late renderer failure'))
+      }
+      await flush()
+      expect(fake.dialog).not.toHaveBeenCalled()
+      expect(fake.print).toHaveBeenCalledTimes(stage === 'print' ? 1 : 0)
+    }
+  )
+
+  it.each(['resolve', 'reject'] as const)(
+    'cancels initial document writing and cleans up after late %s',
+    async (settlement) => {
+      const writing = Promise.withResolvers<void>()
+      fake.write.mockImplementationOnce(async (path) => {
+        fake.tempFiles.add(path)
+        await writing.promise
+      })
+      const sender = new EventEmitter()
+      const result = exportFrom(sender)
+      await flush()
+      const signal = fake.write.mock.calls[0]?.[2]?.signal
+      expect(signal?.aborted).toBe(false)
+      sender.emit('did-navigate')
+      expect(signal?.aborted).toBe(true)
+      expect(fake.windows).toHaveLength(0)
+      expect(fake.tempFiles.size).toBe(1)
+
+      if (settlement === 'resolve') {
+        writing.resolve()
+      } else {
+        writing.reject(new Error('write aborted'))
+      }
+      await expect(result).resolves.toEqual({ success: false, cancelled: true })
+      expectReleased(sender)
+      expect(fake.windows).toHaveLength(0)
+      expect(fake.dialog).not.toHaveBeenCalled()
+    }
+  )
+
+  it('prevents a late load resolution from admitting image or print work', async () => {
+    const load = Promise.withResolvers<void>()
+    fake.load.mockReturnValueOnce(load.promise)
+    const sender = new EventEmitter()
+    const result = exportFrom(sender)
+    await flush()
+    sender.emit('destroyed')
+    await flush()
+    expectReleased(sender)
+    await expect(result).resolves.toEqual({ success: false, cancelled: true })
+    load.resolve()
+    fake.windows[0]?.events.emit('did-finish-load')
+    await flush()
+    expectReleased(sender)
+    expect(fake.images).not.toHaveBeenCalled()
+    expect(fake.print).not.toHaveBeenCalled()
+  })
+
+  it('keeps a replacement document and another requester independent of late old work', async () => {
+    const images = Promise.withResolvers<void>()
+    const otherImages = Promise.withResolvers<void>()
+    fake.images.mockReturnValueOnce(images.promise).mockReturnValueOnce(otherImages.promise)
+    const sender = new EventEmitter()
+    const oldResult = exportFrom(sender)
+    await flush()
+    const other = new EventEmitter()
+    const otherResult = exportFrom(other)
+    await flush()
+    sender.emit('did-navigate')
+    await flush()
+    expect(fake.windows.map((window) => window.destroyed)).toEqual([true, false])
+    expect(fake.tempFiles.size).toBe(1)
+    expect(other.listenerCount('render-process-gone')).toBe(1)
+    await expect(oldResult).resolves.toEqual({ success: false, cancelled: true })
+    fake.dialog.mockResolvedValueOnce({ canceled: false, filePath: '/chosen/new.pdf' })
+    fake.dialog.mockResolvedValueOnce({ canceled: false, filePath: '/chosen/other.pdf' })
+    await expect(exportFrom(sender)).resolves.toEqual({
+      success: true,
+      filePath: '/chosen/new.pdf'
+    })
+    otherImages.resolve()
+    await expect(otherResult).resolves.toEqual({
+      success: true,
+      filePath: '/chosen/other.pdf'
+    })
+    images.resolve()
+    await flush()
+    expect(fake.print).toHaveBeenCalledTimes(2)
+    expect(fake.dialog).toHaveBeenCalledTimes(2)
+    expect(fake.savedFiles).toEqual(
+      new Map([
+        ['/chosen/new.pdf', PDF],
+        ['/chosen/other.pdf', PDF]
+      ])
+    )
+    expectReleased(sender)
+    expectReleased(other)
+  })
+
+  it('preserves captured content through tab/file switches and same-document navigation', async () => {
+    const images = Promise.withResolvers<void>()
+    fake.images.mockReturnValueOnce(images.promise)
+    fake.dialog.mockResolvedValueOnce({ canceled: false, filePath: '/chosen/snapshot.pdf' })
+    const sender = new EventEmitter()
+    const result = exportFrom(sender)
+    await flush()
+    sender.emit('did-navigate-in-page')
+    // Tab/file selection is renderer-local; the captured HTML request remains owned by this document.
+    images.resolve()
+    await expect(result).resolves.toEqual({ success: true, filePath: '/chosen/snapshot.pdf' })
+    expect(fake.savedFiles.get('/chosen/snapshot.pdf')).toBe(PDF)
+    expectReleased(sender)
+  })
+
+  it('does not write a save choice returned after the requesting document retired', async () => {
+    const choice = Promise.withResolvers<{ canceled: boolean; filePath: string }>()
+    fake.dialog.mockReturnValueOnce(choice.promise)
+    const sender = new EventEmitter()
+    const result = exportFrom(sender)
+    await flush()
+    expect(fake.dialog).toHaveBeenCalledOnce()
+    sender.emit('render-process-gone')
+    choice.resolve({ canceled: false, filePath: '/chosen/late.pdf' })
+    await expect(result).resolves.toEqual({ success: false, cancelled: true })
+    expect(fake.savedFiles.size).toBe(0)
+    expectReleased(sender)
+  })
+
+  it('finishes a user-accepted write that started before retirement', async () => {
+    const writing = Promise.withResolvers<void>()
+    fake.dialog.mockResolvedValueOnce({ canceled: false, filePath: '/chosen/accepted.pdf' })
+    fake.write.mockImplementation(async (path, data) => {
+      if (typeof data === 'string') {
+        fake.tempFiles.add(path)
+        return
+      }
+      await writing.promise
+      fake.savedFiles.set(path, data)
+    })
+    const sender = new EventEmitter()
+    const result = exportFrom(sender)
+    await flush()
+    expect(fake.write).toHaveBeenCalledWith('/chosen/accepted.pdf', PDF)
+    sender.emit('destroyed')
+    writing.resolve()
+    await expect(result).resolves.toEqual({ success: true, filePath: '/chosen/accepted.pdf' })
+    expect(fake.savedFiles.get('/chosen/accepted.pdf')).toBe(PDF)
+    expectReleased(sender)
+  })
+
+  it('releases listeners and resources across repeated successful and retired exports', async () => {
+    const sender = new EventEmitter()
+    for (let index = 0; index < 10; index++) {
+      const retired = index % 2 === 0
+      const filePath = `/chosen/${index}.pdf`
+      const images = Promise.withResolvers<void>()
+      fake.images.mockReturnValueOnce(images.promise)
+      if (!retired) {
+        fake.dialog.mockResolvedValueOnce({ canceled: false, filePath })
+      }
+      const prints = fake.print.mock.calls.length
+      const dialogs = fake.dialog.mock.calls.length
+      const result = exportFrom(sender)
+      await flush()
+      if (retired) {
+        sender.emit('render-process-gone')
+      } else {
+        images.resolve()
+      }
+      await flush()
+      expectReleased(sender)
+      if (retired) {
+        await expect(result).resolves.toEqual({ success: false, cancelled: true })
+        expect(fake.print.mock.calls.length).toBe(prints)
+        expect(fake.dialog.mock.calls.length).toBe(dialogs)
+        expect(fake.savedFiles.has(filePath)).toBe(false)
+      } else {
+        await expect(result).resolves.toEqual({ success: true, filePath })
+        expect(fake.print.mock.calls.length).toBe(prints + 1)
+        expect(fake.savedFiles.get(filePath)).toEqual(PDF)
+      }
+      images.reject(new Error('late image failure'))
+      await flush()
+      expectReleased(sender)
+    }
+  })
+})

src/main/ipc/export.ts

@@ -1,6 +1,7 @@
 import { BrowserWindow, dialog, ipcMain } from 'electron'
 import { writeFile } from 'node:fs/promises'
 import { ExportTimeoutError, htmlToPdf } from '../lib/html-to-pdf'
+import { abortWhenRendererGone } from './renderer-lifetime-abort'
 
 export type ExportHtmlToPdfArgs = {
   html: string
@@ -21,8 +22,10 @@ export function registerExportHandlers(): void {
         return { success: false, error: 'No content to export' }
       }
 
+      const lifetime = abortWhenRendererGone(event.sender)
       try {
-        const pdfBuffer = await htmlToPdf(html)
+        const pdfBuffer = await htmlToPdf(html, lifetime.signal)
+        lifetime.signal.throwIfAborted()
 
         // Keep long titles and Windows-forbidden characters out of suggested save filenames.
         const sanitizedTitle = title.replace(/[/\\:*?"<>|]/g, '_').slice(0, 100) || 'export'
@@ -37,13 +40,18 @@ export function registerExportHandlers(): void {
           ? await dialog.showSaveDialog(parent, dialogOptions)
           : await dialog.showSaveDialog(dialogOptions)
 
+        lifetime.signal.throwIfAborted()
         if (canceled || !filePath) {
           return { success: false, cancelled: true }
         }
 
+        lifetime.dispose()
         await writeFile(filePath, pdfBuffer)
         return { success: true, filePath }
       } catch (error) {
+        if (lifetime.signal.aborted) {
+          return { success: false, cancelled: true }
+        }
         if (error instanceof ExportTimeoutError) {
           return { success: false, error: 'Export timed out' }
         }
@@ -51,6 +59,8 @@ export function registerExportHandlers(): void {
           success: false,
           error: error instanceof Error ? error.message : 'Failed to export PDF'
         }
+      } finally {
+        lifetime.dispose()
       }
     }
   )

src/main/lib/html-to-pdf.test.ts

@@ -25,6 +25,11 @@ vi.mock('electron', () => ({
       once: (event: string, listener: (...args: unknown[]) => void) => {
         fake.handlers.set(event, listener)
       },
+      removeListener: (event: string, listener: (...args: unknown[]) => void) => {
+        if (fake.handlers.get(event) === listener) {
+          fake.handlers.delete(event)
+        }
+      },
       executeJavaScript: fake.images,
       printToPDF: fake.print
     }
@@ -67,6 +72,56 @@ afterEach(() => {
 })
 
 describe('htmlToPdf resource ownership', () => {
+  it('does not create a document for an already-aborted request', async () => {
+    const controller = new AbortController()
+    const error = new Error('request retired')
+    controller.abort(error)
+
+    await expect(htmlToPdf(HTML, controller.signal)).rejects.toBe(error)
+
+    expect(fake.write).not.toHaveBeenCalled()
+    expect(fake.construct).not.toHaveBeenCalled()
+    expect(vi.getTimerCount()).toBe(0)
+  })
+
+  it.each(['success', 'failure', 'abort'] as const)(
+    'detaches conversion listeners after %s',
+    async (outcome) => {
+      const controller = new AbortController()
+      const added = vi.spyOn(controller.signal, 'addEventListener')
+      const removed = vi.spyOn(controller.signal, 'removeEventListener')
+      const images = Promise.withResolvers<void>()
+      fake.images.mockReturnValueOnce(images.promise)
+      const error = new Error('request retired or renderer failed')
+      const result = htmlToPdf(HTML, controller.signal)
+      const assertion =
+        outcome === 'success'
+          ? expect(result).resolves.toBe(PDF)
+          : expect(result).rejects.toBe(error)
+      await vi.advanceTimersByTimeAsync(0)
+      expect(added).toHaveBeenCalledWith('abort', expect.any(Function), { once: true })
+
+      if (outcome === 'abort') {
+        controller.abort(error)
+      } else if (outcome === 'failure') {
+        images.reject(error)
+      } else {
+        images.resolve()
+      }
+      await assertion
+
+      const listener = added.mock.calls.find(([event]) => event === 'abort')?.[1]
+      expect(listener).toBeTypeOf('function')
+      expect(removed).toHaveBeenCalledWith('abort', listener)
+      expect(fake.handlers.size).toBe(0)
+      expect(fake.files.size).toBe(0)
+      expect(fake.destroy).toHaveBeenCalledOnce()
+      expect(vi.getTimerCount()).toBe(0)
+      images.reject(new Error('late image failure'))
+      await Promise.resolve()
+    }
+  )
+
   it('removes the temporary document when window construction throws', async () => {
     const error = new Error('window construction failed')
     fake.construct.mockImplementation(() => {

src/main/lib/html-to-pdf.ts

@@ -27,13 +27,18 @@ new Promise((resolve) => {
 })
 `
 
-export async function htmlToPdf(html: string): Promise<Buffer> {
+export async function htmlToPdf(html: string, signal?: AbortSignal): Promise<Buffer> {
+  signal?.throwIfAborted()
   const tempDir = app.getPath('temp')
   const tempPath = path.join(tempDir, `orca-export-${randomUUID()}.html`)
 
   // Exclusive creation prevents writing through pre-seated symlinks; EEXIST paths belong to someone else.
   try {
-    await writeFile(tempPath, html, { encoding: 'utf-8', flag: 'wx' })
+    await writeFile(tempPath, html, {
+      encoding: 'utf-8',
+      flag: 'wx',
+      ...(signal ? { signal } : {})
+    })
   } catch (error) {
     if (isAlreadyExists(error)) {
       throw error
@@ -43,6 +48,7 @@ export async function htmlToPdf(html: string): Promise<Buffer> {
   }
 
   try {
+    signal?.throwIfAborted()
     const win = new BrowserWindow({
       show: false,
       webPreferences: {
@@ -55,24 +61,49 @@ export async function htmlToPdf(html: string): Promise<Buffer> {
     })
 
     let timer: NodeJS.Timeout | undefined
+    let removeAbortListener: (() => void) | undefined
+    let removeLoadListeners: (() => void) | undefined
 
     try {
+      const abortPromise = new Promise<never>((_resolve, reject) => {
+        if (!signal) {
+          return
+        }
+        const abort = (): void => reject(signal.reason)
+        signal.addEventListener('abort', abort, { once: true })
+        removeAbortListener = () => signal.removeEventListener('abort', abort)
+        if (signal.aborted) {
+          abort()
+        }
+      })
       // Bound loading too: a non-yielding document script never fires load completion or failure.
       const timeoutPromise = new Promise<never>((_resolve, reject) => {
         timer = setTimeout(() => reject(new ExportTimeoutError()), EXPORT_TIMEOUT_MS)
       })
 
       const loadAndPrint = (async (): Promise<Buffer> => {
+        signal?.throwIfAborted()
         const loadPromise = new Promise<void>((resolve, reject) => {
-          win.webContents.once('did-finish-load', () => resolve())
-          win.webContents.once('did-fail-load', (_event, errorCode, errorDescription) => {
+          const finish = (): void => resolve()
+          const fail = (
+            _event: Electron.Event,
+            errorCode: number,
+            errorDescription: string
+          ): void => {
             reject(new Error(`Failed to load export document: ${errorDescription} (${errorCode})`))
-          })
+          }
+          win.webContents.once('did-finish-load', finish)
+          win.webContents.once('did-fail-load', fail)
+          removeLoadListeners = () => {
+            win.webContents.removeListener('did-finish-load', finish)
+            win.webContents.removeListener('did-fail-load', fail)
+          }
         })
 
-        await win.loadFile(tempPath)
-        await loadPromise
+        await Promise.all([win.loadFile(tempPath), loadPromise])
+        signal?.throwIfAborted()
         await win.webContents.executeJavaScript(WAIT_FOR_IMAGES_SCRIPT, true)
+        signal?.throwIfAborted()
         return win.webContents.printToPDF({
           printBackground: true,
           pageSize: 'A4',
@@ -85,8 +116,10 @@ export async function htmlToPdf(html: string): Promise<Buffer> {
         })
       })()
 
-      return await Promise.race([loadAndPrint, timeoutPromise])
+      return await Promise.race([loadAndPrint, timeoutPromise, abortPromise])
     } finally {
+      removeAbortListener?.()
+      removeLoadListeners?.()
       if (timer) {
         clearTimeout(timer)
       }