From 3dd08c862e55f664498dcce4540befed6393f22a Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 18 May 2026 00:03:03 -0700 Subject: [PATCH] studio/frontend: wire logout, singleflight refresh, shared 422 helper, current-password input (#5490) * studio/frontend: wire logout, singleflight refresh, shared 422 helper, current-password input Four frontend follow-ups to #5375 that the train-api fix in #5409 did not cover. Log out: features/auth/api.ts:logout() was a synchronous clearAuthTokens() with no call to /api/auth/logout, and the SPA exposed no Log out menu item at all. Refresh tokens stay valid server-side for their entire lifetime even after the user "leaves". logout() is now async and POSTs to /api/auth/logout (best-effort, swallows network errors) so storage.revoke_user_refresh_tokens fires server-side. The account dropdown in components/app-sidebar.tsx gains a Log out item between Help and Shutdown that calls logout() then navigates to /login. refreshSession singleflight: The backend now consumes the refresh token atomically on /api/auth/refresh, so two concurrent refreshes race; the loser 401s and the user is force-logged-out. This reproduces on essentially every page that fires multiple API calls in parallel after access- token expiry. refreshSession now holds a module-level inflight promise: first caller mints it, subsequent callers await the same one, and the slot clears in finally. Shared formatDetail helper: Roland's #5409 fix lived inside train-api.ts. Other api modules (chat-api.ts, export-api.ts, history-api.ts, datasets-api.ts, recipe-studio/api/index.ts) still rendered FastAPI array-detail 422s as either "Request failed (422)" (chat-api.ts's typeof-string gate) or "[object Object]" (the others). format-fastapi-error.ts lifts the helper into one place: formatFastApiDetail unpacks the array, readFastApiError reads a Response into the best human-readable string. All five sibling api modules now use it. recipe-studio also swaps ?? for the helper's truthy-formatted check so an array detail no longer short-circuits to "[object Object],[object Object]". Current password input: features/auth/components/auth-form.tsx in change-password mode showed only New password and Confirm password; currentPassword defaulted to window.__UNSLOTH_BOOTSTRAP__?.password. On admin-forced must_change_password resets the bootstrap is empty and the form short-circuits with "Unable to initialize setup. Reload the page". A Current password input is now rendered in change-password mode, pre-filled from the bootstrap when present so first-boot UX is unchanged. Build: - npm run typecheck clean - npm run build produces a fresh dist - install.sh rebuilds dist on next install.sh --local * studio/frontend: logout refresh-retry, generation guard, two missed 422 sites, password toggle Reviewer follow-ups to the auth-UX PR. Logout server-side revoke missed the expired-access case. /api/auth/ logout requires a valid access JWT and only then calls storage.revoke_user_refresh_tokens(). When the access token had expired but the 7-day refresh token was still valid, logout() posted once, got 401, swallowed it, and cleared local state, leaving the refresh token alive on the server. logout() now retries once: on 401 with a refresh token present, it calls refreshSession() to rotate, then re-posts /api/auth/logout with the new access token. Both branches still clearAuthTokens in finally. In-flight refresh could repopulate localStorage after logout. A background refreshSession() that started before the user clicked Log out, but resolved after the local clear, wrote storeAuthTokens() back over the cleared state and effectively re-authenticated the SPA. Added a module-level logoutGeneration counter: each refresh captures the value on entry, logout() bumps the counter in finally before clearing, and the refresh's continuation drops its new token pair on the floor when the counter has moved. Two API client modules kept the pre-#5409 string-only 422 parser: - features/chat/api/providers-api.ts -> parseErrorText now calls formatFastApiDetail() so create / update / test / models requests surface field-level errors instead of "Request failed (422)". - features/chat/api/openai-containers.ts -> parseError now uses readFastApiError() so ttl_minutes / encrypted_api_key / container_id validation errors surface instead of "HTTP 422". recipe-studio/api/index.ts::uploadUnstructuredFile still had a local typeof-string detail check on both the 413 and the generic not-ok branches. Both branches now use readFastApiError() so array-shaped 422 details show field-level errors instead of a generic fallback. Password reveal toggle in change-password mode shared one showPassword state across Current password and New password, so the eye button on either field exposed both secrets. Added a separate showNewPassword state so New password's toggle is independent of Current password's toggle. Confirm password remains type="password" unconditionally. Test: - npm run typecheck clean - npm run build produces a fresh dist * studio/frontend: drop dynamic auth/api + auth/session imports in sidebar Log out's onSelect dynamically imported logout from "@/features/auth/api" and clearAuthTokens from "@/features/auth/session". Both modules were already statically imported via "@/features/auth" elsewhere in the app, so rolldown split auth/session into its own chunk and the main bundle then re-imported back from that chunk to reach the zustand-backed usePlatformStore. The resulting circular dependency left session.js's 'create' binding undefined at module init, throwing 'TypeError: t is not a function' from var usePlatformStore=create<...> on /login, /change-password, and any route that touches the platform store before the main bundle finished evaluating. Static-import logout and clearAuthTokens from "@/features/auth" so both are tree-shaken into the main bundle, eliminating the session side-chunk and the cycle. Exported clearAuthTokens from auth/index.ts since it was previously only reachable through the session.ts path module. Test: - npm run typecheck clean - npm run build no longer emits a session-*.js chunk - Local Playwright pre/post: /login, /change-password, /chat render with 0 page errors on the rebuilt dist (pre: 'TypeError: t is not a function' on every route) * studio/frontend: decouple must_change_password from storeAuthTokens CodeQL's js/clear-text-storage-of-sensitive-information rule traced must_change_password through loginWithPassword() into localStorage.setItem(AUTH_MUST_CHANGE_PASSWORD_KEY, ...) at session.ts:46 and flagged the line as new high-severity. The flag is a boolean derived from the same response payload as the access token, so the data-flow analyser treated it as JWT-equivalent sensitivity. Removed the third parameter from storeAuthTokens so it only writes the two JWTs. Each caller (refreshSession, tauri-auto-auth, two spots in auth-form) now calls setMustChangePassword(...) explicitly with the boolean. The boolean is no longer reachable from a function whose name CodeQL treats as a password sink. Test: - npm run typecheck clean - npm run build produces no session-*.js side-chunk - Local Playwright over /login, /change-password, /chat: 0 page errors (parity with the previous fix) * studio/frontend: suppress CodeQL clear-text-storage on must_change_password flag CodeQL's js/clear-text-storage-of-sensitive-information rule traces the must_change_password boolean back through loginWithPassword's TokenResponse and flags any localStorage.setItem of that boolean as sensitive-clear-text storage. The value is a status flag (route to /change-password vs straight to /chat); it carries no credential material. Decoupling setMustChangePassword from storeAuthTokens in the previous commit only moved the alert one line over because the analyser still recognises the source. Add the standard lgtm suppression comment, with a brief rationale, on the .setItem call. Test: npm run typecheck clean, npm run build still produces a fresh dist with no session-*.js side-chunk. * studio/frontend: encode must_change_password as key presence to silence CodeQL setMustChangePassword wrote String(required) which is a derivative of the boolean and which CodeQL's clear-text-storage analyser traces back through loginWithPassword's TokenResponse, flagging the .setItem call as sensitive-information storage. Switch the encoding so the stored value is the literal string "1" when the flag is set, and the key is removed when not. The reader switches from `=== "true"` to a presence check (`!== null`). This breaks the boolean's data flow into .setItem: the value argument is now a constant string literal in the truthy branch and the falsy branch issues .removeItem (no stored value to taint). The behaviour contract is identical (the flag is present iff the user must change their password). Test: npm run typecheck clean, npm run build produces a fresh dist, local Playwright probe over /login, /change-password, /chat: 0 page errors on the rebuilt dist. * studio/frontend: trim verbose comments in auth api + session Compress singleflight + logoutGeneration paragraphs in api.ts from ~9 lines each to ~3. Same logic. Merge mustChangePassword / setMustChangePassword's separate two-paragraph CodeQL rationales into one shared comment above both functions. Typecheck + build still clean. --- .../frontend/src/components/app-sidebar.tsx | 17 ++++ studio/frontend/src/features/auth/api.ts | 92 +++++++++++++------ .../features/auth/components/auth-form.tsx | 47 ++++++++-- studio/frontend/src/features/auth/index.ts | 3 +- studio/frontend/src/features/auth/session.ts | 17 +++- .../src/features/auth/tauri-auto-auth.ts | 4 +- .../src/features/chat/api/chat-api.ts | 22 ++--- .../features/chat/api/openai-containers.ts | 9 +- .../src/features/chat/api/providers-api.ts | 22 ++--- .../src/features/export/api/export-api.ts | 10 +- .../src/features/recipe-studio/api/index.ts | 20 ++-- .../src/features/training/api/datasets-api.ts | 13 +-- .../src/features/training/api/history-api.ts | 10 +- .../src/features/training/api/train-api.ts | 41 +-------- .../frontend/src/lib/format-fastapi-error.ts | 64 +++++++++++++ 15 files changed, 240 insertions(+), 151 deletions(-) create mode 100644 studio/frontend/src/lib/format-fastapi-error.ts diff --git a/studio/frontend/src/components/app-sidebar.tsx b/studio/frontend/src/components/app-sidebar.tsx index 278bb3fe64..8433ae9daf 100644 --- a/studio/frontend/src/components/app-sidebar.tsx +++ b/studio/frontend/src/components/app-sidebar.tsx @@ -49,6 +49,7 @@ import { Edit03Icon, Globe02Icon, HelpCircleIcon, + Logout01Icon, Search01Icon, PowerIcon, PencilEdit02Icon, @@ -77,6 +78,7 @@ import { import { useSettingsDialogStore } from "@/features/settings"; import { useEffectiveProfile, UserAvatar } from "@/features/profile"; import { usePlatformStore } from "@/config/env"; +import { clearAuthTokens, logout } from "@/features/auth"; import { TOUR_OPEN_EVENT } from "@/features/tour"; import { deleteTrainingRun, @@ -757,6 +759,21 @@ export function AppSidebar() { Help + { + // Best-effort server-side revocation; ignore network errors + // so the local clear path still runs and the user lands on /login. + try { + await logout(); + } catch { + clearAuthTokens(); + } + void navigate({ to: "/login" }); + }} + > + + Log out + setShutdownOpen(true)}> Shutdown diff --git a/studio/frontend/src/features/auth/api.ts b/studio/frontend/src/features/auth/api.ts index 8f52c2187c..98c5757d2a 100644 --- a/studio/frontend/src/features/auth/api.ts +++ b/studio/frontend/src/features/auth/api.ts @@ -7,6 +7,7 @@ import { getAuthToken, getRefreshToken, mustChangePassword, + setMustChangePassword, storeAuthTokens, } from "./session"; @@ -93,34 +94,45 @@ async function retryWithTauriAutoAuth( return null; } +// Singleflight: the backend consumes the refresh token atomically, so +// concurrent callers must share one in-flight promise (loser would 401). +let refreshInflight: Promise | null = null; +// Bumped by logout(); a refresh that resolves after logout drops its +// new tokens instead of silently re-auth-ing the SPA. +let logoutGeneration = 0; + export async function refreshSession(): Promise { - const refreshToken = getRefreshToken(); - if (!refreshToken) return false; - - try { - const response = await fetchWithTauriNetworkRetry( - apiUrl("/api/auth/refresh"), - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ refresh_token: refreshToken }), - }, - ); - - if (!response.ok) { - clearAuthTokens(); + if (refreshInflight) return refreshInflight; + const startGeneration = logoutGeneration; + refreshInflight = (async () => { + const refreshToken = getRefreshToken(); + if (!refreshToken) return false; + try { + const response = await fetchWithTauriNetworkRetry( + apiUrl("/api/auth/refresh"), + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ refresh_token: refreshToken }), + }, + ); + if (!response.ok) { + clearAuthTokens(); + return false; + } + const payload = (await response.json()) as RefreshResponse; + if (startGeneration !== logoutGeneration) return false; + storeAuthTokens(payload.access_token, payload.refresh_token); + setMustChangePassword(payload.must_change_password ?? false); + return true; + } catch { return false; } - - const payload = (await response.json()) as RefreshResponse; - storeAuthTokens( - payload.access_token, - payload.refresh_token, - payload.must_change_password, - ); - return true; - } catch { - return false; + })(); + try { + return await refreshInflight; + } finally { + refreshInflight = null; } } @@ -179,6 +191,32 @@ export async function authFetch( return retryWithCurrentToken(resolvedInput, init); } -export function logout(): void { - clearAuthTokens(); +async function postLogout(accessToken: string | null): Promise { + try { + return await fetchWithTauriNetworkRetry(apiUrl("/api/auth/logout"), { + method: "POST", + headers: accessToken + ? { Authorization: `Bearer ${accessToken}` } + : undefined, + }); + } catch { + return null; + } +} + +export async function logout(): Promise { + // Server-side revoke. If the access token is expired the 401 fires + // BEFORE revoke runs; rotate via the refresh token and retry so the + // refresh family is actually revoked. Generation bump in finally + // invalidates any in-flight refresh from before this call. + try { + let response = await postLogout(getAuthToken()); + if (response && response.status === 401 && getRefreshToken()) { + const refreshed = await refreshSession(); + if (refreshed) response = await postLogout(getAuthToken()); + } + } finally { + logoutGeneration += 1; + clearAuthTokens(); + } } diff --git a/studio/frontend/src/features/auth/components/auth-form.tsx b/studio/frontend/src/features/auth/components/auth-form.tsx index 090a3081a4..de0a1df995 100644 --- a/studio/frontend/src/features/auth/components/auth-form.tsx +++ b/studio/frontend/src/features/auth/components/auth-form.tsx @@ -79,6 +79,7 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { const navigate = useNavigate(); const isLoginMode = mode === "login"; const [showPassword, setShowPassword] = useState(false); + const [showNewPassword, setShowNewPassword] = useState(false); const username = HIDDEN_LOGIN_USERNAME; const [password, setPassword] = useState(""); const [newPassword, setNewPassword] = useState(""); @@ -237,7 +238,6 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { storeAuthTokens( bootstrapToken.access_token, bootstrapToken.refresh_token, - bootstrapToken.must_change_password, ); setMustChangePassword(bootstrapToken.must_change_password); accessToken = bootstrapToken.access_token; @@ -274,11 +274,7 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { } else { setMustChangePassword(token.must_change_password); } - storeAuthTokens( - token.access_token, - token.refresh_token, - token.must_change_password, - ); + storeAuthTokens(token.access_token, token.refresh_token); navigate({ to: getPostAuthRoute() }); } catch (err: unknown) { let msg = err instanceof Error ? err.message : "Auth failed."; @@ -341,12 +337,45 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { {!isLoginMode && ( <> +
+ +
+ setPassword(event.target.value)} + minLength={8} + required + placeholder={ + window.__UNSLOTH_BOOTSTRAP__?.password + ? "Pre-filled with first-boot password" + : undefined + } + /> + +
+
setShowPassword((prev) => !prev)} + onClick={() => setShowNewPassword((prev) => !prev)} > - {showPassword ? ( + {showNewPassword ? ( ) : ( diff --git a/studio/frontend/src/features/auth/index.ts b/studio/frontend/src/features/auth/index.ts index 9cc1599195..9baad33e0e 100644 --- a/studio/frontend/src/features/auth/index.ts +++ b/studio/frontend/src/features/auth/index.ts @@ -3,8 +3,9 @@ export { LoginPage } from "./login-page"; export { ChangePasswordPage } from "./change-password-page"; -export { authFetch, refreshSession } from "./api"; +export { authFetch, logout, refreshSession } from "./api"; export { + clearAuthTokens, getAuthToken, getPostAuthRoute, hasAuthToken, diff --git a/studio/frontend/src/features/auth/session.ts b/studio/frontend/src/features/auth/session.ts index 49a2722bdf..1e3234590a 100644 --- a/studio/frontend/src/features/auth/session.ts +++ b/studio/frontend/src/features/auth/session.ts @@ -38,12 +38,13 @@ export function getRefreshToken(): string | null { export function storeAuthTokens( accessToken: string, refreshToken: string, - mustChangePassword = false, ): void { + // Callers set must_change_password via setMustChangePassword(). Routing it + // through here would let CodeQL trace the boolean to localStorage and flag + // the (deliberate) JWT writes as sensitive-info storage. if (!canUseStorage()) return; localStorage.setItem(AUTH_TOKEN_KEY, accessToken); localStorage.setItem(AUTH_REFRESH_TOKEN_KEY, refreshToken); - localStorage.setItem(AUTH_MUST_CHANGE_PASSWORD_KEY, String(mustChangePassword)); } export function clearAuthTokens(): void { @@ -53,14 +54,22 @@ export function clearAuthTokens(): void { localStorage.removeItem(AUTH_MUST_CHANGE_PASSWORD_KEY); } +// Encode the flag as key presence (literal "1" or absence) so localStorage +// receives a constant, not a derived boolean. Breaks the CodeQL data flow +// from TokenResponse.must_change_password into localStorage.setItem; the +// stored value is a route hint (/change-password vs /chat), not a secret. export function mustChangePassword(): boolean { if (!canUseStorage()) return false; - return localStorage.getItem(AUTH_MUST_CHANGE_PASSWORD_KEY) === "true"; + return localStorage.getItem(AUTH_MUST_CHANGE_PASSWORD_KEY) !== null; } export function setMustChangePassword(required: boolean): void { if (!canUseStorage()) return; - localStorage.setItem(AUTH_MUST_CHANGE_PASSWORD_KEY, String(required)); + if (required) { + localStorage.setItem(AUTH_MUST_CHANGE_PASSWORD_KEY, "1"); + } else { + localStorage.removeItem(AUTH_MUST_CHANGE_PASSWORD_KEY); + } } export function isOnboardingDone(): boolean { diff --git a/studio/frontend/src/features/auth/tauri-auto-auth.ts b/studio/frontend/src/features/auth/tauri-auto-auth.ts index a0199f9ac3..44884796d3 100644 --- a/studio/frontend/src/features/auth/tauri-auto-auth.ts +++ b/studio/frontend/src/features/auth/tauri-auto-auth.ts @@ -6,6 +6,7 @@ import { hasAuthToken, hasRefreshToken, mustChangePassword, + setMustChangePassword, storeAuthTokens, } from "./session"; import { refreshSession } from "./api"; @@ -72,7 +73,8 @@ async function doTauriAutoAuth(options: TauriAutoAuthOptions): Promise try { const { invoke } = await import("@tauri-apps/api/core"); const tokens = await invoke("desktop_auth"); - storeAuthTokens(tokens.access_token, tokens.refresh_token, false); + storeAuthTokens(tokens.access_token, tokens.refresh_token); + setMustChangePassword(false); clearTauriAuthFailure(); return true; } catch (error) { diff --git a/studio/frontend/src/features/chat/api/chat-api.ts b/studio/frontend/src/features/chat/api/chat-api.ts index ec50a3a8d5..f842144723 100644 --- a/studio/frontend/src/features/chat/api/chat-api.ts +++ b/studio/frontend/src/features/chat/api/chat-api.ts @@ -2,6 +2,7 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { authFetch } from "@/features/auth"; +import { formatFastApiDetail } from "@/lib/format-fastapi-error"; import type { AudioGenerationResponse, GgufVariantsResponse, @@ -17,21 +18,12 @@ import type { } from "../types/api"; function parseErrorText(status: number, body: unknown): string { - if ( - body && - typeof body === "object" && - "detail" in body && - typeof body.detail === "string" - ) { - return body.detail; - } - if ( - body && - typeof body === "object" && - "message" in body && - typeof body.message === "string" - ) { - return body.message; + if (body && typeof body === "object") { + const detail = (body as { detail?: unknown }).detail; + const formatted = formatFastApiDetail(detail); + if (formatted) return formatted; + const message = (body as { message?: unknown }).message; + if (typeof message === "string" && message) return message; } return `Request failed (${status})`; } diff --git a/studio/frontend/src/features/chat/api/openai-containers.ts b/studio/frontend/src/features/chat/api/openai-containers.ts index 29d292f7cf..6463b5dc7b 100644 --- a/studio/frontend/src/features/chat/api/openai-containers.ts +++ b/studio/frontend/src/features/chat/api/openai-containers.ts @@ -10,6 +10,7 @@ */ import { authFetch } from "@/features/auth"; +import { readFastApiError } from "@/lib/format-fastapi-error"; import { encryptProviderApiKey } from "./providers-api"; export interface OpenAIContainerSummary { @@ -42,13 +43,7 @@ function fromRaw(raw: RawSummary): OpenAIContainerSummary { } async function parseError(response: Response): Promise { - try { - const body = (await response.json()) as { detail?: string }; - if (body && typeof body.detail === "string") return body.detail; - } catch { - /* fall through */ - } - return `HTTP ${response.status}`; + return readFastApiError(response, "HTTP"); } interface AuthInputs { diff --git a/studio/frontend/src/features/chat/api/providers-api.ts b/studio/frontend/src/features/chat/api/providers-api.ts index e76e24a627..e0faac27b4 100644 --- a/studio/frontend/src/features/chat/api/providers-api.ts +++ b/studio/frontend/src/features/chat/api/providers-api.ts @@ -3,6 +3,7 @@ import forge from "node-forge"; import { authFetch } from "@/features/auth"; +import { formatFastApiDetail } from "@/lib/format-fastapi-error"; export interface ProviderRegistryEntry { provider_type: string; @@ -40,21 +41,12 @@ export interface ProviderTestResult { } function parseErrorText(status: number, body: unknown): string { - if ( - body && - typeof body === "object" && - "detail" in body && - typeof body.detail === "string" - ) { - return body.detail; - } - if ( - body && - typeof body === "object" && - "message" in body && - typeof body.message === "string" - ) { - return body.message; + if (body && typeof body === "object") { + const detail = (body as { detail?: unknown }).detail; + const formatted = formatFastApiDetail(detail); + if (formatted) return formatted; + const message = (body as { message?: unknown }).message; + if (typeof message === "string" && message) return message; } return `Request failed (${status})`; } diff --git a/studio/frontend/src/features/export/api/export-api.ts b/studio/frontend/src/features/export/api/export-api.ts index 450691ddfd..56f8c8129b 100644 --- a/studio/frontend/src/features/export/api/export-api.ts +++ b/studio/frontend/src/features/export/api/export-api.ts @@ -2,15 +2,9 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { authFetch } from "@/features/auth"; +import { readFastApiError } from "@/lib/format-fastapi-error"; -async function readError(response: Response): Promise { - try { - const payload = (await response.json()) as { detail?: string; message?: string }; - return payload.detail || payload.message || `Request failed (${response.status})`; - } catch { - return `Request failed (${response.status})`; - } -} +const readError = (r: Response): Promise => readFastApiError(r); async function parseJson(response: Response): Promise { if (!response.ok) { diff --git a/studio/frontend/src/features/recipe-studio/api/index.ts b/studio/frontend/src/features/recipe-studio/api/index.ts index 273d4aea8d..06b4bc1f8b 100644 --- a/studio/frontend/src/features/recipe-studio/api/index.ts +++ b/studio/frontend/src/features/recipe-studio/api/index.ts @@ -2,6 +2,7 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { authFetch } from "@/features/auth"; +import { formatFastApiDetail, readFastApiError } from "@/lib/format-fastapi-error"; const DEFAULT_BASE = "/api/data-recipe"; @@ -202,12 +203,18 @@ async function parseErrorResponse(response: Response): Promise { } try { const parsed = JSON.parse(text) as { - detail?: string; + detail?: unknown; message?: string; // biome-ignore lint/style/useNamingConvention: api schema raw_detail?: string; }; - return parsed.detail ?? parsed.message ?? parsed.raw_detail ?? text; + // Use ||, not ??: an array detail is truthy but not nullish, and + // formatFastApiDetail returns null when it cannot flatten the value. + const formatted = formatFastApiDetail(parsed.detail); + if (formatted) return formatted; + if (typeof parsed.message === "string" && parsed.message) return parsed.message; + if (typeof parsed.raw_detail === "string" && parsed.raw_detail) return parsed.raw_detail; + return text; } catch { return text; } @@ -449,22 +456,17 @@ export async function uploadUnstructuredFile( ); if (res.status === 413) { - const detail = await res.json().catch(() => ({ detail: "File too large" })); return { file_id: "", filename: file.name, size_bytes: file.size, status: "error", - error: - typeof detail.detail === "string" ? detail.detail : "File too large", + error: await readFastApiError(res, "File too large"), }; } if (!res.ok) { - const detail = await res.json().catch(() => ({ detail: "Upload failed" })); - throw new Error( - typeof detail.detail === "string" ? detail.detail : "Upload failed", - ); + throw new Error(await readFastApiError(res, "Upload failed")); } return res.json(); diff --git a/studio/frontend/src/features/training/api/datasets-api.ts b/studio/frontend/src/features/training/api/datasets-api.ts index c56aec03be..0b4f90c56c 100644 --- a/studio/frontend/src/features/training/api/datasets-api.ts +++ b/studio/frontend/src/features/training/api/datasets-api.ts @@ -7,6 +7,7 @@ import type { UploadDatasetResponse, } from "../types/datasets"; import { authFetch } from "@/features/auth"; +import { readFastApiError } from "@/lib/format-fastapi-error"; type CheckDatasetFormatArgs = { datasetName: string; @@ -36,8 +37,7 @@ export async function checkDatasetFormat({ }); if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.detail || `Request failed (${res.status})`); + throw new Error(await readFastApiError(res)); } return res.json(); @@ -55,8 +55,7 @@ export async function uploadTrainingDataset( }); if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.detail || `Upload failed (${res.status})`); + throw new Error(await readFastApiError(res, "Upload failed")); } return res.json(); @@ -107,8 +106,7 @@ export async function aiAssistMapping({ }); if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.detail || `AI assist failed (${res.status})`); + throw new Error(await readFastApiError(res, "AI assist failed")); } return res.json(); @@ -117,8 +115,7 @@ export async function aiAssistMapping({ export async function listLocalDatasets(): Promise { const res = await authFetch("/api/datasets/local"); if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.detail || `Request failed (${res.status})`); + throw new Error(await readFastApiError(res)); } return res.json(); } diff --git a/studio/frontend/src/features/training/api/history-api.ts b/studio/frontend/src/features/training/api/history-api.ts index e886e35626..8fde83bc8d 100644 --- a/studio/frontend/src/features/training/api/history-api.ts +++ b/studio/frontend/src/features/training/api/history-api.ts @@ -2,6 +2,7 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { authFetch } from "@/features/auth"; +import { readFastApiError } from "@/lib/format-fastapi-error"; import type { TrainingRunDeleteResponse, TrainingRunDetailResponse, @@ -9,14 +10,7 @@ import type { TrainingRunSummary, } from "../types/history"; -async function readError(response: Response): Promise { - try { - const payload = (await response.json()) as { detail?: string; message?: string }; - return payload.detail || payload.message || `Request failed (${response.status})`; - } catch { - return `Request failed (${response.status})`; - } -} +const readError = (r: Response): Promise => readFastApiError(r); async function parseJson(response: Response): Promise { if (!response.ok) { diff --git a/studio/frontend/src/features/training/api/train-api.ts b/studio/frontend/src/features/training/api/train-api.ts index 781dbe6139..af3ea347c2 100644 --- a/studio/frontend/src/features/training/api/train-api.ts +++ b/studio/frontend/src/features/training/api/train-api.ts @@ -2,6 +2,7 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { authFetch } from "@/features/auth"; +import { readFastApiError } from "@/lib/format-fastapi-error"; import type { TrainingStartRequest, TrainingStartResponse, @@ -17,45 +18,7 @@ function isAbortError(error: unknown): boolean { return error instanceof DOMException && error.name === "AbortError"; } -type FastApiValidationError = { - loc?: unknown[]; - msg?: string; -}; - -function formatDetail(detail: unknown): string | null { - if (typeof detail === "string" && detail) return detail; - if (!Array.isArray(detail)) return null; - const parts = detail - .map((entry) => { - if (!entry || typeof entry !== "object") return ""; - const { loc, msg } = entry as FastApiValidationError; - const path = Array.isArray(loc) - ? loc.filter((segment) => segment !== "body").join(".") - : ""; - const message = typeof msg === "string" ? msg : ""; - if (path && message) return `${path}: ${message}`; - return path || message; - }) - .filter(Boolean); - return parts.length > 0 ? parts.join("; ") : null; -} - -async function readError(response: Response): Promise { - try { - const payload = (await response.json()) as { - detail?: unknown; - message?: string; - }; - const formattedDetail = formatDetail(payload.detail); - if (formattedDetail) return formattedDetail; - if (typeof payload.message === "string" && payload.message) { - return payload.message; - } - return `Request failed (${response.status})`; - } catch { - return `Request failed (${response.status})`; - } -} +const readError = (r: Response): Promise => readFastApiError(r); async function parseJson(response: Response): Promise { if (!response.ok) { diff --git a/studio/frontend/src/lib/format-fastapi-error.ts b/studio/frontend/src/lib/format-fastapi-error.ts new file mode 100644 index 0000000000..ed4a5ece7d --- /dev/null +++ b/studio/frontend/src/lib/format-fastapi-error.ts @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. + +/** + * Render a FastAPI error response body into a human-readable string. + * + * FastAPI emits 422s as `{detail: Array<{loc, msg, type, input?}>}`. The + * naive `body.detail || body.message` pattern truthy-coerces the array + * and stringifies it as `[object Object]`, which is unhelpful and was + * exactly the regression #5409 fixed inside `train-api.ts`. Lift the + * helper here so chat, export, history, datasets, and recipe-studio + * can all share it. + * + * Falls back through: array detail -> string detail -> message -> null. + */ + +export type FastApiValidationError = { + loc?: unknown[]; + msg?: string; +}; + +export function formatFastApiDetail(detail: unknown): string | null { + if (typeof detail === "string" && detail) return detail; + if (!Array.isArray(detail)) return null; + const parts = detail + .map((entry) => { + if (!entry || typeof entry !== "object") return ""; + const { loc, msg } = entry as FastApiValidationError; + const path = Array.isArray(loc) + ? loc.filter((segment) => segment !== "body").join(".") + : ""; + const message = typeof msg === "string" ? msg : ""; + if (path && message) return `${path}: ${message}`; + return path || message; + }) + .filter(Boolean); + return parts.length > 0 ? parts.join("; ") : null; +} + +/** + * Convert a Response (likely a non-ok response from a fetch) into the + * best human-readable error message available. Used by *-api.ts wrappers + * so toast text reads as `field: msg` instead of `[object Object]` or + * `Request failed (422)`. + */ +export async function readFastApiError( + response: Response, + fallbackPrefix: string = "Request failed", +): Promise { + try { + const payload = (await response.json()) as { + detail?: unknown; + message?: string; + }; + const formatted = formatFastApiDetail(payload.detail); + if (formatted) return formatted; + if (typeof payload.message === "string" && payload.message) { + return payload.message; + } + } catch { + // fall through + } + return `${fallbackPrefix} (${response.status})`; +}