Fixes/request deduplication #44

Merged
ilia merged 4 commits from fixes/request-deduplication into main 2026-05-28 19:54:58 +00:00
6 changed files with 59 additions and 14 deletions
Showing only changes of commit 7f20f36d0a - Show all commits
@@ -21,6 +21,7 @@ vi.mock('$shared/api/api', () => ({
import { api } from '$shared/api/api'; import { api } from '$shared/api/api';
import { queryClient } from '$shared/api/queryClient'; import { queryClient } from '$shared/api/queryClient';
import { fontKeys } from '$shared/api/queryKeys'; import { fontKeys } from '$shared/api/queryKeys';
import { FontResponseError } from '../../lib/errors/errors';
import { import {
fetchFontsByIds, fetchFontsByIds,
fetchProxyFontById, fetchProxyFontById,
@@ -86,16 +87,20 @@ describe('proxyFonts', () => {
expect(calledUrl).toContain('offset=0'); expect(calledUrl).toContain('offset=0');
}); });
test('should throw on invalid response (missing fonts array)', async () => { test('should throw FontResponseError on invalid response (missing fonts array)', async () => {
mockApiGet({ total: 0 }); mockApiGet({ total: 0 });
await expect(fetchProxyFonts()).rejects.toThrow('Proxy API returned invalid response'); await expect(fetchProxyFonts()).rejects.toSatisfy(
e => e instanceof FontResponseError && e.field === 'response.fonts',
);
}); });
test('should throw on null response data', async () => { test('should throw FontResponseError on null response data', async () => {
vi.mocked(api.get).mockResolvedValueOnce({ data: null, status: 200 }); vi.mocked(api.get).mockResolvedValueOnce({ data: null, status: 200 });
await expect(fetchProxyFonts()).rejects.toThrow('Proxy API returned invalid response'); await expect(fetchProxyFonts()).rejects.toSatisfy(
e => e instanceof FontResponseError && e.field === 'response',
);
}); });
}); });
+13 -4
View File
@@ -15,6 +15,7 @@ import { queryClient } from '$shared/api/queryClient';
import { fontKeys } from '$shared/api/queryKeys'; import { fontKeys } from '$shared/api/queryKeys';
import { buildQueryString } from '$shared/lib/utils'; import { buildQueryString } from '$shared/lib/utils';
import type { QueryParams } from '$shared/lib/utils'; import type { QueryParams } from '$shared/lib/utils';
import { FontResponseError } from '../../lib/errors/errors';
import type { UnifiedFont } from '../../model/types'; import type { UnifiedFont } from '../../model/types';
/** /**
@@ -96,11 +97,16 @@ export interface ProxyFontsParams extends QueryParams {
/** /**
* Proxy API response * Proxy API response
* *
* Includes pagination metadata alongside font data * Includes pagination metadata alongside font data.
*
* Contract: `fonts` is always an array — never `null` or omitted, even when
* `total === 0`. Returning `null` on the wire is a backend regression and
* surfaces as FontResponseError (non-retryable) on the client.
*/ */
export interface ProxyFontsResponse { export interface ProxyFontsResponse {
/** /**
* List of font objects returned by the proxy * List of font objects returned by the proxy.
* Always an array; empty when no matches.
*/ */
fonts: UnifiedFont[]; fonts: UnifiedFont[];
@@ -156,8 +162,11 @@ export async function fetchProxyFonts(
const response = await api.get<ProxyFontsResponse>(url); const response = await api.get<ProxyFontsResponse>(url);
if (!response.data || !Array.isArray(response.data.fonts)) { if (!response.data) {
throw new Error('Proxy API returned invalid response'); throw new FontResponseError('response', response.data);
}
if (!Array.isArray(response.data.fonts)) {
throw new FontResponseError('response.fonts', response.data.fonts);
} }
return response.data; return response.data;
+5 -1
View File
@@ -1,3 +1,5 @@
import { NonRetryableError } from '$shared/api/queryClient';
/** /**
* Thrown when the network request to the proxy API fails. * Thrown when the network request to the proxy API fails.
* Wraps the underlying fetch error (timeout, DNS failure, connection refused, etc.). * Wraps the underlying fetch error (timeout, DNS failure, connection refused, etc.).
@@ -12,11 +14,13 @@ export class FontNetworkError extends Error {
/** /**
* Thrown when the proxy API returns a response with an unexpected shape. * Thrown when the proxy API returns a response with an unexpected shape.
* Extends NonRetryableError because schema mismatches are not transient —
* retrying will produce the same failure and only delay surfacing the bug.
* *
* @property field - The name of the field that failed validation (e.g. `'response'`, `'response.fonts'`). * @property field - The name of the field that failed validation (e.g. `'response'`, `'response.fonts'`).
* @property received - The actual value received at that field, for debugging. * @property received - The actual value received at that field, for debugging.
*/ */
export class FontResponseError extends Error { export class FontResponseError extends NonRetryableError {
readonly name = 'FontResponseError'; readonly name = 'FontResponseError';
constructor( constructor(
@@ -441,6 +441,11 @@ export class FontCatalogStore {
try { try {
response = await fetchProxyFonts(params); response = await fetchProxyFonts(params);
} catch (cause) { } catch (cause) {
// Preserve non-retryable validation errors so the query client doesn't
// burn the retry budget on a deterministic schema mismatch.
if (cause instanceof FontResponseError) {
throw cause;
}
throw new FontNetworkError(cause); throw new FontNetworkError(cause);
} }
@@ -9,6 +9,7 @@
import { api } from '$shared/api/api'; import { api } from '$shared/api/api';
import { API_ENDPOINTS } from '$shared/api/endpoints'; import { API_ENDPOINTS } from '$shared/api/endpoints';
import { NonRetryableError } from '$shared/api/queryClient';
const PROXY_API_URL = API_ENDPOINTS.filters; const PROXY_API_URL = API_ENDPOINTS.filters;
@@ -37,7 +38,8 @@ export interface FilterMetadata {
type: 'enum' | 'string' | 'array'; type: 'enum' | 'string' | 'array';
/** /**
* Available filter options * Available filter options.
* Always an array; empty when the group has no options.
*/ */
options: FilterOption[]; options: FilterOption[];
} }
@@ -68,11 +70,16 @@ export interface FilterOption {
} }
/** /**
* Proxy filters API response * Proxy filters API response.
*
* Contract: `filters` (and each nested `options`) is always an array — never
* `null` or omitted. Wire-level `null` here is a backend regression and
* surfaces as a non-retryable error on the client.
*/ */
export interface ProxyFiltersResponse { export interface ProxyFiltersResponse {
/** /**
* Array of filter metadata * Array of filter metadata.
* Always an array; empty when no filter groups are configured.
*/ */
filters: FilterMetadata[]; filters: FilterMetadata[];
} }
@@ -99,7 +106,7 @@ export async function fetchProxyFilters(): Promise<FilterMetadata[]> {
const response = await api.get<FilterMetadata[]>(PROXY_API_URL); const response = await api.get<FilterMetadata[]>(PROXY_API_URL);
if (!response.data || !Array.isArray(response.data)) { if (!response.data || !Array.isArray(response.data)) {
throw new Error('Proxy API returned invalid response'); throw new NonRetryableError('Proxy API returned invalid filters response');
} }
return response.data; return response.data;
+16 -1
View File
@@ -1,5 +1,15 @@
import { QueryClient } from '@tanstack/query-core'; import { QueryClient } from '@tanstack/query-core';
/**
* Marker base class for errors that retrying will never fix — schema-validation
* failures, unauthorized responses, contract violations, etc.
*
* The queryClient retry handler short-circuits when it sees this; without it,
* a non-transient backend bug pins the UI through the full retry budget
* (default 3× exponential backoff ≈ 7s).
*/
export class NonRetryableError extends Error {}
/** /**
* Data remains fresh for this long after fetch. Stores that override * Data remains fresh for this long after fetch. Stores that override
* staleness (e.g. filtered queries) can use 0 to bypass. * staleness (e.g. filtered queries) can use 0 to bypass.
@@ -51,7 +61,12 @@ export const queryClient = new QueryClient({
* Refetch on mount if data is stale * Refetch on mount if data is stale
*/ */
refetchOnMount: true, refetchOnMount: true,
retry: QUERY_RETRY_COUNT, retry: (failureCount, error) => {
if (error instanceof NonRetryableError) {
return false;
}
return failureCount < QUERY_RETRY_COUNT;
},
/** /**
* Exponential backoff: 1s, 2s, 4s, 8s... capped at 30s * Exponential backoff: 1s, 2s, 4s, 8s... capped at 30s
*/ */