Skip to content

Commit a05c774

Browse files
gildesmaraisgildesmarais
committed
Harden create-flow retry recovery with explicit retry actions
1 parent 755eef1 commit a05c774

4 files changed

Lines changed: 129 additions & 39 deletions

File tree

‎frontend/src/__tests__/App.test.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ describe('App', () => {
574574
mockConvertFeed
575575
.mockRejectedValueOnce(
576576
Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), {
577+
retryAction: 'alternate',
577578
manualRetryStrategy: 'browserless',
578579
})
579580
)
@@ -610,7 +611,7 @@ describe('App', () => {
610611
mockConvertFeed
611612
.mockRejectedValueOnce(
612613
Object.assign(new Error('Tried faraday first, then browserless. Browserless failed.'), {
613-
manualRetryStrategy: '',
614+
retryAction: 'primary',
614615
})
615616
)
616617
.mockResolvedValueOnce(mockCreatedFeedResult);
@@ -646,7 +647,7 @@ describe('App', () => {
646647
});
647648
mockConvertFeed.mockRejectedValueOnce(
648649
Object.assign(new Error('URL not allowed for this account'), {
649-
manualRetryStrategy: 'browserless',
650+
retryAction: undefined,
650651
})
651652
);
652653

‎frontend/src/__tests__/useFeedConversion.test.ts‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -171,16 +171,22 @@ describe('useFeedConversion', () => {
171171
fetchMock.mockRejectedValueOnce(new Error('Network error'));
172172

173173
const { result } = renderHook(() => useFeedConversion());
174+
let thrownError: (Error & { manualRetryStrategy?: string; retryAction?: string }) | undefined;
174175

175176
await act(async () => {
176-
await expect(result.current.convertFeed('https://example.com', 'faraday', 'testtoken')).rejects.toThrow(
177-
'Network error'
178-
);
177+
try {
178+
await result.current.convertFeed('https://example.com', 'faraday', 'testtoken');
179+
} catch (error) {
180+
thrownError = error as Error & { manualRetryStrategy?: string; retryAction?: string };
181+
}
179182
});
180183

181184
expect(result.current.isConverting).toBe(false);
182185
expect(result.current.result).toBeUndefined();
183186
expect(result.current.error).toBe('Network error');
187+
expect(thrownError?.message).toBe('Network error');
188+
expect(thrownError?.manualRetryStrategy).toBe('browserless');
189+
expect(thrownError?.retryAction).toBe('alternate');
184190
});
185191

186192
it('preserves the created feed when preview loading fails after feed creation', async () => {
@@ -627,16 +633,22 @@ describe('useFeedConversion', () => {
627633
);
628634

629635
const { result } = renderHook(() => useFeedConversion());
636+
let thrownError: (Error & { manualRetryStrategy?: string; retryAction?: string }) | undefined;
630637

631638
await act(async () => {
632-
await expect(
633-
result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken')
634-
).rejects.toThrow('Unauthorized');
639+
try {
640+
await result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken');
641+
} catch (error) {
642+
thrownError = error as Error & { manualRetryStrategy?: string; retryAction?: string };
643+
}
635644
});
636645

637646
expect(fetchMock).toHaveBeenCalledTimes(1);
638647
expect(result.current.result).toBeUndefined();
639648
expect(result.current.error).toBe('Unauthorized');
649+
expect(thrownError?.message).toBe('Unauthorized');
650+
expect(thrownError?.manualRetryStrategy).toBeUndefined();
651+
expect(thrownError?.retryAction).toBeUndefined();
640652
});
641653

642654
it('does not auto-retry when API returns a non-retryable BAD_REQUEST code', async () => {
@@ -654,16 +666,22 @@ describe('useFeedConversion', () => {
654666
);
655667

656668
const { result } = renderHook(() => useFeedConversion());
669+
let thrownError: (Error & { manualRetryStrategy?: string; retryAction?: string }) | undefined;
657670

658671
await act(async () => {
659-
await expect(
660-
result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken')
661-
).rejects.toThrow('Input rejected');
672+
try {
673+
await result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken');
674+
} catch (error) {
675+
thrownError = error as Error & { manualRetryStrategy?: string; retryAction?: string };
676+
}
662677
});
663678

664679
expect(fetchMock).toHaveBeenCalledTimes(1);
665680
expect(result.current.result).toBeUndefined();
666681
expect(result.current.error).toBe('Input rejected');
682+
expect(thrownError?.message).toBe('Input rejected');
683+
expect(thrownError?.manualRetryStrategy).toBeUndefined();
684+
expect(thrownError?.retryAction).toBeUndefined();
667685
});
668686

669687
it('still auto-retries when API returns INTERNAL_SERVER_ERROR even if message contains a url', async () => {
@@ -763,19 +781,20 @@ describe('useFeedConversion', () => {
763781

764782
const { result } = renderHook(() => useFeedConversion());
765783

766-
let thrownError: (Error & { manualRetryStrategy?: string }) | undefined;
784+
let thrownError: (Error & { manualRetryStrategy?: string; retryAction?: string }) | undefined;
767785
await act(async () => {
768786
try {
769787
await result.current.convertFeed('https://example.com/articles', 'faraday', 'testtoken');
770788
} catch (error) {
771-
thrownError = error as Error & { manualRetryStrategy?: string };
789+
thrownError = error as Error & { manualRetryStrategy?: string; retryAction?: string };
772790
}
773791
});
774792

775793
expect(thrownError?.message).toBe(
776794
'Tried faraday first, then browserless. First attempt failed with: Upstream timeout. Second attempt failed with: Browserless also failed'
777795
);
778796
expect(thrownError?.manualRetryStrategy).toBeUndefined();
797+
expect(thrownError?.retryAction).toBe('primary');
779798
expect(result.current.result).toBeUndefined();
780799
expect(result.current.error).toBe(
781800
'Tried faraday first, then browserless. First attempt failed with: Upstream timeout. Second attempt failed with: Browserless also failed'

‎frontend/src/components/App.tsx‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,26 +39,9 @@ function isAccessTokenError(message: string) {
3939
);
4040
}
4141

42-
function isActionableStrategySwitch(message: string, currentStrategy: string, retryStrategy: string) {
43-
if (currentStrategy !== 'faraday' || retryStrategy !== 'browserless') return false;
44-
45-
const normalized = message.toLowerCase();
46-
return !(
47-
normalized.includes('unauthorized') ||
48-
normalized.includes('forbidden') ||
49-
normalized.includes('not allowed') ||
50-
normalized.includes('disabled') ||
51-
normalized.includes('access token') ||
52-
normalized.includes('token') ||
53-
normalized.includes('authentication') ||
54-
normalized.includes('bad request') ||
55-
normalized.includes('url') ||
56-
normalized.includes('unsupported strategy')
57-
);
58-
}
59-
6042
interface ConversionErrorWithMeta extends Error {
6143
manualRetryStrategy?: string;
44+
retryAction?: 'alternate' | 'primary';
6245
}
6346

6447
function classifyWorkflowError(message?: string): WorkflowErrorKind | undefined {
@@ -206,6 +189,7 @@ export function App() {
206189
const [tokenDraft, setTokenDraft] = useState('');
207190
const [tokenError, setTokenError] = useState('');
208191
const [manualRetryStrategy, setManualRetryStrategy] = useState('');
192+
const [showPrimaryRetry, setShowPrimaryRetry] = useState(false);
209193
const [focusCreateComposerKey, setFocusCreateComposerKey] = useState(0);
210194
const [resultRouteRecoveryAttempted, setResultRouteRecoveryAttempted] = useState(false);
211195
const autoSubmitUrlReference = useRef<string | undefined>(route.prefillUrl);
@@ -271,6 +255,7 @@ export function App() {
271255
});
272256
setFeedFieldErrors((previous) => ({ ...previous, url: '', form: '' }));
273257
setManualRetryStrategy('');
258+
setShowPrimaryRetry(false);
274259
clearError();
275260
};
276261

@@ -315,14 +300,16 @@ export function App() {
315300
} catch (submitError) {
316301
const message = submitError instanceof Error ? submitError.message : 'Unable to start feed generation.';
317302
const retryStrategy = (submitError as ConversionErrorWithMeta).manualRetryStrategy ?? '';
318-
setManualRetryStrategy(
319-
isActionableStrategySwitch(message, strategy, retryStrategy) ? retryStrategy : ''
320-
);
303+
const retryAction = (submitError as ConversionErrorWithMeta).retryAction;
304+
setManualRetryStrategy(retryAction === 'alternate' ? retryStrategy : '');
305+
setShowPrimaryRetry(retryAction === 'primary');
321306

322307
if (feedCreation.access_token_required && isAccessTokenError(message)) {
323308
clearToken();
324309
clearError();
325310
setTokenDraft('');
311+
setManualRetryStrategy('');
312+
setShowPrimaryRetry(false);
326313
if (route.kind !== 'token') navigate({ kind: 'token', prefillUrl: normalizedUrl });
327314
setTokenError('Access token was rejected. Paste a valid token to continue.');
328315
setFeedFieldErrors(EMPTY_FEED_ERRORS);
@@ -341,12 +328,16 @@ export function App() {
341328
const handleFeedSubmit = async (event: Event) => {
342329
event.preventDefault();
343330
setFeedFieldErrors(EMPTY_FEED_ERRORS);
331+
setManualRetryStrategy('');
332+
setShowPrimaryRetry(false);
344333
await attemptFeedCreation(token ?? '');
345334
};
346335

347336
const handleSaveToken = async () => {
348337
try {
349338
const normalizedToken = tokenDraft.trim();
339+
setManualRetryStrategy('');
340+
setShowPrimaryRetry(false);
350341
await saveToken(normalizedToken);
351342
setTokenError('');
352343
const created = await attemptFeedCreation(normalizedToken);
@@ -359,13 +350,16 @@ export function App() {
359350
const handleCreateAnother = () => {
360351
clearResult();
361352
setManualRetryStrategy('');
353+
setShowPrimaryRetry(false);
362354
setFocusCreateComposerKey((current) => current + 1);
363355
navigate({ kind: 'create', prefillUrl: feedFormData.url || undefined });
364356
};
365357

366358
const handleRetryCreation = () => {
367359
setFeedFieldErrors(EMPTY_FEED_ERRORS);
368360
clearError();
361+
setManualRetryStrategy('');
362+
setShowPrimaryRetry(false);
369363
void attemptFeedCreation(token ?? '');
370364
};
371365

@@ -374,6 +368,7 @@ export function App() {
374368

375369
setFeedFieldErrors(EMPTY_FEED_ERRORS);
376370
clearError();
371+
setShowPrimaryRetry(false);
377372
void attemptFeedCreation(token ?? '', manualRetryStrategy);
378373
};
379374

@@ -496,9 +491,7 @@ export function App() {
496491
tokenDraft={tokenDraft}
497492
tokenError={tokenError}
498493
showTokenPrompt={isTokenRoute}
499-
showPrimaryRetry={Boolean(
500-
(conversionError || feedFieldErrors.form)?.startsWith('Tried ') && !manualRetryStrategy
501-
)}
494+
showPrimaryRetry={showPrimaryRetry}
502495
onFeedSubmit={handleFeedSubmit}
503496
onFeedFieldChange={setFeedField}
504497
onTokenDraftChange={(value) => {
@@ -510,6 +503,8 @@ export function App() {
510503
onCancelTokenPrompt={() => {
511504
setTokenError('');
512505
clearError();
506+
setManualRetryStrategy('');
507+
setShowPrimaryRetry(false);
513508
navigate({ kind: 'create', prefillUrl: feedFormData.url || undefined });
514509
}}
515510
manualRetryStrategy={manualRetryStrategy}

‎frontend/src/hooks/useFeedConversion.ts‎

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,14 @@ interface ConversionState {
2626

2727
interface ConversionError extends Error {
2828
manualRetryStrategy?: string;
29+
retryAction?: 'alternate' | 'primary';
30+
code?: string;
31+
status?: number;
32+
kind?: ConversionFailureKind;
2933
}
3034

35+
export type ConversionFailureKind = 'auth' | 'input' | 'network' | 'server';
36+
3137
const PREVIEW_UNAVAILABLE_MESSAGE = 'Preview unavailable right now.';
3238
const FEED_NOT_READY_MESSAGE = 'Feed is still preparing. Try again in a few seconds.';
3339
const NON_RETRYABLE_ERROR_CODES = new Set(['BAD_REQUEST', 'UNAUTHORIZED', 'FORBIDDEN']);
@@ -98,12 +104,24 @@ export function useFeedConversion() {
98104
requestedStrategy,
99105
fallbackStrategy
100106
);
101-
failConversion(setState, message, { manualRetryStrategy: undefined });
107+
failConversion(setState, message, {
108+
kind: classifyConversionFailure(firstError),
109+
retryAction: 'primary',
110+
...extractFailureMetadata(secondError),
111+
});
102112
}
103113
}
104114

105115
const message = toErrorMessage(firstError);
106-
failConversion(setState, message, { manualRetryStrategy: alternateStrategy(requestedStrategy) });
116+
const retryStrategy = shouldOfferManualRetry(requestedStrategy, fallbackStrategy, firstError)
117+
? alternateStrategy(requestedStrategy)
118+
: undefined;
119+
failConversion(setState, message, {
120+
...extractFailureMetadata(firstError),
121+
kind: classifyConversionFailure(firstError),
122+
manualRetryStrategy: retryStrategy,
123+
retryAction: retryStrategy ? 'alternate' : undefined,
124+
});
107125
}
108126
};
109127

@@ -368,6 +386,63 @@ function shouldAutoRetry(
368386
return retryableForFallback(error);
369387
}
370388

389+
function shouldOfferManualRetry(
390+
strategy: string,
391+
fallbackStrategy: string | undefined,
392+
error: unknown
393+
): fallbackStrategy is string {
394+
if (strategy !== 'faraday' || !fallbackStrategy) return false;
395+
396+
const details = extractErrorDetails(error);
397+
const errorCode = details?.code?.toUpperCase();
398+
if (errorCode && NON_RETRYABLE_ERROR_CODES.has(errorCode)) return false;
399+
400+
const status = details?.status;
401+
if (status && status < 500) return false;
402+
403+
const message = (details?.message ?? toErrorMessage(error)).toLowerCase();
404+
if (
405+
message.includes('unauthorized') ||
406+
message.includes('forbidden') ||
407+
message.includes('not allowed') ||
408+
message.includes('disabled') ||
409+
message.includes('access token') ||
410+
message.includes('token') ||
411+
message.includes('authentication') ||
412+
message.includes('bad request') ||
413+
message.includes('url') ||
414+
message.includes('unsupported strategy') ||
415+
message.includes('invalid response format') ||
416+
message.includes('not valid json') ||
417+
message.includes('unexpected token')
418+
) {
419+
return false;
420+
}
421+
422+
return true;
423+
}
424+
425+
function extractFailureMetadata(error: unknown): { code?: string; status?: number } {
426+
const details = extractErrorDetails(error);
427+
return {
428+
code: details?.code,
429+
status: details?.status,
430+
};
431+
}
432+
433+
function classifyConversionFailure(error: unknown): ConversionFailureKind {
434+
const details = extractErrorDetails(error);
435+
const code = details?.code?.toUpperCase();
436+
const status = details?.status;
437+
const message = (details?.message ?? toErrorMessage(error)).toLowerCase();
438+
439+
if (code === 'UNAUTHORIZED' || status === 401) return 'auth';
440+
if (code === 'BAD_REQUEST' || status === 400) return 'input';
441+
if (message.includes('network') || message.includes('failed to fetch')) return 'network';
442+
443+
return 'server';
444+
}
445+
371446
function buildRetryFailureMessage(
372447
firstError: unknown,
373448
secondError: unknown,

0 commit comments

Comments
 (0)