From 1ca21889caeb77f12f28d8f9ad78f256ded00ad8 Mon Sep 17 00:00:00 2001 From: Paul Huliganga Date: Tue, 24 Mar 2026 23:54:23 -0400 Subject: [PATCH] Add URL import retry/timeout handling and friendly failure messaging --- TODO.md | 2 +- frontend/src/pages/ImportUrlPage.tsx | 14 ++++ src/backend/routes/import.ts | 24 ++++++- src/backend/services/UrlImportService.ts | 83 +++++++++++++++++++++--- src/backend/tests/import.test.ts | 49 ++++++++++++++ 5 files changed, 158 insertions(+), 14 deletions(-) diff --git a/TODO.md b/TODO.md index dff5195..0289560 100644 --- a/TODO.md +++ b/TODO.md @@ -38,7 +38,7 @@ MVP is functionally complete (core app + docs + tests). ### Phase 3: Fallback Parsing + Hardening - [x] Add heuristic fallback parser when Schema.org missing -- [ ] Add timeout/retry + user-friendly import failure messages +- [x] Add timeout/retry + user-friendly import failure messages - [ ] Add logging/telemetry for import success/failure reasons ### Phase 4: Browser Extension (after URL import stable) diff --git a/frontend/src/pages/ImportUrlPage.tsx b/frontend/src/pages/ImportUrlPage.tsx index 2fcc379..137665e 100644 --- a/frontend/src/pages/ImportUrlPage.tsx +++ b/frontend/src/pages/ImportUrlPage.tsx @@ -34,6 +34,20 @@ function getImportErrorDetails(message: string): { type: ImportErrorType; messag }; } + if (normalized.includes('network error') || normalized.includes('could not fetch the page')) { + return { + type: 'generic', + message: 'We could not reach that recipe page right now. Please try again in a moment.', + }; + } + + if (normalized.includes('did not return an html page')) { + return { + type: 'generic', + message: 'That link did not point to an HTML recipe page. Try the direct recipe URL.', + }; + } + return { type: 'generic', message, diff --git a/src/backend/routes/import.ts b/src/backend/routes/import.ts index c4190ce..98899e9 100644 --- a/src/backend/routes/import.ts +++ b/src/backend/routes/import.ts @@ -1,6 +1,6 @@ import { Router } from 'express'; import { z } from 'zod'; -import { UrlImportService } from '../services/UrlImportService.js'; +import { UrlImportError, UrlImportService } from '../services/UrlImportService.js'; import { SchemaOrgRecipeParserService } from '../services/SchemaOrgRecipeParserService.js'; import { HeuristicRecipeParserService } from '../services/HeuristicRecipeParserService.js'; @@ -8,6 +8,16 @@ const importUrlSchema = z.object({ url: z.string().url('A valid URL is required'), }); +function mapImportErrorToStatus(error: UrlImportError): number { + if (error.code === 'IMPORT_TIMEOUT') return 504; + if (error.code === 'IMPORT_NETWORK') return 502; + if (error.code === 'IMPORT_FETCH_FAILED') { + if (error.status !== undefined && error.status >= 500) return 502; + return 400; + } + return 400; +} + export function createImportRoutes(): Router { const router = Router(); const urlImportService = new UrlImportService(); @@ -50,9 +60,17 @@ export function createImportRoutes(): Router { return; } + if (error instanceof UrlImportError) { + res.status(mapImportErrorToStatus(error)).json({ + success: false, + data: null, + error: error.message, + }); + return; + } + if (error instanceof Error) { - const status = error.message.includes('timed out') ? 504 : 400; - res.status(status).json({ + res.status(500).json({ success: false, data: null, error: error.message, diff --git a/src/backend/services/UrlImportService.ts b/src/backend/services/UrlImportService.ts index e42f969..88b0410 100644 --- a/src/backend/services/UrlImportService.ts +++ b/src/backend/services/UrlImportService.ts @@ -4,14 +4,32 @@ export interface UrlImportFetchResult { json_ld_blocks: string[]; } +export type UrlImportErrorCode = + | 'IMPORT_TIMEOUT' + | 'IMPORT_NETWORK' + | 'IMPORT_FETCH_FAILED' + | 'IMPORT_UNSUPPORTED_CONTENT'; + +export class UrlImportError extends Error { + constructor( + public readonly code: UrlImportErrorCode, + message: string, + public readonly status?: number, + ) { + super(message); + this.name = 'UrlImportError'; + } +} + /** * Foundation service for importing recipe content from public URLs. */ export class UrlImportService { private static readonly DEFAULT_TIMEOUT_MS = 10000; + private static readonly MAX_RETRIES = 2; async fetchFromUrl(url: string): Promise { - const html = await this.fetchHtml(url); + const html = await this.fetchHtmlWithRetry(url); const jsonLdBlocks = this.extractJsonLdBlocks(html); return { @@ -21,7 +39,35 @@ export class UrlImportService { }; } - private async fetchHtml(url: string): Promise { + private async fetchHtmlWithRetry(url: string): Promise { + let lastError: UrlImportError | null = null; + + for (let attempt = 0; attempt <= UrlImportService.MAX_RETRIES; attempt += 1) { + try { + return await this.fetchHtmlOnce(url); + } catch (error) { + if (error instanceof UrlImportError) { + lastError = error; + + // Retry only on transient failures + if ( + (error.code === 'IMPORT_TIMEOUT' || error.code === 'IMPORT_NETWORK' || (error.status !== undefined && error.status >= 500)) && + attempt < UrlImportService.MAX_RETRIES + ) { + continue; + } + + throw error; + } + + throw error; + } + } + + throw lastError ?? new UrlImportError('IMPORT_FETCH_FAILED', 'Unable to import this URL right now. Please try again.'); + } + + private async fetchHtmlOnce(url: string): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), UrlImportService.DEFAULT_TIMEOUT_MS); @@ -36,25 +82,42 @@ export class UrlImportService { }); if (!response.ok) { - throw new Error(`Failed to fetch URL: HTTP ${response.status}`); + throw new UrlImportError( + 'IMPORT_FETCH_FAILED', + `Could not fetch the page (HTTP ${response.status}).`, + response.status, + ); } const contentType = response.headers.get('content-type') ?? ''; if (!contentType.includes('text/html')) { - throw new Error('URL did not return an HTML document'); + throw new UrlImportError( + 'IMPORT_UNSUPPORTED_CONTENT', + 'The URL did not return an HTML page. Please use a direct recipe page URL.', + ); } return await response.text(); } catch (error) { - if (error instanceof Error && error.name === 'AbortError') { - throw new Error('Import request timed out while fetching URL'); - } - - if (error instanceof Error) { + if (error instanceof UrlImportError) { throw error; } - throw new Error('Unknown error while fetching URL'); + if (error instanceof Error && error.name === 'AbortError') { + throw new UrlImportError( + 'IMPORT_TIMEOUT', + 'Import timed out while contacting the recipe page. Please try again.', + ); + } + + if (error instanceof Error) { + throw new UrlImportError( + 'IMPORT_NETWORK', + 'Network error while fetching recipe URL. Please try again.', + ); + } + + throw new UrlImportError('IMPORT_FETCH_FAILED', 'Unknown error while fetching URL'); } finally { clearTimeout(timeout); } diff --git a/src/backend/tests/import.test.ts b/src/backend/tests/import.test.ts index 2d4c25a..8938be2 100644 --- a/src/backend/tests/import.test.ts +++ b/src/backend/tests/import.test.ts @@ -189,6 +189,55 @@ describe('Import API', () => { expect(response.body.data.draft_recipe).toBeNull(); }); + + it('should retry transient fetch failures and eventually succeed', async () => { + const html = '

Retry Recipe

Ingredients

  • 1 egg

Instructions

  1. Cook it.
'; + let callCount = 0; + + vi.spyOn(globalThis, 'fetch').mockImplementation(async () => { + callCount += 1; + if (callCount < 3) { + throw new Error('temporary network issue'); + } + + return { + ok: true, + status: 200, + headers: new Headers({ 'content-type': 'text/html; charset=utf-8' }), + text: async () => html, + } as Response; + }); + + const response = await request(app) + .post('/api/import/url') + .send({ url: 'https://example.com/retry-recipe' }) + .expect(200); + + expect(callCount).toBe(3); + expect(response.body.success).toBe(true); + expect(response.body.data.draft_recipe).toMatchObject({ + title: 'Retry Recipe', + ingredients: ['1 egg'], + instructions: ['Cook it.'], + }); + }); + + it('should return timeout-friendly message after retries are exhausted', async () => { + const timeoutError = new Error('aborted'); + timeoutError.name = 'AbortError'; + + vi.spyOn(globalThis, 'fetch').mockRejectedValue(timeoutError); + + const response = await request(app) + .post('/api/import/url') + .send({ url: 'https://example.com/slow-recipe' }) + .expect(504); + + expect(response.body.success).toBe(false); + expect(response.body.error).toContain('timed out'); + }); + + it('should return an error for non-HTML responses', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue({ ok: true,