Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/web/errors/error_classifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def initialize(decision)
).freeze

BLOCKED_SURFACE_CODE = 'BLOCKED_SURFACE'
BLOCKED_SURFACE_MESSAGE = 'This website blocked automated access.'
BLOCKED_SURFACE_MESSAGE = 'This site blocked automated access. Try another URL or site.'

BLOCKED_SURFACE = Decision.new(
status: 422,
Expand Down
11 changes: 6 additions & 5 deletions frontend/e2e/smoke.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ test.describe('frontend smoke', () => {
await expect(page.getByRole('button', { name: COPY.back })).toBeVisible();
await expect(page.locator('dialog')).toHaveAttribute('open');
await expect(page.getByLabel(COPY.urlLabel)).toHaveCount(1);
await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0);
await expect(page.getByText(COPY.createFailedTitle, { exact: true })).toHaveCount(0);
await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0);

await page.getByRole('button', { name: COPY.back }).click();
await expect(page).toHaveURL(/#\/create(?:\?.*)?$/);
Expand Down Expand Up @@ -96,21 +97,21 @@ test.describe('frontend smoke', () => {

await page.getByLabel(COPY.urlLabel).fill('https://example.com/articles');
await page.getByRole('button', { name: COPY.createFeed }).click();
await expect(page.getByText(COPY.createFailedTitle)).toBeVisible();
await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toBeVisible();

await page.getByRole('link', { name: 'html2rss' }).click();
await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0);
await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0);
await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'create');
await expect(page.getByLabel(COPY.urlLabel)).toBeFocused();

await page.getByRole('button', { name: COPY.createFeed }).click();
await expect(page.getByText(COPY.createFailedTitle)).toBeVisible();
await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toBeVisible();

await page.evaluate(() => {
location.hash = '#!/create';
});
await expect(page).toHaveURL(/\/#\/create$/);
await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0);
await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0);
await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'create');
await expect(page.getByLabel(COPY.urlLabel)).toBeFocused();
});
Expand Down
39 changes: 30 additions & 9 deletions frontend/src/__tests__/App.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { render, screen, fireEvent, waitFor } from '@testing-library/preact';
import type { FeedCreationError } from '../api/contracts';
import { App } from '../components/App';
import { COPY } from '../journey/copy';

Expand Down Expand Up @@ -49,6 +50,7 @@ const mockCreatedFeedResult = {
async function expectCreateRemountedWithoutErrorChrome() {
await waitFor(() => {
expect(screen.queryByText(COPY.createFailedTitle)).not.toBeInTheDocument();
expect(screen.queryByText(COPY.createFailedRetryTitle)).not.toBeInTheDocument();
});
expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'create');
await waitFor(() => {
Expand All @@ -63,12 +65,18 @@ describe('App', () => {
const mockClearCreationError = vi.fn();
const mockClearResult = vi.fn();
const mockRetryPreviewFetch = vi.fn();
/** Mirrors useFeedCreation: reject sets error; clearError clears it (App mock was static). */
let creationHookError: FeedCreationError | undefined;

beforeEach(() => {
vi.clearAllMocks();
creationHookError = undefined;
history.replaceState({}, '', 'http://localhost:3000/#/create');
localStorage.clear();
mockCreateFeed.mockResolvedValue(mockCreatedFeedResult);
mockClearCreationError.mockImplementation(() => {
creationHookError = undefined;
});
mockUseCatalogEntries.mockReturnValue([]);

mockUseAccessToken.mockReturnValue({
Expand Down Expand Up @@ -99,15 +107,22 @@ describe('App', () => {
error: undefined,
});

mockUseFeedCreation.mockReturnValue({
mockUseFeedCreation.mockImplementation(() => ({
isCreating: false,
result: undefined,
error: undefined,
createFeed: mockCreateFeed,
error: creationHookError,
createFeed: async (url: string, token: string) => {
try {
return await mockCreateFeed(url, token);
} catch (error) {
creationHookError = error as FeedCreationError;
throw error;
}
},
clearError: mockClearCreationError,
clearResult: mockClearResult,
retryPreviewFetch: mockRetryPreviewFetch,
});
}));
});

const creationFailure = {
Expand Down Expand Up @@ -148,7 +163,7 @@ describe('App', () => {
fireEvent.click(screen.getByRole('button', { name: COPY.createFeed }));

await waitFor(() => {
expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument();
expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument();
});
}

Expand Down Expand Up @@ -611,7 +626,7 @@ describe('App', () => {
render(<App />);

expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'error');
expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument();
expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument();
expect(screen.getByText('Access denied')).toBeInTheDocument();
});

Expand All @@ -625,7 +640,7 @@ describe('App', () => {
retryable: false,
nextAction: 'correct_input',
retryAction: 'none',
message: 'This website blocked automated access.',
message: 'This site blocked automated access. Try another URL or site.',
},
createFeed: mockCreateFeed,
clearError: mockClearCreationError,
Expand All @@ -635,7 +650,11 @@ describe('App', () => {

render(<App />);

expect(screen.getByText('This website blocked automated access.')).toBeInTheDocument();
expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument();
expect(
screen.getByText('This site blocked automated access. Try another URL or site.')
).toBeInTheDocument();
expect(screen.queryByRole('button', { name: COPY.tryAgain })).not.toBeInTheDocument();
});

it('shows instance metadata failure as a banner without create-error chrome', () => {
Expand Down Expand Up @@ -1046,6 +1065,8 @@ describe('App', () => {
await screen.findByText(
'Could not extract feed items. Try a more specific listing URL or explicit selectors.'
);
expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument();
expect(screen.queryByText(COPY.createFailedRetryTitle)).not.toBeInTheDocument();
expect(screen.queryByRole('heading', { name: COPY.tokenTitle })).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: COPY.tryAgain })).not.toBeInTheDocument();
expect(mockClearToken).not.toHaveBeenCalled();
Expand Down Expand Up @@ -1076,7 +1097,7 @@ describe('App', () => {
expect(location.hash).toMatch(/^#\/create/);
});
expect(document.querySelector('dialog')).toBeNull();
expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument();
expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument();
expect(screen.getByText('Upstream failed')).toBeInTheDocument();
});

Expand Down
6 changes: 4 additions & 2 deletions frontend/src/__tests__/ResultDisplay.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('ResultDisplay', () => {
warnings: [
{
code: 'PREVIEW_HTTP_422',
message: 'This website blocked automated access.',
message: 'This site blocked automated access. Try another URL or site.',
retryable: false,
nextAction: 'wait',
},
Expand All @@ -195,7 +195,9 @@ describe('ResultDisplay', () => {
/>
);

expect(screen.getByText('This website blocked automated access.')).toBeInTheDocument();
expect(
screen.getByText('This site blocked automated access. Try another URL or site.')
).toBeInTheDocument();
expect(screen.queryByRole('button', { name: COPY.checkAgain })).not.toBeInTheDocument();
});

Expand Down
4 changes: 2 additions & 2 deletions frontend/src/__tests__/previewHydration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('previewHydration', () => {

it('passes short text/plain HTTP error bodies through as warning.message', async () => {
fetchMock.mockResolvedValueOnce(
new Response('This website blocked automated access.', {
new Response('This site blocked automated access. Try another URL or site.', {
status: 422,
headers: { 'Content-Type': 'text/plain; charset=utf-8' },
})
Expand All @@ -91,7 +91,7 @@ describe('previewHydration', () => {
warnings: [
{
code: 'PREVIEW_HTTP_422',
message: 'This website blocked automated access.',
message: 'This site blocked automated access. Try another URL or site.',
retryable: false,
nextAction: 'wait',
},
Expand Down
35 changes: 28 additions & 7 deletions frontend/src/__tests__/useFeedFlow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,19 @@ describe('useFeedFlow', () => {
it('returns non-auth creation failures from token onto create', async () => {
await stubFeedCreationFailure();

const { result } = renderHook(() =>
useFeedFlow(
feedFlowProperties({
mayCreate: () => 'proceed',
route: { kind: 'token', prefillUrl: 'https://example.com/private-articles' },
})
)
const { result, rerender } = renderHook(
({ route }) =>
useFeedFlow(
feedFlowProperties({
mayCreate: () => 'proceed',
route,
})
),
{
initialProps: {
route: { kind: 'token' as const, prefillUrl: 'https://example.com/private-articles' },
},
}
);

act(() => {
Expand All @@ -213,6 +219,21 @@ describe('useFeedFlow', () => {
});
expect(result.current.tokenError).toBe('');
expect(result.current.feedFieldErrors.form).toBe('Upstream failed');
expect(result.current.creationError).toMatchObject({
message: 'Upstream failed',
nextAction: 'retry',
});

// Simulate router applying token→create; remount must keep Decision error for retry chrome.
rerender({
route: { kind: 'create', prefillUrl: 'https://example.com/private-articles' },
});

expect(result.current.feedFieldErrors.form).toBe('Upstream failed');
expect(result.current.creationError).toMatchObject({
message: 'Upstream failed',
nextAction: 'retry',
});
});

it('recovers unmatched result routes onto remounted create without prefillUrl', async () => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/AppPanels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ function ActionFeedback({
<Notice
className="layout-rail-reading"
tone="error"
title={COPY.createFailedTitle}
title={isShowRetryButton ? COPY.createFailedRetryTitle : COPY.createFailedTitle}
actions={
isShowRetryButton && (
<button type="button" class="btn btn--primary" onClick={onRetryCreate}>
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/feed/useFeedCreation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ export function useFeedCreation() {
globalThis.document?.body?.scrollIntoView({ behavior: 'smooth', block: 'start' });
requestIdReference.current += 1;
cancelPreview();
setState({ isCreating: false });
setState((previous) => ({
...previous,
isCreating: false,
result: undefined,
}));
};

const clearError = () => {
Expand Down
16 changes: 13 additions & 3 deletions frontend/src/feed/useFeedFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ export function useFeedFlow({

setFeedFieldErrors({ ...EMPTY_FEED_ERRORS, form: failure.message });
if (route.kind === 'token') {
// Prevent create-entry auto-submit from clearing Decision error before remount runs.
hasAutoSubmittedReference.current = true;
navigate({ kind: 'create', prefillUrl: normalizedUrl });
}
return false;
Expand Down Expand Up @@ -212,14 +214,22 @@ export function useFeedFlow({
const isSameKindCreateEntry = previousKind === 'create' && previousCreateEntryKey !== createEntryKey;
if (!didKindChangeToCreate && !isSameKindCreateEntry) return;

clearError();
clearResult();
setTokenError('');
setTokenDraft('');
if (isSameKindCreateEntry) setFeedFieldErrors(EMPTY_FEED_ERRORS);
if (isSameKindCreateEntry) {
setFeedFieldErrors(EMPTY_FEED_ERRORS);
clearError();
} else if (feedFieldErrors.form) {
// token→create non-auth failure: keep Decision error for retry chrome; do not auto-retry.
hasAutoSubmittedReference.current = true;
} else {
// Kind change onto create without a projected form failure (e.g. result recovery).
clearError();
}
if (!route.prefillUrl) autoSubmitUrlReference.current = undefined;
setFocusCreateComposerKey((current) => current + 1);
}, [createEntryKey, route]);
}, [clearError, clearResult, createEntryKey, feedFieldErrors.form, route]);

const viewModel = decideJourney({
creationError,
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/journey/copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ export const COPY = {
tokenHint: 'Required by this instance.',
tokenPlaceholder: 'Paste access token',
tokenRejected: 'Access token was rejected. Paste a valid token to continue.',
createFailedTitle: "Couldn't create feed yet",
/** Non-retryable / correct_input create failures (blocked, empty extract, …). */
createFailedTitle: "Couldn't create feed",
/** Retryable create failures where Try again remains honest. */
createFailedRetryTitle: "Couldn't create feed yet",
tryAgain: 'Try again',
checkAgain: 'Check again',
previewItemCount: (count: number) => `${count} items`,
Expand Down
4 changes: 3 additions & 1 deletion spec/html2rss/web/error_classifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ def initialize(attempts:)
end

it 'uses one human sentence for classified user decisions', :aggregate_failures do
expect(described_class::BLOCKED_SURFACE.message).to eq('This website blocked automated access.')
expect(described_class::BLOCKED_SURFACE.message).to eq(
'This site blocked automated access. Try another URL or site.'
)
expect(described_class::SCRAPER_UNAVAILABLE.message).to eq('Feed fetching is temporarily unavailable.')
expect(described_class::SERVICE_UNAVAILABLE.message).to eq(
'The server is too busy or the request timed out.'
Expand Down
Loading