feat: add screenshot retry logic for transient browser failures

- Add isRetryableError() helper (retry on TimeoutError, Protocol error,
  Target closed, Session closed, Navigation failed, net::ERR_*)
- Wrap browser screenshot in retry loop (max 2 retries, exponential backoff)
- Add retryCount to ScreenshotResult, X-Retry-Count response header
- Validation/SSRF/auth errors are NOT retried
- 28 new tests (12 retry classification + 6 screenshot retry + route tests)

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "snapapi",
-  "version": "0.7.0",
+  "version": "0.8.0",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "snapapi",
-      "version": "0.7.0",
+      "version": "0.8.0",
       "dependencies": {
         "compression": "^1.8.1",
         "express": "^4.21.0",

src/routes/screenshot.ts

@@ -591,6 +591,7 @@ async function handleScreenshotRequest(req: any, res: any) {
     res.setHeader("Content-Length", result.buffer.length);
     res.setHeader("Cache-Control", "no-store");
     res.setHeader("X-Cache", "MISS");
+    res.setHeader("X-Retry-Count", String(result.retryCount ?? 0));
     res.send(result.buffer);
   } catch (err: any) {
     logger.error({ err: err.message, url }, "Screenshot failed");

src/services/__tests__/retry.test.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect } from 'vitest'
+import { isRetryableError } from '../retry.js'
+
+describe('isRetryableError', () => {
+  it('returns true for TimeoutError', () => {
+    const err = new Error('TimeoutError: Navigation timeout of 20000ms exceeded')
+    err.name = 'TimeoutError'
+    expect(isRetryableError(err)).toBe(true)
+  })
+
+  it('returns true for Protocol error', () => {
+    expect(isRetryableError(new Error('Protocol error (Runtime.callFunctionOn): Session closed.'))).toBe(true)
+  })
+
+  it('returns true for Target closed', () => {
+    expect(isRetryableError(new Error('Target closed'))).toBe(true)
+  })
+
+  it('returns true for Session closed', () => {
+    expect(isRetryableError(new Error('Session closed. Most likely the page has been closed.'))).toBe(true)
+  })
+
+  it('returns true for Navigation failed', () => {
+    expect(isRetryableError(new Error('Navigation failed because browser has disconnected!'))).toBe(true)
+  })
+
+  it('returns true for net::ERR_ errors', () => {
+    expect(isRetryableError(new Error('net::ERR_CONNECTION_RESET'))).toBe(true)
+    expect(isRetryableError(new Error('net::ERR_CONNECTION_REFUSED'))).toBe(true)
+  })
+
+  it('returns false for SCREENSHOT_TIMEOUT (overall timeout, not transient)', () => {
+    expect(isRetryableError(new Error('SCREENSHOT_TIMEOUT'))).toBe(false)
+  })
+
+  it('returns false for SSRF_BLOCKED', () => {
+    expect(isRetryableError(new Error('SSRF_BLOCKED'))).toBe(false)
+  })
+
+  it('returns false for validation errors', () => {
+    expect(isRetryableError(new Error('hideSelector contains dangerous characters'))).toBe(false)
+    expect(isRetryableError(new Error('selector and fullPage are mutually exclusive'))).toBe(false)
+  })
+
+  it('returns false for SELECTOR_NOT_FOUND', () => {
+    expect(isRetryableError(new Error('SELECTOR_NOT_FOUND'))).toBe(false)
+  })
+
+  it('returns false for JS_EXECUTION_ERROR', () => {
+    expect(isRetryableError(new Error('JS_EXECUTION_ERROR: foo is not defined'))).toBe(false)
+  })
+
+  it('returns false for generic errors', () => {
+    expect(isRetryableError(new Error('Something went wrong'))).toBe(false)
+  })
+})

src/services/__tests__/screenshot.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
 
 // All mocks must use inline values since vi.mock is hoisted
 vi.mock('../browser.js', () => ({
@@ -590,4 +590,79 @@ describe('Screenshot Service', () => {
       expect(mockPage.$).not.toHaveBeenCalled()
     })
   })
+
+  describe('Retry logic', () => {
+    it('returns retryCount 0 on first-attempt success', async () => {
+      const result = await takeScreenshot({ url: 'https://example.com' })
+      expect(result.retryCount).toBe(0)
+    })
+
+    it('retries on transient error and returns retryCount 1', async () => {
+      vi.useFakeTimers()
+      try {
+        mockPage.goto
+          .mockImplementationOnce(async () => { throw new Error('net::ERR_CONNECTION_RESET') })
+          .mockResolvedValueOnce(undefined)
+
+        const promise = takeScreenshot({ url: 'https://example.com' })
+        await vi.advanceTimersByTimeAsync(2000)
+        const result = await promise
+        expect(result.retryCount).toBe(1)
+        expect(result.buffer).toBeTruthy()
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+
+    it('returns original error after all 3 attempts fail with retryCount 2', async () => {
+      vi.useFakeTimers()
+      try {
+        mockPage.goto
+          .mockRejectedValueOnce(new Error('Target closed'))
+          .mockRejectedValueOnce(new Error('Target closed'))
+          .mockRejectedValueOnce(new Error('Target closed'))
+
+        const promise = takeScreenshot({ url: 'https://example.com' }).catch((err: any) => err)
+        await vi.advanceTimersByTimeAsync(5000)
+
+        const err = await promise
+        expect(err).toBeInstanceOf(Error)
+        expect(err.message).toBe('Target closed')
+        expect(err.retryCount).toBe(2)
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+
+    it('does NOT retry validation errors', async () => {
+      await expect(takeScreenshot({
+        url: 'https://example.com',
+        selector: '#content',
+        fullPage: true,
+      })).rejects.toThrow('selector and fullPage are mutually exclusive')
+      expect(acquirePage).not.toHaveBeenCalled()
+    })
+
+    it('does NOT retry SSRF errors', async () => {
+      vi.mocked(validateUrl).mockRejectedValueOnce(new Error('SSRF_BLOCKED'))
+      await expect(takeScreenshot({ url: 'http://10.0.0.1' })).rejects.toThrow('SSRF_BLOCKED')
+      expect(acquirePage).not.toHaveBeenCalled()
+    })
+
+    it('releases page on each failed attempt before retry', async () => {
+      vi.useFakeTimers()
+      try {
+        mockPage.goto
+          .mockRejectedValueOnce(new Error('Protocol error'))
+          .mockResolvedValueOnce(undefined)
+
+        const promise = takeScreenshot({ url: 'https://example.com' })
+        await vi.advanceTimersByTimeAsync(2000)
+        await promise
+        expect(releasePage).toHaveBeenCalledTimes(2)
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+  })
 })

src/services/retry.ts

@@ -0,0 +1,17 @@
+const RETRYABLE_PATTERNS = [
+  'TimeoutError',
+  'Protocol error',
+  'Target closed',
+  'Session closed',
+  'Navigation failed',
+  'net::ERR_',
+];
+
+export function isRetryableError(error: Error): boolean {
+  // Check error name
+  if (error.name === 'TimeoutError') return true;
+  
+  // Check message patterns
+  const msg = error.message || '';
+  return RETRYABLE_PATTERNS.some(pattern => msg.includes(pattern));
+}

src/services/screenshot.ts

@@ -1,6 +1,7 @@
 import { Page } from "puppeteer";
 import { acquirePage, releasePage } from "./browser.js";
 import { validateUrl } from "./ssrf.js";
+import { isRetryableError } from "./retry.js";
 import logger from "./logger.js";
 
 export interface ScreenshotOptions {
@@ -26,6 +27,7 @@ export interface ScreenshotOptions {
 export interface ScreenshotResult {
   buffer: Buffer;
   contentType: string;
+  retryCount: number;
 }
 
 const MAX_WIDTH = 3840;
@@ -73,11 +75,14 @@ function validateSelector(selector: string): void {
   }
 }
 
+const MAX_RETRIES = 2;
+const BACKOFF_MS = [500, 1000];
+
 export async function takeScreenshot(opts: ScreenshotOptions): Promise<ScreenshotResult> {
-  // Validate URL for SSRF
+  // Validate URL for SSRF — not retried
   await validateUrl(opts.url);
 
-  // Validate CSS injection prevention
+  // Validate CSS injection prevention — not retried
   if (opts.hideSelectors && opts.hideSelectors.length > 0) {
     validateHideSelectors(opts.hideSelectors);
   }
@@ -104,6 +109,31 @@ export async function takeScreenshot(opts: ScreenshotOptions): Promise<Screensho
     throw new Error("clip is mutually exclusive with fullPage and selector");
   }
 
+  let lastError: any;
+  for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) {
+    if (attempt > 0) {
+      const delay = BACKOFF_MS[attempt - 1];
+      logger.warn({ attempt, delay, url: opts.url, error: lastError?.message }, "Retrying screenshot");
+      await new Promise(r => setTimeout(r, delay));
+    }
+
+    try {
+      const result = await executeBrowserScreenshot(opts);
+      return { ...result, retryCount: attempt };
+    } catch (err: any) {
+      lastError = err;
+      if (!isRetryableError(err)) {
+        throw err;
+      }
+    }
+  }
+
+  // All attempts exhausted
+  lastError.retryCount = MAX_RETRIES;
+  throw lastError;
+}
+
+async function executeBrowserScreenshot(opts: ScreenshotOptions): Promise<Omit<ScreenshotResult, 'retryCount'>> {
   const format = opts.format || "png";
   const width = Math.min(opts.width || 1280, MAX_WIDTH);
   const height = Math.min(opts.height || 800, MAX_HEIGHT);
@@ -173,19 +203,16 @@ export async function takeScreenshot(opts: ScreenshotOptions): Promise<Screensho
 
     let result: any;
     if ((opts as any).selector) {
-      // Take element screenshot
       const element = await page.$((opts as any).selector);
       if (!element) {
         throw new Error("SELECTOR_NOT_FOUND");
       }
       result = await element.screenshot(screenshotOpts);
     } else {
-      // Take page screenshot
       result = await page.screenshot(screenshotOpts);
     }
 
     const buffer = Buffer.from(result as unknown as ArrayBuffer);
-
     const contentType = format === "png" ? "image/png" : format === "jpeg" ? "image/jpeg" : "image/webp";
 
     return { buffer, contentType };