Make reflow scans fail gracefully

.github/actions/find/tests/findForUrl.test.ts

@@ -186,6 +186,20 @@ describe('findForUrl', () => {
       expect(playwrightMocks.contextClose).toHaveBeenCalledTimes(1)
       expect(playwrightMocks.browserClose).toHaveBeenCalledTimes(1)
     })
+
+    it('preserves caller logging and partial-result behavior when reflow fails', async () => {
+      const error = new Error('overflow evaluation timed out')
+      loadedPlugins = [{name: 'reflow-scan', default: vi.fn().mockRejectedValue(error)}]
+      actionInput = JSON.stringify(['reflow-scan', 'axe'])
+      clearAll()
+
+      await expect(findForUrl({url: 'test.com'})).resolves.toEqual([])
+
+      expect(core.error).toHaveBeenCalledWith(`Error during accessibility scan: ${error}`)
+      expect(AxeBuilder.prototype.analyze).not.toHaveBeenCalled()
+      expect(playwrightMocks.contextClose).toHaveBeenCalledTimes(1)
+      expect(playwrightMocks.browserClose).toHaveBeenCalledTimes(1)
+    })
   })
 
   describe('when no scans list is provided', () => {

.github/actions/find/tests/reflowScan.test.ts

@@ -0,0 +1,156 @@
+import {afterEach, describe, expect, it, vi} from 'vitest'
+import reflowScan, {FINDING_TIMEOUT_MS, OPERATION_TIMEOUT_MS} from '../../../scanner-plugins/reflow-scan/index.js'
+
+const originalViewport = {width: 1280, height: 720}
+
+function createPage({scrollWidth = 320, clientWidth = 320} = {}) {
+  return {
+    viewportSize: vi.fn<() => typeof originalViewport | null>(() => originalViewport),
+    url: vi.fn(() => 'https://example.com'),
+    setViewportSize: vi.fn().mockResolvedValue(undefined),
+    evaluate: vi.fn().mockResolvedValue({scrollWidth, clientWidth}),
+  }
+}
+
+afterEach(() => {
+  vi.useRealTimers()
+})
+
+describe('reflow scan', () => {
+  it('restores the original viewport when no overflow is found', async () => {
+    const page = createPage()
+    const addFinding = vi.fn()
+
+    await reflowScan({page, addFinding})
+
+    expect(page.setViewportSize).toHaveBeenNthCalledWith(1, {width: 320, height: 256})
+    expect(page.setViewportSize).toHaveBeenNthCalledWith(2, originalViewport)
+    expect(addFinding).not.toHaveBeenCalled()
+  })
+
+  it('preserves the overflow finding and restores the original viewport', async () => {
+    const page = createPage({scrollWidth: 400, clientWidth: 320})
+    const addFinding = vi.fn().mockResolvedValue(undefined)
+
+    await reflowScan({page, addFinding})
+
+    expect(addFinding).toHaveBeenCalledWith(
+      expect.objectContaining({
+        scannerType: 'reflow-scan',
+        url: 'https://example.com',
+        problemShort: 'needs review: page presents a horizontal scrollbar at a 320px wide viewport',
+        solutionLong: expect.stringContaining('scroll width of 400px but a client width of only 320px'),
+      }),
+    )
+    expect(page.setViewportSize).toHaveBeenLastCalledWith(originalViewport)
+  })
+
+  it.each([
+    [
+      'viewport resize',
+      (page: ReturnType<typeof createPage>) => page.setViewportSize.mockRejectedValueOnce(new Error('resize')),
+    ],
+    [
+      'overflow evaluation',
+      (page: ReturnType<typeof createPage>) => page.evaluate.mockRejectedValueOnce(new Error('evaluate')),
+    ],
+    [
+      'finding callback',
+      (page: ReturnType<typeof createPage>) =>
+        page.evaluate.mockResolvedValueOnce({scrollWidth: 400, clientWidth: 320}),
+    ],
+  ])('reports a rejected %s and attempts restoration', async (operation, configurePage) => {
+    const page = createPage()
+    configurePage(page)
+    const addFinding = vi.fn().mockRejectedValue(new Error('finding'))
+
+    await expect(reflowScan({page, addFinding})).rejects.toMatchObject({
+      name: 'ReflowScanError',
+      operation,
+    })
+
+    expect(page.setViewportSize).toHaveBeenCalledTimes(2)
+  })
+
+  it('reports restoration failure after a successful check', async () => {
+    const page = createPage()
+    page.setViewportSize.mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error('restore'))
+
+    await expect(reflowScan({page, addFinding: vi.fn()})).rejects.toMatchObject({
+      name: 'ReflowScanError',
+      operation: 'viewport restoration',
+      pageStateUnsafe: true,
+    })
+  })
+
+  it('preserves the scan error when viewport restoration also fails', async () => {
+    const page = createPage()
+    const scanError = new Error('resize')
+    const restoreError = new Error('restore')
+    page.setViewportSize.mockRejectedValueOnce(scanError).mockRejectedValueOnce(restoreError)
+
+    await expect(reflowScan({page, addFinding: vi.fn()})).rejects.toMatchObject({
+      name: 'ReflowScanError',
+      operation: 'viewport resize',
+      cause: expect.objectContaining({cause: scanError}),
+      cleanupError: expect.objectContaining({cause: restoreError}),
+      pageStateUnsafe: true,
+    })
+  })
+
+  it.each(['viewport resize', 'overflow evaluation', 'finding callback', 'viewport restoration'])(
+    'bounds a non-returning %s operation',
+    async operation => {
+      vi.useFakeTimers()
+      const page = createPage({scrollWidth: 400, clientWidth: 320})
+      const never = () => new Promise<never>(() => {})
+
+      if (operation === 'viewport resize') {
+        page.setViewportSize.mockImplementationOnce(never)
+      } else if (operation === 'overflow evaluation') {
+        page.evaluate.mockImplementationOnce(never)
+      } else if (operation === 'finding callback') {
+        page.evaluate.mockResolvedValueOnce({scrollWidth: 400, clientWidth: 320})
+      } else {
+        page.setViewportSize.mockResolvedValueOnce(undefined).mockImplementationOnce(never)
+      }
+
+      const addFinding = operation === 'finding callback' ? vi.fn(never) : vi.fn().mockResolvedValue(undefined)
+      const scan = reflowScan({page, addFinding})
+      const rejection = expect(scan).rejects.toMatchObject({
+        name: 'ReflowScanError',
+        operation,
+        pageStateUnsafe: true,
+      })
+
+      await vi.advanceTimersByTimeAsync(operation === 'finding callback' ? FINDING_TIMEOUT_MS : OPERATION_TIMEOUT_MS)
+
+      await rejection
+    },
+  )
+
+  it('allows a healthy finding callback to take longer than the viewport-operation budget', async () => {
+    vi.useFakeTimers()
+    const page = createPage({scrollWidth: 400})
+    const addFinding = vi.fn(() => new Promise<void>(resolve => setTimeout(resolve, 31000)))
+    const scan = reflowScan({page, addFinding})
+
+    await vi.advanceTimersByTimeAsync(31000)
+    await expect(scan).resolves.toBeUndefined()
+
+    expect(page.setViewportSize).toHaveBeenLastCalledWith(originalViewport)
+    expect(vi.getTimerCount()).toBe(0)
+  })
+
+  it('does not change the page when its original viewport is unavailable', async () => {
+    const page = createPage()
+    page.viewportSize.mockReturnValue(null)
+
+    await expect(reflowScan({page, addFinding: vi.fn()})).rejects.toMatchObject({
+      name: 'ReflowScanError',
+      operation: 'viewport capture',
+    })
+
+    expect(page.setViewportSize).not.toHaveBeenCalled()
+  })
+})

.github/scanner-plugins/reflow-scan/index.ts

@@ -1,32 +1,155 @@
-export default async function reflowScan({page, addFinding} = {}) {
-  const originalViewport = page.viewportSize()
+import type {PluginDefaultParams} from '../../actions/find/src/pluginManager/types.js'
+
+// Cooperative wait budgets only: timers must run, and browser teardown is not included.
+export const OPERATION_TIMEOUT_MS = 5000
+export const FINDING_TIMEOUT_MS = 35000
+
+class ReflowOperationError extends Error {
+  constructor(
+    readonly operation: string,
+    message: string,
+    readonly timedOut: boolean,
+    options?: ErrorOptions,
+  ) {
+    super(`${operation} ${message}`, options)
+    this.name = 'ReflowOperationError'
+  }
+}
+
+export class ReflowScanError extends Error {
+  constructor(
+    readonly operation: string,
+    cause: ReflowOperationError,
+    readonly cleanupError: ReflowOperationError | undefined,
+    readonly pageStateUnsafe: boolean,
+  ) {
+    const cleanupMessage = cleanupError ? `; viewport restoration also failed: ${cleanupError.message}` : ''
+    const unsafeMessage = pageStateUnsafe ? '; page state may be unsafe and will not be reused' : ''
+    super(`${cause.message}${cleanupMessage}${unsafeMessage}`, {cause})
+    this.name = 'ReflowScanError'
+  }
+}
+
+export default async function reflowScan({
+  page,
+  addFinding,
+}: {
+  page: Pick<PluginDefaultParams['page'], 'viewportSize' | 'url' | 'setViewportSize' | 'evaluate'>
+  addFinding: PluginDefaultParams['addFinding']
+}) {
+  let originalViewport
+  try {
+    originalViewport = page.viewportSize()
+  } catch (error) {
+    throw new ReflowScanError(
+      'viewport capture',
+      new ReflowOperationError('viewport capture', `failed: ${errorMessage(error)}`, false, {cause: error}),
+      undefined,
+      false,
+    )
+  }
+
+  if (!originalViewport) {
+    throw new ReflowScanError(
+      'viewport capture',
+      new ReflowOperationError('viewport capture', 'is unavailable; cannot safely change the viewport', false),
+      undefined,
+      false,
+    )
+  }
+
   const url = page.url()
-  // Check for horizontal scrolling at 320 viewport
+  let resizeAttempted = false
+  let operationError: ReflowOperationError | undefined
+  let cleanupError: ReflowOperationError | undefined
+
   try {
-    await page.setViewportSize({width: 320, height: 256})
-    const scrollWidth = await page.evaluate(() => document.documentElement.scrollWidth)
-    const clientWidth = await page.evaluate(() => document.documentElement.clientWidth)
+    resizeAttempted = true
+    await runBounded('viewport resize', () => page.setViewportSize({width: 320, height: 256}))
+
+    const {scrollWidth, clientWidth} = await runBounded('overflow evaluation', () =>
+      page.evaluate(() => ({
+        scrollWidth: document.documentElement.scrollWidth,
+        clientWidth: document.documentElement.clientWidth,
+      })),
+    )
 
-    // If horizontal scroll is required (with 1px tolerance for rounding)
     if (scrollWidth > clientWidth + 1) {
-      await addFinding({
-        scannerType: 'reflow-scan',
-        url,
-        problemShort: 'needs review: page presents a horizontal scrollbar at a 320px wide viewport',
-        problemUrl: 'https://www.w3.org/WAI/WCAG22/Understanding/reflow.html',
-        solutionShort:
-          'verify if sections of content can be viewed within the 320px wide viewport without needing to scroll in two dimensions to read the content of an individual section',
-        solutionLong: `The page has a scroll width of ${scrollWidth}px but a client width of only ${clientWidth}px at a 320px wide viewport, resulting in a horizontal scrollbar. Ensure that multi-line text does not require scrolling in two-dimensions to read, as this would be a violation of WCAG Success Criterion 1.4.10 (Reflow).`,
-      })
+      await runBounded(
+        'finding callback',
+        () =>
+          addFinding({
+            scannerType: 'reflow-scan',
+            url,
+            problemShort: 'needs review: page presents a horizontal scrollbar at a 320px wide viewport',
+            problemUrl: 'https://www.w3.org/WAI/WCAG22/Understanding/reflow.html',
+            solutionShort:
+              'verify if sections of content can be viewed within the 320px wide viewport without needing to scroll in two dimensions to read the content of an individual section',
+            solutionLong: `The page has a scroll width of ${scrollWidth}px but a client width of only ${clientWidth}px at a 320px wide viewport, resulting in a horizontal scrollbar. Ensure that multi-line text does not require scrolling in two-dimensions to read, as this would be a violation of WCAG Success Criterion 1.4.10 (Reflow).`,
+          }),
+        FINDING_TIMEOUT_MS,
+      )
     }
-  } catch (e) {
-    console.error('Error checking horizontal scroll:', e)
+  } catch (error) {
+    operationError =
+      error instanceof ReflowOperationError
+        ? error
+        : new ReflowOperationError('reflow scan', `failed: ${errorMessage(error)}`, false, {cause: error})
   } finally {
-    // Restore original viewport so subsequent scans (e.g. Axe) aren't affected
-    if (originalViewport) {
-      await page.setViewportSize(originalViewport)
+    if (resizeAttempted) {
+      try {
+        await runBounded('viewport restoration', () => page.setViewportSize(originalViewport))
+      } catch (error) {
+        cleanupError =
+          error instanceof ReflowOperationError
+            ? error
+            : new ReflowOperationError('viewport restoration', `failed: ${errorMessage(error)}`, false, {
+                cause: error,
+              })
+      }
     }
   }
+
+  const failure = operationError ?? cleanupError
+  if (failure) {
+    throw new ReflowScanError(
+      failure.operation,
+      failure,
+      operationError ? cleanupError : undefined,
+      failure.timedOut || cleanupError !== undefined,
+    )
+  }
+}
+
+async function runBounded<T>(
+  operation: string,
+  action: () => Promise<T>,
+  timeoutMs = OPERATION_TIMEOUT_MS,
+): Promise<T> {
+  let timer: ReturnType<typeof setTimeout> | undefined
+  const timeout = new Promise<never>((_, reject) => {
+    timer = setTimeout(
+      () => reject(new ReflowOperationError(operation, `timed out after ${timeoutMs}ms`, true)),
+      timeoutMs,
+    )
+  })
+
+  try {
+    return await Promise.race([
+      Promise.resolve()
+        .then(action)
+        .catch(cause => {
+          throw new ReflowOperationError(operation, `failed: ${errorMessage(cause)}`, false, {cause})
+        }),
+      timeout,
+    ])
+  } finally {
+    if (timer) clearTimeout(timer)
+  }
+}
+
+function errorMessage(error: unknown): string {
+  return error instanceof Error ? error.message : String(error)
 }
 
 export const name = 'reflow-scan'

PLUGINS.md

@@ -5,6 +5,18 @@ The plugin system allows teams to create custom scans/tests to run on their page
 Some first-party plugins come built-in with the scanner repository and can be enabled via [actions inputs](https://github.com/github/accessibility-scanner/tree/main/action.yml#L48-L50).
 There are also some [allowlisted](https://github.com/github/accessibility-scanner/blob/7ec4b73ca2fdb471ad6f71a48a88e7b1a84ab3ad/.github/actions/find/src/pluginManager/index.ts#L92) first-party plugins hosted in a different repository; we also have the ability to support loading approved third-party plugins [from NPM](#loading-plugins-from-npm-packages).
 
+## Built-in reflow scan
+
+Enable `reflow-scan` in the `scans` input to check for horizontal overflow at a 320 x 256 viewport. It keeps the existing 1px rounding tolerance and restores the original viewport after checking. If the original viewport is unavailable, it fails without resizing the page.
+
+Viewport resize, overflow evaluation, and viewport restoration each have a **5-second cooperative wait budget**. The finding callback has a separate **35-second budget**, allowing the screenshot library's existing 30-second timeout plus headroom for saving and recording the finding. This avoids reducing normal screenshot capture time to five seconds, but a callback exceeding 35 seconds now fails the reflow check instead of waiting indefinitely.
+
+The total plugin wait budget is 50 seconds when all phases run (15 seconds without an overflow finding). These are not hard wall-clock guarantees: timers need a responsive JavaScript event loop, timed-out Playwright operations are not cancelled, and context/browser teardown is outside these budgets. A blocked controller or browser teardown still requires runtime-level recovery.
+
+Reflow propagates incomplete-check errors to its caller. Restoration failures retain the original scan error with cleanup context; timeouts and failed restoration mark the page state as unsafe even if a timed-out operation later settles. The current caller logs plugin errors, skips remaining scans on that page, attempts context/browser cleanup, and returns partial findings before continuing to later URLs.
+
+**Logging and continuing does not guarantee complete scan coverage or exclude partial findings from issue filing.** The separate configurable caller-policy change will offer opt-in abort behavior to stop incomplete plugin work before findings publication, while retaining log-and-continue as the compatibility default. This plugin change does not alter the scanner's existing error-handling policy.
+
 ## How to create plugins
 
 ### Default function API (all plugins)