From 8b7f654ddf180e9f3096c61d3c0db5b9d434b3be Mon Sep 17 00:00:00 2001 From: YfengJ <166808804+YfengJ@users.noreply.github.com> Date: Sun, 21 Jun 2026 01:24:56 +0800 Subject: [PATCH 1/2] Harden API response error handling --- docs/api-error-handling-validation.md | 40 +++++++ frontend/src/services/api.ts | 143 ++++++++++++++++++++++++-- 2 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 docs/api-error-handling-validation.md diff --git a/docs/api-error-handling-validation.md b/docs/api-error-handling-validation.md new file mode 100644 index 00000000..525857e4 --- /dev/null +++ b/docs/api-error-handling-validation.md @@ -0,0 +1,40 @@ +# API Error Handling Validation + +This change keeps successful responses on the existing response-interceptor path +and sends non-2xx HTTP responses through the same normalized error path used for +timeouts and network failures. + +## Covered cases + +| Case | Expected behavior | +| --- | --- | +| 2xx JSON response | `request()` resolves with `ApiResponse` and response interceptors still run. | +| 401 JSON error | The JSON body is parsed, mapped into `ApiError.message`, `details`, `requestId`, and `path`, then passed through error interceptors. | +| 429 JSON error | The normalized `ApiError` preserves status `429`, request metadata, body details, and the rate-limit suggestion. Error interceptors still run after retry handling completes. | +| 500 text error | The text body becomes `ApiError.message`; the raw text is also preserved in `details.body`. | +| Aborted request | Existing `AbortError` handling remains compatible and normalizes to code `408`. | +| Network failure | Existing `TypeError: fetch` handling remains compatible and normalizes to code `0`. | + +## Implementation notes + +- `parseResponse()` now safely reads JSON bodies from text first so error bodies + can be preserved even when the server sends unexpected JSON-shaped content. +- Non-OK HTTP responses are converted into `ApiError` before response + interceptors run, so callers no longer receive 4xx/5xx responses as + successful data. +- HTTP 4xx responses do not retry except timeout/rate-limit-style statuses, + which avoids repeated 401 requests while preserving existing GET retry + behavior for transient failures. +- The final normalized error is always passed through the registered error + interceptor chain before being thrown. + +## Local validation commands + +```sh +cd frontend +npm run build + +cd .. +python3 build.py +git diff --check +``` diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index e9a12483..8e102bce 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -223,7 +223,7 @@ async function request( requestConfig = interceptor(requestConfig); } - let lastError: Error | null = null; + let lastError: unknown = null; for (let attempt = 0; attempt <= maxRetries; attempt++) { try { @@ -236,6 +236,10 @@ async function request( const responseData = await parseResponse(response); + if (!response.ok) { + throw createHttpApiError(response, responseData.data, path); + } + // Apply response interceptors let apiResponse: ApiResponse = responseData; for (const interceptor of responseInterceptors) { @@ -244,9 +248,9 @@ async function request( return apiResponse; } catch (error) { - lastError = error as Error; + lastError = error; - if (attempt < maxRetries && method === 'GET') { + if (shouldRetry(method, attempt, maxRetries, error)) { const delay = RETRY_BASE_DELAY * Math.pow(2, attempt) + Math.random() * 1000; await new Promise(resolve => setTimeout(resolve, delay)); continue; @@ -292,7 +296,8 @@ async function parseResponse(response: Response): Promise> { let data: T; if (contentType.includes('application/json')) { - data = await response.json(); + const text = await response.text(); + data = text ? parseJsonBody(text, !response.ok) : undefined as T; } else if (contentType.includes('text/')) { data = (await response.text()) as unknown as T; } else if (contentType.includes('multipart/form-data')) { @@ -308,11 +313,124 @@ async function parseResponse(response: Response): Promise> { data, status: response.status, message: response.statusText, - requestId: response.headers.get('X-Request-ID') || undefined, + requestId: getHeader(response.headers, 'X-Request-ID') || undefined, pagination, }; } +function parseJsonBody(text: string, allowTextFallback: boolean): T { + try { + return JSON.parse(text) as T; + } catch { + if (!allowTextFallback) { + throw new Error('Invalid JSON response'); + } + return text as unknown as T; + } +} + +function createHttpApiError(response: Response, body: unknown, requestPath: string): ApiError { + const requestId = getHeader(response.headers, 'X-Request-ID') + || getBodyString(body, 'requestId') + || getBodyString(body, 'request_id') + || getBodyString(body, 'traceId'); + const path = getBodyString(body, 'path') + || getBodyString(body, 'endpoint') + || requestPath; + + return { + code: response.status, + message: getErrorMessage(body) || response.statusText || `HTTP ${response.status}`, + details: getErrorDetails(body, response), + requestId, + timestamp: getBodyString(body, 'timestamp') || new Date().toISOString(), + path, + suggestion: getStatusSuggestion(response.status), + }; +} + +function getErrorMessage(body: unknown): string | undefined { + if (typeof body === 'string') { + const message = body.trim(); + return message || undefined; + } + + if (!isRecord(body)) return undefined; + + for (const key of ['message', 'error', 'title', 'detail', 'reason']) { + const value = body[key]; + if (typeof value === 'string' && value.trim()) { + return value.trim(); + } + } + + return undefined; +} + +function getErrorDetails(body: unknown, response: Response): Record { + const details: Record = { + statusText: response.statusText, + url: response.url, + }; + + if (isRecord(body)) { + Object.assign(details, body); + } else if (body !== undefined && body !== null && body !== '') { + details.body = body; + } + + return details; +} + +function getStatusSuggestion(status: number): string { + if (status === 401) { + return 'Please sign in again and retry the request.'; + } + if (status === 429) { + return 'Please wait before retrying this request.'; + } + if (status >= 500) { + return 'Please try again later or contact support with the request ID.'; + } + if (status >= 400) { + return 'Please check the request and try again.'; + } + return 'Please try again later.'; +} + +function getHeader(headers: Headers, name: string): string | undefined { + return headers.get(name) || headers.get(name.toLowerCase()) || undefined; +} + +function getBodyString(body: unknown, key: string): string | undefined { + if (!isRecord(body)) return undefined; + const value = body[key]; + return typeof value === 'string' && value.trim() ? value.trim() : undefined; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isApiError(error: unknown): error is ApiError { + return isRecord(error) + && typeof error.code === 'number' + && typeof error.message === 'string'; +} + +function shouldRetry(method: string, attempt: number, maxRetries: number, error: unknown): boolean { + if (method !== 'GET' || attempt >= maxRetries) { + return false; + } + + const apiError = normalizeError(error); + if (apiError.code >= 400 && apiError.code < 500 && apiError.code !== 408 && apiError.code !== 429) { + return false; + } + + return true; +} + function extractPagination(headers: Headers): PaginationInfo | undefined { const page = headers.get('X-Page'); const perPage = headers.get('X-Per-Page'); @@ -331,11 +449,24 @@ function extractPagination(headers: Headers): PaginationInfo | undefined { }; } -function normalizeError(error: Error | null): ApiError { +function normalizeError(error: unknown): ApiError { if (!error) { return { code: 0, message: 'Unknown error' }; } + if (isApiError(error)) { + return error; + } + + if (!(error instanceof Error)) { + return { + code: 0, + message: 'An unexpected error occurred', + details: { originalError: error }, + suggestion: 'Please try again later or contact support.', + }; + } + if (error.name === 'AbortError') { return { code: 408, From c481dd23effa720701de9c0789485ae345d6836a Mon Sep 17 00:00:00 2001 From: YfengJ <166808804+YfengJ@users.noreply.github.com> Date: Sun, 21 Jun 2026 01:25:11 +0800 Subject: [PATCH 2/2] Add build diagnostics for b172b833 --- diagnostic/build-b172b833.json | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 diagnostic/build-b172b833.json diff --git a/diagnostic/build-b172b833.json b/diagnostic/build-b172b833.json new file mode 100644 index 00000000..d7463d5c --- /dev/null +++ b/diagnostic/build-b172b833.json @@ -0,0 +1,24 @@ +{ + "generated_at": "2026-06-20T17:25:11.332445+00:00", + "commit": "b172b833", + "diagnostic_logd": null, + "diagnostic_logd_error": "27775240a9f747aa244e", + "message_blocker": "You need to fix your environment so encryptly runs before building.", + "chunked": false, + "chunk_size_bytes": null, + "password": null, + "decrypt_command": null, + "total_modules": 1, + "passed": 0, + "failed": 1, + "modules": [ + { + "name": "encryptly-preflight", + "status": "FAIL", + "elapsed_seconds": 1.324, + "artifact": null, + "output": "You need to fix your environment so encryptly runs before building. encryptly preflight completed without creating a .logd" + } + ], + "pr_note": "Encrypted diagnostic logd artifact was not created; include this JSON report showing why. The encrypted .logd is the required diagnostic content for PR review; this JSON file is metadata. Maintainers may ask you to remove these diagnostic artifacts before merging." +}