From 39279aa34f150df043882c0bf0d74b759936e2d9 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 27 Oct 2026 21:22:07 -0700 Subject: [PATCH] feat(ui): shared error, connectivity and query foundation for disconnects (#15754) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The board reads API data and keeps the current task on screen. > - A restart or temporary network failure can interrupt those reads. > - Separate page error handlers can hide cached data or expose raw server codes. > - The app needs one connection state, readable error copy and a consistent read policy. > - This change provides that foundation and moves affected read surfaces onto it. > - The dependent write layer in #17960 preserves edits and submissions across the same failures. ## Linked Issues or Issue Description **What happened?** Temporary API failures could replace loaded content with errors. Different surfaces handled the same outage differently. Some showed raw server codes. A successful query to an unrelated service could also falsely report that Paperclip had recovered. **Expected behavior** Keep cached content visible. Show one connection banner during sustained trouble. Confirm recovery through the Paperclip health probe or its WebSocket. Retry transient reads and show readable errors for permanent failures. **Steps to reproduce** 1. Open a populated task or workspace page. 2. Interrupt API requests with a 503 or a dropped connection. 3. Keep an unrelated external request in flight. 4. Confirm that cached content remains and that the external result cannot clear the outage. 5. Restore the API and check that reads refresh. The write layer first merged into this feature branch in #15769 now has its own dependent PR, #17960. This PR contains the shared foundation and read changes. Merge this PR first. ## What Changed - Classify API errors and provide readable error copy, including non-JSON proxy responses and Retry-After handling. - Use one connection store and health probe with bounded backoff. Confirm recovery from the server health probe or live-updates WebSocket. - Pause reads while disconnected, retry transient errors, and refresh active reads after recovery. - Keep cached data visible through shared query views and affected page, panel and plugin read surfaces. - Add one app-level connection banner and support the pending-write count consumed by the dependent write layer. - Add a query-error rendering guard. Call it directly from the existing CI step. Update development documentation. ## Verification - Historical-chat lookup now follows the same ready/stale roster state as rendering. Three regressions fail before the fix; the six new cases pass after it, including working Retry, fresh data, no-cache error, permanent 403 and company isolation. All six focused query/recovery suites pass 200 tests. Final-commit CI passes and Apex is 5/5 with no unresolved findings. Canary Dry Run initially refused a preview version that was already on npm. It passed after one rerun with fresh registry metadata; no check was disabled. - Integrated current master and retained both transcript interest registration and connectivity reporting on socket open. The seven focused recovery/query suites pass 288 tests, and UI typecheck and production build pass. All 9,451 UI tests across 799 files pass on the merge commit. Final-commit CI passes and Apex is 5/5 with no unresolved findings. - Current head `70d9335d732219352ef0f98414439766bbcb33df`: all hosted CI checks pass. Apex is **5/5** on that head with no unresolved findings. The branch has no merge conflict. GitHub still requires code-owner approval. - The foundation passes 9,430 UI tests in 804 files. After integrating current master page paths, 190 focused page tests pass. - Two unrelated-success regressions fail before the latest fix. With the fix, 62 focused UI tests, all six guard tests and UI TypeScript pass. - The composed foundation and write layer pass **9,505 UI tests in 808 files**, UI TypeScript, a production UI build, and query-error, token and map guards. - A real Chromium journey on the composed source injects a 503 during title save. The loaded task stays visible; one reconnect banner appears; the edit saves after recovery; another task stays unchanged; the saved title survives reload. - Full recursive local checks were not repeated because Runner compilation exhausted local disk during another repair. Hosted checks must pass before merge. No new staging deployment is claimed. - Latest review follow-up: a cached CaseDetail now survives a failed HTTP 429 refresh. Its page regression fails before the fix and passes after it. Eighty focused foundation tests, UI TypeScript, build and guards pass after that change. - Earlier CI evidence is retained: several foundation jobs received a runner shutdown signal together. The write-layer head also failed an unchanged Runner descriptor close test with `provider_transport_failed`. The same Runner source passed that suite in the foundation run. New CI runs cover the updated heads; no check or assertion was weakened. ## Risks - Shared retry defaults change the time before transient read errors appear. Permanent errors still fail promptly. - ApiError.message now contains readable copy. Callers that compare raw codes must use ApiError.code. - Cache success can come from another service or local data. It must not confirm Paperclip recovery. - Durable writes are reviewed separately in #17960. This PR does not change database or mutation contracts. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`) produced the original work with tool use. OpenAI Codex, GPT-6 family, assisted with restoration, integration, tests and review fixes. The Codex session does not expose its exact serving model ID or context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: Paperclip Co-authored-by: Amy (Opus) --- .github/workflows/pr-trusted.yml | 7 + doc/DEVELOPING.md | 63 ++- maps/feature-map/agent-chat.md | 5 + scripts/check-query-error-rendering.mjs | 200 ++++++++++ scripts/check-query-error-rendering.test.mjs | 73 ++++ ui/src/App.tsx | 2 + ui/src/api/auth.ts | 28 +- ui/src/api/client.outage-simulator.test.ts | 65 ++++ ui/src/api/client.ts | 126 +++++- ui/src/api/errors.test.ts | 191 +++++++++ ui/src/api/errors.ts | 294 ++++++++++++++ ui/src/api/health.ts | 6 +- ui/src/api/response.test.ts | 8 + ui/src/api/response.ts | 26 +- .../components/AgentConversationsSidebar.tsx | 9 +- ui/src/components/CaseRevisionRail.tsx | 18 +- ui/src/components/CloudAccessGate.test.tsx | 90 +++-- ui/src/components/CloudAccessGate.tsx | 79 ++-- .../ConnectionStatusBanner.test.tsx | 177 +++++++++ ui/src/components/ConnectionStatusBanner.tsx | 136 +++++++ ui/src/components/FileViewerSheet.tsx | 15 +- .../IsolatedWorkspacesRouteGate.test.tsx | 15 + .../IsolatedWorkspacesRouteGate.tsx | 26 +- ui/src/components/IssueRelatedWorkPanel.tsx | 2 +- ui/src/components/IssueShareSheet.tsx | 28 +- .../IssueThreadInteractionCard.test.tsx | 2 +- ui/src/components/NewIssueDialog.tsx | 3 - ui/src/components/QueryView.test.tsx | 306 +++++++++++++++ ui/src/components/QueryView.tsx | 297 ++++++++++++++ ui/src/components/Sidebar.production.tsx | 3 - ui/src/components/Sidebar.tsx | 3 - ui/src/components/SidebarAgentChats.tsx | 9 +- ...refixedExecutionWorkspaceRedirect.test.tsx | 67 +++- .../UnprefixedExecutionWorkspaceRedirect.tsx | 63 ++- ui/src/components/WorkspaceFileBrowser.tsx | 38 +- ui/src/components/chat/AgentWorkPanels.tsx | 36 +- .../task-side-panel/TaskAttachmentPanel.tsx | 25 +- .../task-side-panel/TaskDocumentPanel.tsx | 11 +- .../task-side-panel/TaskSkillPanel.test.tsx | 35 +- .../task-side-panel/TaskSkillPanel.tsx | 20 +- ui/src/context/LiveUpdatesProvider.tsx | 7 + ui/src/hooks/useIssueExternalObjects.ts | 9 +- ui/src/index.css | 2 + ui/src/lib/connectivity.test.ts | 284 ++++++++++++++ ui/src/lib/connectivity.ts | 367 ++++++++++++++++++ .../lib/interaction-resolution-error.test.ts | 22 +- ui/src/lib/interaction-resolution-error.ts | 22 +- ui/src/lib/query-client.test.ts | 278 +++++++++++++ ui/src/lib/query-client.ts | 238 ++++++++++++ ui/src/main.tsx | 57 ++- ui/src/pages/AgentChat.test.tsx | 95 +++++ ui/src/pages/AgentChat.tsx | 27 +- ui/src/pages/AgentChats.test.tsx | 3 +- ui/src/pages/AgentChats.tsx | 13 +- ui/src/pages/AgentDetail.production.tsx | 20 +- ui/src/pages/AgentDetail.tsx | 30 +- ui/src/pages/CaseDetail.test.tsx | 13 + ui/src/pages/CaseDetail.tsx | 14 +- ui/src/pages/CompanySettingsPluginPage.tsx | 10 +- ui/src/pages/ExecutionWorkspaceDetail.tsx | 18 +- ui/src/pages/GoalDetail.tsx | 15 +- ui/src/pages/InviteLanding.tsx | 11 +- ui/src/pages/IssueDetail.tsx | 69 ++-- ui/src/pages/PluginManager.tsx | 33 +- ui/src/pages/ProjectDetail.tsx | 47 ++- ui/src/pages/ProjectWorkspaceDetail.tsx | 18 +- ui/src/pages/SkillStudio.tsx | 18 +- ui/src/pages/Workspaces.test.tsx | 25 ++ ui/src/pages/Workspaces.tsx | 26 +- ui/src/plugins/bridge.ts | 36 +- ui/src/plugins/launchers.tsx | 46 +-- ui/src/plugins/outlet-errors.test.tsx | 58 ++- ui/src/plugins/slots.tsx | 62 +-- ui/src/plugins/usePluginData.test.tsx | 123 ++++++ .../cloud-access-recovery.stories.tsx | 15 + 75 files changed, 4302 insertions(+), 436 deletions(-) create mode 100644 scripts/check-query-error-rendering.mjs create mode 100644 scripts/check-query-error-rendering.test.mjs create mode 100644 ui/src/api/client.outage-simulator.test.ts create mode 100644 ui/src/api/errors.test.ts create mode 100644 ui/src/api/errors.ts create mode 100644 ui/src/components/ConnectionStatusBanner.test.tsx create mode 100644 ui/src/components/ConnectionStatusBanner.tsx create mode 100644 ui/src/components/QueryView.test.tsx create mode 100644 ui/src/components/QueryView.tsx create mode 100644 ui/src/lib/connectivity.test.ts create mode 100644 ui/src/lib/connectivity.ts create mode 100644 ui/src/lib/query-client.test.ts create mode 100644 ui/src/lib/query-client.ts create mode 100644 ui/src/pages/AgentChat.test.tsx create mode 100644 ui/src/plugins/usePluginData.test.tsx diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index a04db993e7c..1d4c4ee908a 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -109,6 +109,13 @@ jobs: - name: Test feature module boundary check run: node "$CI_NODE_TEST" ./scripts/check-module-boundaries.test.mjs + # Report-only for now (always exits 0); --enforce turns findings into failures. + - name: Report raw query-error rendering + run: node scripts/check-query-error-rendering.mjs --summary + + - name: Test query-error rendering check + run: node --test ./scripts/check-query-error-rendering.test.mjs + - name: Test PR quality-gate scripts run: node "$CI_NODE_TEST" '.github/scripts/tests/*.test.mjs' diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 8621466701f..f828a2c85e0 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -563,16 +563,59 @@ owns committed updates. ## Hot-Restart Deploys -During a restart, the board's health, session, and access checks retry temporary -network/gateway failures and non-JSON API responses every five seconds. A new -page shows **Reconnecting to Paperclip** with a **Try again** action and waits -for startup health to become ready. Valid startup metadata remains available to -sign-in and invitation pages. An already -open page stays mounted during temporary background failures so unsaved edits -survive. Successful checks resume the same route and refresh other failed reads; -this recovery does not reload the browser or replay mutations. Authorization -failures still require sign-in or an explicit retry. Storybook **App / Connection -recovery** shows the startup recovery states. +During a restart, the UI's connectivity store (`ui/src/lib/connectivity.ts`) +confirms an outage with a `/api/health` probe, then polls health with backoff +(1s, 2s, 5s, 10s, then every 15s; a longer `Retry-After` wins). While the server +is unreachable, React Query pauses reads and `meta.replay = "idempotent"` +mutations instead of failing them; on recovery they resume and live queries +refresh once. Other mutations fail fast with readable copy and are not replayed. +Transient errors (network drops, non-JSON responses, 408/425/429/502/503/504, +and gateway codes such as `tenant_app_unavailable` in any status) are classified +in `ui/src/api/errors.ts`. + +A new page shows **Reconnecting to Paperclip** with a **Try again** action and +waits for startup health to become ready. Valid startup metadata remains +available to sign-in and invitation pages. An already open page stays mounted +during temporary background failures so unsaved edits survive; after about two +seconds of continuous trouble, one app-level **Connection interrupted** banner +appears, followed briefly by **Back online**. Recovery does not reload the +browser. Authorization failures still require sign-in or an explicit retry. +Storybook **App / Connection recovery** shows the startup screens and banner. + +To reproduce a deploy blip in a dev or QA build, set the outage simulator in the +browser console and remove it to end the outage: + +```js +localStorage.paperclipSimulateOutage = "503:tenant_app_unavailable" // until removed +localStorage.paperclipSimulateOutage = "503:tenant_app_unavailable@20" // for 20 seconds +localStorage.paperclipSimulateOutage = "network" // dropped connection +localStorage.removeItem("paperclipSimulateOutage") +``` + +Reads render through `useQueryView(query)` or `` +(`ui/src/components/QueryView.tsx`). The view state is one of `loading`, +`ready`, `stale` (data plus a transient failure: render the data, no red +text), `reconnecting` (no data yet while the app-wide connectivity state is +not `online`: a quiet placeholder that fills in by itself when the probe loop +recovers), or `error` (a real failure: `describeError` copy with a Retry +button). A transient failure with no data while the server is reachable (a +429, one plugin worker restarting, a 504 on one slow route) is also `error` +once the retry policy gives up, because no recovery loop would refetch it. +"Not found" renders only when `classifyError` says `not_found`, never for an +outage. `` is the error presentation on its own, in `page`, +`panel`, and `inline` sizes. Plugin chrome (`usePluginSlots`, +`usePluginLaunchers`) is the one exception: it never reports a transient +failure and stays collapsed instead. + +`node scripts/check-query-error-rendering.mjs --summary` reports UI code that renders raw query errors +(`{error.message}`, `isError ?`) or sets `retry: false`. It is report-only for +the long tail; pass `--enforce` to fail on every finding. Files listed in +`ENFORCED_FILES` in the script have already migrated, and a finding in one of +them fails the check in every mode. Add a file to that list when you migrate +it. For an intentional exception (a mutation result rendered inline, a +prop that is already outage-aware), end the line with `// query-error-ok: +`. Reads that must fail fast use `retry: retryTransientOnly(0)` +instead of `retry: false`. Primary-instance rebuilds that restart `paperclip.service` can request one-shot live-run adoption instead of using the normal graceful shutdown drain. Before restarting the service, write the marker from the newly staged app with the current service PID: diff --git a/maps/feature-map/agent-chat.md b/maps/feature-map/agent-chat.md index 467abd1867c..b4091f7e897 100644 --- a/maps/feature-map/agent-chat.md +++ b/maps/feature-map/agent-chat.md @@ -41,6 +41,11 @@ Preconditions: follow the [baseline](./README.md#before-driving-a-journey). Enab Automated: [chat landing](../../ui/src/pages/AgentChats.test.tsx) checks recent-chat routing; [conversation service](../../server/src/__tests__/agent-conversations.test.ts) covers persisted conversation behavior. +Automated: [historical chat](../../ui/src/pages/AgentChat.test.tsx) keeps an older +conversation reachable after a cached roster refresh ends in HTTP 429. It checks +lookup Retry and preserves permission and company boundaries. This is component +evidence with real query state, not live provider proof. + Automated: [thread and composer](../../ui/src/components/TaskChatThread.test.tsx) keeps an in-flight attachment through first-chat creation on desktop and mobile, while initial history for an existing conversation stays deferred. The diff --git a/scripts/check-query-error-rendering.mjs b/scripts/check-query-error-rendering.mjs new file mode 100644 index 00000000000..b8e9675c314 --- /dev/null +++ b/scripts/check-query-error-rendering.mjs @@ -0,0 +1,200 @@ +#!/usr/bin/env node +/** + * Disconnect-handling guard: finds UI code that renders raw query + * errors or opts out of the shared retry policy. + * + * During an outage the app shows one connection banner and keeps loaded data + * on screen. These patterns break that: + * + * raw-error-message {error.message}, {query.error?.message} + * cast-error-message {(error as Error).message} + * is-error-render isError ? … / isError && … (usually hides cached data) + * retry-false retry: false (skips transient retry; use the defaults + * in ui/src/lib/query-client.ts) + * + * Use `describeError` (ui/src/api/errors.ts) for copy and render cached data + * before errors. Intentional exceptions go in ALLOWLIST below as + * ":" with a reason. + * + * Report-only by default: prints findings and exits 0, except for findings in + * ENFORCED_FILES (surfaces already migrated), which always exit 1. `--enforce` + * exits 1 on any finding outside the allowlist (CI turns this on once the + * long tail migrates). + * + * Usage: node scripts/check-query-error-rendering.mjs [--enforce] [--summary] + */ +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join, relative, resolve, sep } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const defaultScanRoot = join(repoRoot, "ui/src"); + +export const RULES = [ + { + id: "raw-error-message", + description: "renders a raw error message; use describeError()", + pattern: /\{\s*[\w$.?!]*\b(?:error|err|e|[a-z]\w*Error)\s*[?!]?\.message\s*\}/, + }, + { + id: "cast-error-message", + description: "renders a cast error message; use describeError()", + pattern: /\{\s*\(\s*[\w$.?!]+\s+as\s+(?:Error|ApiError)\s*\)\s*\??\.message\s*\}/, + }, + { + id: "is-error-render", + description: "branches rendering on isError; render cached data first", + // `isError ?` / `isError &&`, not `isError?.x`, `isError ?? x`, or `isError?: boolean`. + pattern: /\bisError\s*(?:\?(?![.?:])|&&)/, + }, + { + id: "retry-false", + description: "disables retry; rely on the query-client defaults", + pattern: /\bretry\s*:\s*false\b/, + }, +]; + +/** + * Intentional exceptions, as ":". Each entry needs + * a reason. The list shrinks as surfaces migrate to the shared error handling. + */ +export const ALLOWLIST = new Set([ + // Empty for now: report-only mode lists every finding as the baseline. +]); + +/** + * Files already moved onto the shared read handling (`useQueryView` / + * `` in ui/src/components/QueryView.tsx). A finding in one of these + * fails the check even in report-only mode, so a migrated surface cannot + * quietly regress. Add a file here when you migrate it; mark an intentional + * exception on its line with `// query-error-ok: `. + */ +export const ENFORCED_FILES = new Set([ + "ui/src/components/AgentChatPicker.tsx", + "ui/src/components/AgentConversationSidebar.tsx", + "ui/src/components/AgentConversationsSidebar.tsx", + "ui/src/components/BreadcrumbBar.production.tsx", + "ui/src/components/BreadcrumbBar.tsx", + "ui/src/components/CaseRevisionRail.tsx", + "ui/src/components/chat/AgentWorkPanels.tsx", + "ui/src/components/FileViewerSheet.tsx", + "ui/src/components/IssueRelatedWorkPanel.tsx", + "ui/src/components/IssueShareSheet.tsx", + "ui/src/components/issue-properties/external-object-rows.tsx", + "ui/src/components/NewIssueDialog.tsx", + "ui/src/components/QueryView.tsx", + "ui/src/components/Sidebar.production.tsx", + "ui/src/components/Sidebar.tsx", + "ui/src/components/SidebarAgentChats.tsx", + "ui/src/components/task-side-panel/TaskAttachmentPanel.tsx", + "ui/src/components/task-side-panel/TaskDocumentPanel.tsx", + "ui/src/components/task-side-panel/TaskSkillPanel.tsx", + "ui/src/components/UnprefixedExecutionWorkspaceRedirect.tsx", + "ui/src/components/WorkspaceFileBrowser.tsx", + "ui/src/hooks/useIssueExternalObjects.ts", + "ui/src/lib/query-client.ts", + "ui/src/pages/AgentChat.tsx", + "ui/src/pages/AgentChats.tsx", + "ui/src/pages/AgentDetail.production.tsx", + "ui/src/pages/AgentDetail.tsx", + "ui/src/pages/CaseDetail.tsx", + "ui/src/pages/CompanySettingsPluginPage.tsx", + "ui/src/pages/ExecutionWorkspaceDetail.tsx", + "ui/src/pages/GoalDetail.tsx", + "ui/src/pages/IssueDetail.tsx", + "ui/src/pages/PluginManager.tsx", + "ui/src/pages/ProjectDetail.tsx", + "ui/src/pages/ProjectWorkspaceDetail.tsx", + "ui/src/pages/SkillStudio.tsx", + "ui/src/pages/Workspaces.tsx", + "ui/src/plugins/bridge.ts", + "ui/src/plugins/launchers.tsx", + "ui/src/plugins/slots.tsx", +]); + +function isCheckable(path) { + if (!/\.(ts|tsx)$/.test(path)) return false; + if (/\.(test|spec|stories)\.(ts|tsx)$/.test(path)) return false; + if (/\.d\.ts$/.test(path)) return false; + return true; +} + +function walk(dir, out) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === "node_modules" || entry.name.startsWith(".")) continue; + const full = join(dir, entry.name); + if (entry.isDirectory()) walk(full, out); + else if (isCheckable(full)) out.push(full); + } + return out; +} + +/** Findings for one file's source text. Lines ending in `// query-error-ok` are skipped. */ +export function scanSource(sourceText, path) { + const findings = []; + const lines = sourceText.split("\n"); + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; + if (/\/\/\s*query-error-ok\b/.test(line)) continue; + const trimmed = line.trim(); + if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) continue; + for (const rule of RULES) { + if (rule.pattern.test(line)) findings.push({ path, line: index + 1, rule: rule.id, text: trimmed }); + } + } + return findings; +} + +export function scanQueryErrorRendering({ + root = defaultScanRoot, + allowlist = ALLOWLIST, + enforcedFiles = ENFORCED_FILES, +} = {}) { + const findings = []; + for (const file of walk(root, [])) { + const path = relative(repoRoot, file).split(sep).join("/"); + for (const finding of scanSource(readFileSync(file, "utf8"), path)) { + if (allowlist.has(`${finding.path}:${finding.rule}`)) continue; + findings.push({ ...finding, enforced: enforcedFiles.has(finding.path) }); + } + } + return findings; +} + +function main() { + const args = new Set(process.argv.slice(2)); + const enforce = args.has("--enforce"); + const summaryOnly = args.has("--summary"); + const findings = scanQueryErrorRendering(); + const enforcedFindings = findings.filter((finding) => finding.enforced); + + if (!summaryOnly) { + for (const finding of findings) { + console.log(`${finding.path}:${finding.line} [${finding.rule}] ${finding.text}`); + } + } + const files = new Set(findings.map((finding) => finding.path)); + console.log(""); + console.log(`check-query-error-rendering: ${findings.length} finding(s) in ${files.size} file(s)`); + for (const rule of RULES) { + const count = findings.filter((finding) => finding.rule === rule.id).length; + console.log(` ${rule.id.padEnd(20)} ${String(count).padStart(5)} ${rule.description}`); + } + if (enforcedFindings.length > 0) { + // Always listed, even with --summary: these fail the check in every mode. + console.log(""); + console.log(`${enforcedFindings.length} finding(s) in migrated files (ENFORCED_FILES); these always fail:`); + for (const finding of enforcedFindings) { + console.log(` ${finding.path}:${finding.line} [${finding.rule}] ${finding.text}`); + } + console.log("Render cached data first and use describeError(), or mark the line with `// query-error-ok: `."); + process.exitCode = 1; + } + if (!enforce) { + console.log("Report-only mode: findings outside migrated files do not fail. Pass --enforce to fail on all findings."); + return; + } + if (findings.length > 0) process.exitCode = 1; +} + +if (resolve(process.argv[1] ?? "") === fileURLToPath(import.meta.url)) main(); diff --git a/scripts/check-query-error-rendering.test.mjs b/scripts/check-query-error-rendering.test.mjs new file mode 100644 index 00000000000..55573ef0933 --- /dev/null +++ b/scripts/check-query-error-rendering.test.mjs @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { scanQueryErrorRendering, scanSource } from "./check-query-error-rendering.mjs"; + +const rulesFor = (source) => scanSource(source, "ui/src/x.tsx").map((finding) => finding.rule); + +test("flags raw error renders", () => { + assert.deepEqual(rulesFor("

{error.message}

"), ["raw-error-message"]); + assert.deepEqual(rulesFor("

{query.error?.message}

"), ["raw-error-message"]); + assert.deepEqual(rulesFor("

{saveError.message}

"), ["raw-error-message"]); + assert.deepEqual(rulesFor("

{(error as Error).message}

"), ["cast-error-message"]); +}); + +test("flags isError render branches but not optional chaining or types", () => { + assert.deepEqual(rulesFor("{isError ? : }"), ["is-error-render"]); + assert.deepEqual(rulesFor("{query.isError && }"), ["is-error-render"]); + assert.deepEqual(rulesFor("isError?: boolean;"), []); + assert.deepEqual(rulesFor("const x = state.isError ?? false;"), []); +}); + +test("flags retry: false", () => { + assert.deepEqual(rulesFor("useQuery({ queryKey, queryFn, retry: false })"), ["retry-false"]); + assert.deepEqual(rulesFor("useQuery({ retry: shouldRetryRequest })"), []); +}); + +test("skips comments and explicitly accepted lines", () => { + assert.deepEqual(rulesFor("// {error.message} is what we used to render"), []); + assert.deepEqual(rulesFor(" retry: false, // query-error-ok: probe must fail fast"), []); +}); + +test("marks findings in migrated files as enforced", () => { + const root = mkdtempSync(join(tmpdir(), "query-error-rendering-")); + try { + mkdirSync(join(root, "pages")); + writeFileSync(join(root, "pages", "Migrated.tsx"), "export const A = () =>

{error.message}

;\n"); + writeFileSync(join(root, "pages", "Legacy.tsx"), "export const B = () =>

{error.message}

;\n"); + const findings = scanQueryErrorRendering({ root, allowlist: new Set(), enforcedFiles: new Set() }); + assert.equal(findings.length, 2); + assert.ok(findings.every((finding) => finding.enforced === false)); + const migrated = findings.find((finding) => finding.path.endsWith("Migrated.tsx")); + const enforced = scanQueryErrorRendering({ root, allowlist: new Set(), enforcedFiles: new Set([migrated.path]) }); + assert.deepEqual( + enforced.map((finding) => [finding.path.endsWith("Migrated.tsx"), finding.enforced]).sort(), + [[false, false], [true, true]], + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("scans a tree, skips tests, and honors the allowlist", () => { + const root = mkdtempSync(join(tmpdir(), "query-error-rendering-")); + try { + mkdirSync(join(root, "pages")); + writeFileSync(join(root, "pages", "Page.tsx"), "export const A = () =>

{error.message}

;\n"); + writeFileSync(join(root, "pages", "Page.test.tsx"), "expect(

{error.message}

);\n"); + const findings = scanQueryErrorRendering({ root, allowlist: new Set(), enforcedFiles: new Set() }); + assert.equal(findings.length, 1); + assert.equal(findings[0].rule, "raw-error-message"); + assert.equal(findings[0].line, 1); + const allowed = scanQueryErrorRendering({ + root, + allowlist: new Set([`${findings[0].path}:raw-error-message`]), + enforcedFiles: new Set(), + }); + assert.equal(allowed.length, 0); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 41684f9a9e3..9a05001cc35 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -18,6 +18,7 @@ import { import { useHiddenSettings } from "./hooks/useHiddenSettings"; import { OnboardingWizardVariant } from "./components/OnboardingWizardVariant"; import { CloudAccessGate } from "./components/CloudAccessGate"; +import { ConnectionStatusBanner } from "./components/ConnectionStatusBanner"; import { ShellConfigurationGate } from "./components/ShellConfigurationGate"; import { PaperclipLoading } from "./components/AnimatedPaperclipIcon"; import { AGENT_FILTER_TABS } from "./lib/agent-filter-tabs"; @@ -773,6 +774,7 @@ export function App() { return ( <> + }> } /> } /> diff --git a/ui/src/api/auth.ts b/ui/src/api/auth.ts index 202e59e8c1f..d7ecc1d9086 100644 --- a/ui/src/api/auth.ts +++ b/ui/src/api/auth.ts @@ -7,6 +7,8 @@ import { } from "@paperclipai/shared"; import { redactUrlSecrets } from "@/lib/redact-url-secrets"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; +import { apiFetch } from "./client"; +import { apiErrorMessage, isRawErrorCode } from "./errors"; import { readApiJson } from "./response"; type AuthErrorBody = @@ -45,6 +47,17 @@ function toSession(value: unknown): AuthSession | null { return nested.success ? nested.data : null; } +/** The server's own explanation on an auth error body, if it sent one. */ +export function authErrorServerMessage(payload: unknown): string | null { + const body = payload && typeof payload === "object" ? payload as Exclude : null; + const nested = body?.error && typeof body.error === "object" ? body.error : null; + const candidates = [nested?.message, body?.message, typeof body?.error === "string" ? body.error : undefined]; + for (const candidate of candidates) { + if (typeof candidate === "string" && candidate.trim().length > 0) return candidate; + } + return null; +} + function extractAuthError(payload: AuthErrorBody, status: number) { const nested = payload?.error && typeof payload.error === "object" @@ -56,16 +69,11 @@ function extractAuthError(payload: AuthErrorBody, status: number) { : typeof payload?.code === "string" ? payload.code : null; - const message = - typeof nested?.message === "string" && nested.message.trim().length > 0 - ? nested.message - : typeof payload?.message === "string" && payload.message.trim().length > 0 - ? payload.message - : typeof payload?.error === "string" && payload.error.trim().length > 0 - ? payload.error - : `Request failed: ${status}`; + const message = authErrorServerMessage(payload); - return new AuthApiError(message, status, payload, code); + // Never surface a raw gateway code ("tenant_app_unavailable") as the message. + const readable = message && !isRawErrorCode(message) ? message : apiErrorMessage(status, payload); + return new AuthApiError(readable, status, payload, code); } // Rich diagnostics for auth requests. Network-layer failures (Safari @@ -153,7 +161,7 @@ async function authPatch(path: string, body: Record, parse: export const authApi = { getSession: async (): Promise => { - const res = await fetch("/api/auth/get-session", { + const res = await apiFetch("/api/auth/get-session", { credentials: "include", headers: { Accept: "application/json" }, cache: "no-store", diff --git a/ui/src/api/client.outage-simulator.test.ts b/ui/src/api/client.outage-simulator.test.ts new file mode 100644 index 00000000000..1ee6dcda8e8 --- /dev/null +++ b/ui/src/api/client.outage-simulator.test.ts @@ -0,0 +1,65 @@ +// @vitest-environment jsdom + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { activeSimulatedOutage, api, OUTAGE_SIMULATOR_STORAGE_KEY, parseSimulatedOutage } from "./client"; +import { classifyError, describeError } from "./errors"; +import { healthApi } from "./health"; + +describe("dev outage simulator", () => { + const fetchMock = vi.fn(async () => Response.json({ status: "ok" })); + + beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + window.localStorage.removeItem(OUTAGE_SIMULATOR_STORAGE_KEY); + vi.useRealTimers(); + vi.unstubAllGlobals(); + fetchMock.mockClear(); + }); + + it("parses the supported shapes", () => { + expect(parseSimulatedOutage("503:tenant_app_unavailable")).toEqual({ + kind: "status", status: 503, code: "tenant_app_unavailable", durationMs: null, + }); + expect(parseSimulatedOutage("502@20")).toEqual({ kind: "status", status: 502, code: null, durationMs: 20_000 }); + expect(parseSimulatedOutage("network")).toEqual({ kind: "network", durationMs: null }); + expect(parseSimulatedOutage("200")).toBeNull(); + expect(parseSimulatedOutage("yes please")).toBeNull(); + expect(parseSimulatedOutage(null)).toBeNull(); + }); + + it("fails API requests with the chosen gateway code without calling the server", async () => { + window.localStorage.setItem(OUTAGE_SIMULATOR_STORAGE_KEY, "503:tenant_app_unavailable"); + const error = await api.get("/companies").catch((caught: unknown) => caught); + expect(error).toMatchObject({ name: "ApiError", status: 503, code: "tenant_app_unavailable" }); + expect(classifyError(error)).toBe("transient"); + expect((error as Error).message).not.toMatch(/tenant_app_unavailable/); + expect(describeError(error).body).not.toMatch(/tenant_app_unavailable/); + await expect(healthApi.get()).rejects.toMatchObject({ status: 503 }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("simulates a dropped connection", async () => { + window.localStorage.setItem(OUTAGE_SIMULATOR_STORAGE_KEY, "network"); + const error = await api.get("/companies").catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(TypeError); + expect(classifyError(error)).toBe("transient"); + }); + + it("ends a timed outage on its own", async () => { + vi.useFakeTimers(); + window.localStorage.setItem(OUTAGE_SIMULATOR_STORAGE_KEY, "503:tenant_app_unavailable@20"); + expect(activeSimulatedOutage()).not.toBeNull(); + vi.advanceTimersByTime(20_000); + expect(activeSimulatedOutage()).toBeNull(); + expect(window.localStorage.getItem(OUTAGE_SIMULATOR_STORAGE_KEY)).toBeNull(); + await expect(api.get("/companies")).resolves.toEqual({ status: "ok" }); + }); + + it("is a no-op when the key is absent", async () => { + await expect(api.get("/companies")).resolves.toEqual({ status: "ok" }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index 0476b2b51bc..ae5cabd9450 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -1,21 +1,137 @@ import { getPageVisibility, getVisibilityHeaderValue } from "@/lib/page-visibility"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; +import { apiErrorMessage, parseRetryAfter } from "./errors"; import { readApiJson } from "./response"; const BASE = "/api"; +export interface ApiErrorOptions { + /** Raw server error code; defaults to `body.error` when that is a string. */ + code?: string | null; + /** Server-requested delay from the `Retry-After` header, in milliseconds. */ + retryAfterMs?: number | null; +} + export class ApiError extends Error { status: number; body: unknown; + /** + * The raw `body.error` string (for example `tenant_app_unavailable`). Match + * on this, never on `message`: `message` is readable copy for people and may + * differ from the code (see `apiErrorMessage` in `api/errors.ts`). + */ + code: string | null; + retryAfterMs: number | null; - constructor(message: string, status: number, body: unknown) { + constructor(message: string, status: number, body: unknown, options: ApiErrorOptions = {}) { super(message); this.name = "ApiError"; this.status = status; this.body = body; + const rawError = body && typeof body === "object" ? (body as { error?: unknown }).error : undefined; + this.code = options.code !== undefined ? options.code : typeof rawError === "string" ? rawError : null; + this.retryAfterMs = options.retryAfterMs ?? null; } } +/** Build the `ApiError` for a non-OK response whose body has been read. */ +export function apiErrorFromResponse(res: Response, body: unknown): ApiError { + return new ApiError(apiErrorMessage(res.status, body), res.status, body, { + retryAfterMs: parseRetryAfter(res.headers?.get("Retry-After") ?? null), + }); +} + +// --- Dev/QA outage simulator ---------------------------------------------------- +// +// Reproduces a deploy blip without a deploy. Dev and QA builds only: +// +// localStorage.paperclipSimulateOutage = "503:tenant_app_unavailable" // until removed +// localStorage.paperclipSimulateOutage = "503:tenant_app_unavailable@20" // for 20 seconds +// localStorage.paperclipSimulateOutage = "502" // non-JSON proxy page +// localStorage.paperclipSimulateOutage = "network" // fetch rejects (connection drop) +// +// Every same-origin API request made through `apiFetch` (the client, health, +// session) fails with the chosen shape. Remove the key to end the outage. + +export const OUTAGE_SIMULATOR_STORAGE_KEY = "paperclipSimulateOutage"; + +export type SimulatedOutage = + | { kind: "network"; durationMs: number | null } + | { kind: "status"; status: number; code: string | null; durationMs: number | null }; + +export function parseSimulatedOutage(raw: string | null | undefined): SimulatedOutage | null { + const match = /^\s*(network|(\d{3})(?::([A-Za-z0-9_.-]+))?)(?:@(\d+(?:\.\d+)?))?\s*$/.exec(raw ?? ""); + if (!match) return null; + const durationMs = match[4] ? Math.round(Number(match[4]) * 1000) : null; + if (match[1] === "network") return { kind: "network", durationMs }; + const status = Number(match[2]); + if (status < 400 || status > 599) return null; + return { kind: "status", status, code: match[3] ?? null, durationMs }; +} + +function outageSimulatorEnabled(): boolean { + return import.meta.env.DEV || import.meta.env.MODE === "qa"; +} + +let simulatedOutageWindow: { raw: string; endsAt: number | null } | null = null; + +/** The outage to simulate for the next request, or null. Never throws. */ +export function activeSimulatedOutage(now = Date.now()): SimulatedOutage | null { + if (!outageSimulatorEnabled() || typeof window === "undefined") return null; + let raw: string | null = null; + try { + raw = window.localStorage?.getItem(OUTAGE_SIMULATOR_STORAGE_KEY) ?? null; + } catch { + return null; + } + const outage = parseSimulatedOutage(raw); + if (!raw || !outage) { + simulatedOutageWindow = null; + return null; + } + if (simulatedOutageWindow?.raw !== raw) { + simulatedOutageWindow = { raw, endsAt: outage.durationMs === null ? null : now + outage.durationMs }; + } + if (simulatedOutageWindow.endsAt !== null && now >= simulatedOutageWindow.endsAt) { + simulatedOutageWindow = null; + try { + window.localStorage.removeItem(OUTAGE_SIMULATOR_STORAGE_KEY); + } catch { + // Storage can be unavailable; the outage still ends for this tab. + } + return null; + } + return outage; +} + +function simulatedOutageResponse(outage: Extract): Response { + if (!outage.code) { + // What a proxy serves while the app is down: an HTML page, not JSON. + return new Response("Unavailable", { + status: outage.status, + headers: { "Content-Type": "text/html" }, + }); + } + return new Response(JSON.stringify({ error: outage.code }), { + status: outage.status, + headers: { "Content-Type": "application/json" }, + }); +} + +/** + * `fetch` for same-origin API calls. Identical to `fetch` except that the + * dev/QA outage simulator can intercept it. + */ +export async function apiFetch(input: string, init?: RequestInit): Promise { + const outage = activeSimulatedOutage(); + if (outage) { + if (init?.signal?.aborted) throw abortError(); + if (outage.kind === "network") throw new TypeError("Failed to fetch"); + return simulatedOutageResponse(outage); + } + return fetch(input, init); +} + export interface RequestOptions { /** Abort signal wired through to `fetch` and coalescing (per-caller). */ signal?: AbortSignal; @@ -51,7 +167,7 @@ export async function requestResponse(path: string, init?: RequestInit): Promise } applyObservabilityHeaders(headers); - const res = await fetch(`${BASE}${path}`, { + const res = await apiFetch(`${BASE}${path}`, { credentials: "include", ...init, headers, @@ -60,11 +176,7 @@ export async function requestResponse(path: string, init?: RequestInit): Promise const errorBody = await readApiJson(res); const recovery = tenantSessionRecovery.recoverIfNeeded(res.status, errorBody); if (recovery) return recovery; - throw new ApiError( - (errorBody as { error?: string } | null)?.error ?? `Request failed: ${res.status}`, - res.status, - errorBody, - ); + throw apiErrorFromResponse(res, errorBody); } return res; } diff --git a/ui/src/api/errors.test.ts b/ui/src/api/errors.test.ts new file mode 100644 index 00000000000..c1fc62b6cc8 --- /dev/null +++ b/ui/src/api/errors.test.ts @@ -0,0 +1,191 @@ +import { describe, expect, it } from "vitest"; +import { ApiError } from "./client"; +import { + apiErrorMessage, + classifyError, + describeError, + errorCodes, + isConnectivityError, + isRawErrorCode, + parseRetryAfter, + type ErrorKind, +} from "./errors"; +import { ApiUnavailableError } from "./response"; + +function apiError(status: number, body: unknown = null) { + return new ApiError(apiErrorMessage(status, body), status, body); +} + +describe("classifyError", () => { + it.each<[string, unknown, ErrorKind]>([ + ["a browser network failure", new TypeError("Failed to fetch"), "transient"], + ["a Safari network failure", new TypeError("Load failed"), "transient"], + ["a proxy HTML page", new ApiUnavailableError(502), "transient"], + ["408", apiError(408), "transient"], + ["425", apiError(425), "transient"], + ["429", apiError(429, { error: "Too many requests" }), "transient"], + ["502", apiError(502), "transient"], + ["503", apiError(503, { error: "Service unavailable" }), "transient"], + ["504", apiError(504), "transient"], + ["401", apiError(401, { error: "Unauthorized" }), "auth"], + ["403", apiError(403, { error: "Forbidden" }), "forbidden"], + ["404", apiError(404, { error: "Issue not found" }), "not_found"], + ["410", apiError(410), "not_found"], + ["409", apiError(409, { error: "Document is locked" }), "conflict"], + ["412", apiError(412), "conflict"], + ["400", apiError(400, { error: "Title is required" }), "invalid"], + ["422", apiError(422), "invalid"], + ["500", apiError(500, { error: "boom" }), "unknown"], + ["an abort", new DOMException("Aborted", "AbortError"), "aborted"], + ["a programming TypeError", new TypeError("Cannot read properties of undefined"), "unknown"], + ["a plain Error", new Error("nope"), "unknown"], + ["undefined", undefined, "unknown"], + ])("classifies %s", (_label, error, kind) => { + expect(classifyError(error)).toBe(kind); + }); + + it.each([200, 400, 403, 404, 409, 500])( + "treats the gateway's tenant_app_unavailable as transient in any status (%s)", + (status) => { + expect(classifyError(apiError(status, { error: "tenant_app_unavailable" }))).toBe("transient"); + expect(classifyError(apiError(status, { error: "tenant_app_starting" }))).toBe("transient"); + }, + ); + + it.each(["WORKER_UNAVAILABLE", "TIMEOUT"])("treats plugin bridge %s as transient", (code) => { + expect(classifyError(apiError(500, { code, message: "Plugin worker is restarting" }))).toBe("transient"); + }); + + it("reads a code from details, as HttpErrors serialize it", () => { + const error = apiError(409, { error: "Locked", details: { code: "tenant_app_starting" } }); + expect(classifyError(error)).toBe("transient"); + }); + + it("classifies structurally, without the ApiError class", () => { + expect(classifyError({ status: 503, body: null })).toBe("transient"); + expect(classifyError({ name: "AuthApiError", status: 401, code: null })).toBe("auth"); + }); +}); + +describe("isConnectivityError", () => { + it("is true for server-wide outages only", () => { + expect(isConnectivityError(new TypeError("Failed to fetch"))).toBe(true); + expect(isConnectivityError(new ApiUnavailableError(200))).toBe(true); + expect(isConnectivityError(apiError(503))).toBe(true); + expect(isConnectivityError(apiError(404, { error: "tenant_app_unavailable" }))).toBe(true); + // Transient, but one route: left to query retry, not the app banner. + expect(isConnectivityError(apiError(429))).toBe(false); + expect(isConnectivityError(apiError(503, { code: "WORKER_UNAVAILABLE", message: "x" }))).toBe(false); + expect(isConnectivityError(apiError(500))).toBe(false); + expect(isConnectivityError(apiError(403))).toBe(false); + }); +}); + +describe("describeError", () => { + const rawCode = /\b[a-z]+(?:_[a-z]+)+\b|\b[A-Z]+_[A-Z]+\b/; + + it("never returns a raw code", () => { + const errors = [ + apiError(503, { error: "tenant_app_unavailable" }), + apiError(200, { error: "tenant_app_starting" }), + apiError(404, { error: "issue_not_found" }), + apiError(500, { error: "internal_error" }), + apiError(503, { code: "WORKER_UNAVAILABLE", message: "WORKER_UNAVAILABLE" }), + new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }), + ]; + for (const error of errors) { + const { title, body } = describeError(error); + expect(title).not.toMatch(rawCode); + expect(body).not.toMatch(rawCode); + } + }); + + it("uses the shared transient copy for outages", () => { + expect(describeError(new ApiUnavailableError(502))).toEqual({ + title: "Connection interrupted", + body: "Paperclip is temporarily unavailable. Please try again in a moment.", + retryable: true, + }); + expect(describeError(apiError(503, { error: "tenant_app_unavailable" }))).toMatchObject({ + title: "Reconnecting to Paperclip", + retryable: true, + }); + }); + + it("keeps a readable server message for client errors", () => { + expect(describeError(apiError(400, { error: "Title is required" }))).toEqual({ + title: "Check your input", + body: "Title is required", + retryable: false, + }); + }); + + it("falls back to the kind's copy when the server sent a code or nothing", () => { + expect(describeError(apiError(404, { error: "issue_not_found" })).body).toBe("This item doesn’t exist or was moved."); + expect(describeError(apiError(403, null))).toMatchObject({ title: "Not allowed", retryable: false }); + expect(describeError(new Error("Something odd"))).toMatchObject({ body: "Something odd", retryable: true }); + expect(describeError(undefined)).toMatchObject({ title: "Something went wrong", retryable: true }); + }); + + it("names the action in the title for real failures, not outages", () => { + expect(describeError(apiError(409, { error: "Document is locked" }), { action: "save the document" })).toEqual({ + title: "Couldn't save the document", + body: "Document is locked", + retryable: false, + }); + expect(describeError(apiError(503), { action: "save the document" }).title).toBe("Connection interrupted"); + }); +}); + +describe("ApiError", () => { + it("keeps the raw body.error as code and a readable message", () => { + const error = apiError(503, { error: "tenant_app_unavailable" }); + expect(error.code).toBe("tenant_app_unavailable"); + expect(error.message).toBe("Paperclip is restarting or updating. Please try again in a moment."); + }); + + it("passes a readable server message through unchanged", () => { + const error = apiError(409, { error: "Document is locked" }); + expect(error.code).toBe("Document is locked"); + expect(error.message).toBe("Document is locked"); + }); + + it("replaces the generic status line with readable copy", () => { + expect(apiError(404).message).toBe("This item doesn’t exist or was moved."); + expect(apiError(404).code).toBeNull(); + }); + + it("exposes every code for matching", () => { + expect(errorCodes(apiError(403, { error: "Denied", code: "interaction_human_only", details: { code: "x_y" } }))) + .toEqual(["Denied", "interaction_human_only", "x_y"]); + }); +}); + +describe("isRawErrorCode", () => { + it.each(["tenant_app_unavailable", "WORKER_UNAVAILABLE", "TIMEOUT", "issue.not_found"])("%s is a code", (value) => { + expect(isRawErrorCode(value)).toBe(true); + }); + + it.each(["Forbidden", "Unauthorized", "Document is locked", "Request failed: 503", ""])("%s is not a code", (value) => { + expect(isRawErrorCode(value)).toBe(false); + }); +}); + +describe("parseRetryAfter", () => { + it("parses delta-seconds", () => { + expect(parseRetryAfter("5")).toBe(5_000); + expect(parseRetryAfter(" 1.5 ")).toBe(1_500); + }); + + it("parses an HTTP date relative to now", () => { + const now = Date.parse("2026-10-09T12:00:00Z"); + expect(parseRetryAfter("Fri, 09 Oct 2026 12:00:30 GMT", now)).toBe(30_000); + expect(parseRetryAfter("Fri, 09 Oct 2026 11:00:00 GMT", now)).toBe(0); + }); + + it("caps absurd values and ignores garbage", () => { + expect(parseRetryAfter("86400")).toBe(5 * 60_000); + expect(parseRetryAfter("soon")).toBeNull(); + expect(parseRetryAfter(null)).toBeNull(); + }); +}); diff --git a/ui/src/api/errors.ts b/ui/src/api/errors.ts new file mode 100644 index 00000000000..1c94961d285 --- /dev/null +++ b/ui/src/api/errors.ts @@ -0,0 +1,294 @@ +/** + * Shared error classification and user-facing copy. + * + * Every surface that reacts to a failed request asks two questions: "is this a + * blip that will fix itself?" and "what do I tell the user?". This module is + * the single answer to both, so retry policy, the connectivity banner, toasts, + * and inline errors agree with each other. + * + * Classification is structural on purpose: it reads `status`, `code`, `body`, + * and `name` instead of importing `ApiError`/`ApiUnavailableError`, so the API + * client can depend on this module without an import cycle, and errors thrown + * by hand-rolled fetches (health, uploads) classify the same way. + */ + +export type ErrorKind = + | "transient" + | "auth" + | "forbidden" + | "not_found" + | "conflict" + | "invalid" + | "aborted" + | "unknown"; + +export interface ErrorDescription { + title: string; + body: string; + /** True when trying the same request again later can succeed. */ + retryable: boolean; +} + +export interface DescribeErrorOptions { + /** What the user was doing, as a verb phrase: "save the comment". */ + action?: string; +} + +/** HTTP statuses that mean "the server or a proxy will likely recover". */ +const TRANSIENT_STATUSES = new Set([408, 425, 429, 502, 503, 504]); + +/** + * Error codes that mean "temporarily unavailable" regardless of the HTTP status + * they arrive with. The Cloud gateway has sent `tenant_app_unavailable` with + * non-5xx statuses, so these are matched by code, never by status alone. + */ +const TRANSIENT_CODES = new Set([ + "tenant_app_unavailable", + "tenant_app_starting", + // Plugin bridge (`PluginBridgeErrorCode`). + "WORKER_UNAVAILABLE", + "TIMEOUT", +]); + +/** Transient errors that say "the whole server is unreachable", not one route. */ +const CONNECTIVITY_STATUSES = new Set([502, 503, 504]); +const CONNECTIVITY_CODES = new Set(["tenant_app_unavailable", "tenant_app_starting"]); + +// --- Copy table --------------------------------------------------------------- +// +// The only place user-facing error copy lives. Keys are server error codes (the +// raw `body.error`/`body.code`) or an `ErrorKind` fallback. Never return a raw +// code to the user: anything not in this table falls back to its kind's copy. + +const UNAVAILABLE_BODY = "Paperclip is temporarily unavailable. Please try again in a moment."; + +const CODE_COPY: Record = { + tenant_app_unavailable: { + title: "Reconnecting to Paperclip", + body: "Paperclip is restarting or updating. Please try again in a moment.", + }, + tenant_app_starting: { + title: "Paperclip is starting", + body: "Paperclip is starting up. Please try again in a few seconds.", + }, + WORKER_UNAVAILABLE: { + title: "Plugin unavailable", + body: "This plugin is restarting. Please try again in a moment.", + }, + TIMEOUT: { + title: "Request timed out", + body: "This took too long to respond. Please try again in a moment.", + }, + // Issue-thread interaction resolution (moved from lib/interaction-resolution-error.ts). + interaction_audience_denied: { + title: "You can’t respond to this card", + body: "You are not in this card's resolver audience.", + }, + interaction_forbidden: { + title: "You can’t respond to this card", + body: "You do not have permission to respond to this card.", + }, + interaction_settled: { + title: "Already handled", + body: "This request is no longer waiting for a decision.", + }, + interaction_submit_failed: { + title: "Couldn't submit", + body: "Couldn't submit. Try again.", + }, +}; + +const KIND_COPY: Record = { + // Moved from `ApiUnavailableError` (api/response.ts). + transient: { title: "Connection interrupted", body: UNAVAILABLE_BODY }, + auth: { title: "Signed out", body: "Your session has ended. Sign in again to continue." }, + forbidden: { title: "Not allowed", body: "You don’t have permission to do that." }, + not_found: { title: "Not found", body: "This item doesn’t exist or was moved." }, + conflict: { title: "Out of date", body: "This changed since you loaded it. Refresh and try again." }, + invalid: { title: "Check your input", body: "Some details aren’t valid. Review them and try again." }, + aborted: { title: "Cancelled", body: "The request was cancelled." }, + unknown: { title: "Something went wrong", body: "An unexpected error occurred. Please try again." }, +}; + +/** Copy for a specific code or kind, for callers that build their own messages. */ +export function errorCopy(key: string): { title: string; body: string } { + return CODE_COPY[key] ?? KIND_COPY[key as ErrorKind] ?? KIND_COPY.unknown; +} + +// --- Field extraction ------------------------------------------------------------- + +function record(value: unknown): Record | null { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : null; +} + +function stringField(value: unknown): string | null { + return typeof value === "string" && value.trim() ? value.trim() : null; +} + +/** The HTTP status an error carries, if any. */ +export function errorStatus(err: unknown): number | null { + const status = record(err)?.status; + return typeof status === "number" && Number.isFinite(status) ? status : null; +} + +/** + * Every server-provided code on an error: `ApiError.code` (the raw + * `body.error`), `body.code`, and `body.details.code`. Plugin bridge errors put + * their code on `body.code`; HttpErrors put a machine code on `details.code`. + */ +export function errorCodes(err: unknown): string[] { + const error = record(err); + if (!error) return []; + const body = record(error.body); + const codes = [ + stringField(error.code), + stringField(body?.error), + stringField(body?.code), + stringField(record(body?.details)?.code), + ]; + return [...new Set(codes.filter((code): code is string => code !== null))]; +} + +/** + * True for a machine code (`tenant_app_unavailable`, `WORKER_UNAVAILABLE`, + * `TIMEOUT`) rather than a sentence a person wrote. Single readable words such + * as "Forbidden" are not codes. + */ +export function isRawErrorCode(value: string): boolean { + const text = value.trim(); + if (!text || /\s/.test(text)) return false; + return /^[A-Za-z0-9]+(?:[_.][A-Za-z0-9]+)+$/.test(text) || /^[A-Z][A-Z0-9_]{2,}$/.test(text); +} + +/** A server message that is safe to show: not a raw code, not a generic status line. */ +function readableServerMessage(err: unknown): string | null { + const error = record(err); + const candidates = [stringField(record(error?.body)?.message), stringField(record(error?.body)?.error)]; + if (err instanceof Error) candidates.push(stringField(err.message)); + for (const candidate of candidates) { + if (!candidate) continue; + if (isRawErrorCode(candidate)) continue; + if (/^Request failed: \d+$/.test(candidate)) continue; + if (candidate === UNAVAILABLE_BODY) continue; + return candidate; + } + return null; +} + +// --- Classification --------------------------------------------------------------- + +function isAbortError(err: unknown): boolean { + const name = record(err)?.name; + return name === "AbortError" || name === "CancelledError"; +} + +function isNetworkTypeError(err: unknown): boolean { + // Fetch uses different network-failure messages across browsers. Do not + // classify arbitrary TypeErrors (programming bugs) as outages. + return err instanceof TypeError && /fetch|network|load failed/i.test(err.message); +} + +function isApiUnavailableError(err: unknown): boolean { + return record(err)?.name === "ApiUnavailableError"; +} + +export function classifyError(err: unknown): ErrorKind { + if (err === null || err === undefined) return "unknown"; + if (isAbortError(err)) return "aborted"; + if (isApiUnavailableError(err) || isNetworkTypeError(err)) return "transient"; + if (errorCodes(err).some((code) => TRANSIENT_CODES.has(code))) return "transient"; + + const status = errorStatus(err); + if (status === null) return "unknown"; + if (TRANSIENT_STATUSES.has(status)) return "transient"; + if (status === 401) return "auth"; + if (status === 403) return "forbidden"; + if (status === 404 || status === 410) return "not_found"; + if (status === 409 || status === 412) return "conflict"; + if (status >= 400 && status < 500) return "invalid"; + return "unknown"; +} + +export function isTransientError(err: unknown): boolean { + return classifyError(err) === "transient"; +} + +/** + * A transient error that means the server as a whole is unreachable (network + * drop, proxy 5xx, gateway "app unavailable"), as opposed to one route being + * busy (429, a plugin worker restarting). Only these move the app-wide + * connectivity state; per-route blips are left to query retry. + */ +export function isConnectivityError(err: unknown): boolean { + if (isApiUnavailableError(err) || isNetworkTypeError(err)) return true; + if (errorCodes(err).some((code) => CONNECTIVITY_CODES.has(code))) return true; + const status = errorStatus(err); + if (status === null || !CONNECTIVITY_STATUSES.has(status)) return false; + // A plugin worker 503 is one route, not the server. + return !errorCodes(err).some((code) => TRANSIENT_CODES.has(code) && !CONNECTIVITY_CODES.has(code)); +} + +/** HTTP 4xx other than the transient ones; retrying the same request cannot help. */ +export function isClientError(err: unknown): boolean { + const kind = classifyError(err); + return kind === "auth" || kind === "forbidden" || kind === "not_found" || kind === "conflict" || kind === "invalid"; +} + +// --- Retry-After -------------------------------------------------------------------- + +const MAX_RETRY_AFTER_MS = 5 * 60_000; + +/** Parse an HTTP `Retry-After` value (delta-seconds or HTTP-date) into milliseconds. */ +export function parseRetryAfter(value: string | null | undefined, now = Date.now()): number | null { + if (!value) return null; + const text = value.trim(); + if (/^\d+(?:\.\d+)?$/.test(text)) { + return Math.min(Math.round(Number(text) * 1000), MAX_RETRY_AFTER_MS); + } + const date = Date.parse(text); + if (Number.isNaN(date)) return null; + return Math.min(Math.max(0, date - now), MAX_RETRY_AFTER_MS); +} + +/** The server-requested delay carried by an error, if any. */ +export function errorRetryAfterMs(err: unknown): number | null { + const value = record(err)?.retryAfterMs; + return typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : null; +} + +// --- Copy ----------------------------------------------------------------------------- + +/** + * User-facing copy for any error. Never returns a raw `snake_case` code: + * known codes use the table above, readable server messages pass through for + * client errors, and everything else uses the copy for its kind. + */ +export function describeError(err: unknown, options: DescribeErrorOptions = {}): ErrorDescription { + const kind = classifyError(err); + const retryable = kind === "transient" || kind === "unknown"; + const coded = errorCodes(err).map((code) => CODE_COPY[code]).find(Boolean); + const fallback = KIND_COPY[kind]; + + let body: string; + if (coded) body = coded.body; + else if (kind === "transient" || kind === "aborted") body = fallback.body; + else body = readableServerMessage(err) ?? fallback.body; + + const title = options.action && kind !== "transient" + ? `Couldn't ${options.action}` + : (coded?.title ?? fallback.title); + return { title, body, retryable }; +} + +/** + * The `message` for an `ApiError` built from an HTTP response: the server's own + * text when it is readable, otherwise copy from the table. Keeps raw codes out + * of every `{error.message}` render and `toast(err.message)` call. + */ +export function apiErrorMessage(status: number, body: unknown): string { + const raw = stringField(record(body)?.error); + if (raw && !isRawErrorCode(raw)) return raw; + return describeError({ status, body, code: raw }).body; +} diff --git a/ui/src/api/health.ts b/ui/src/api/health.ts index 2612f019e0a..ee38beb85fd 100644 --- a/ui/src/api/health.ts +++ b/ui/src/api/health.ts @@ -1,6 +1,6 @@ import type { ServerInfoSnapshot } from "@paperclipai/shared"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; -import { ApiError } from "./client"; +import { apiErrorFromResponse, apiFetch } from "./client"; import { ApiUnavailableError, readApiJson } from "./response"; export type DevServerHealthStatus = { @@ -53,7 +53,7 @@ export type HealthStatus = { export const healthApi = { get: async (): Promise => { - const res = await fetch("/api/health", { + const res = await apiFetch("/api/health", { credentials: "include", headers: { Accept: "application/json" }, cache: "no-store", @@ -62,7 +62,7 @@ export const healthApi = { if (!res.ok) { const recovery = tenantSessionRecovery.recoverIfNeeded(res.status, payload); if (recovery) return recovery; - throw new ApiError(payload?.error ?? `Failed to load health (${res.status})`, res.status, payload); + throw apiErrorFromResponse(res, payload); } // Startup recovery can still serve sign-in and deployment metadata. if (payload?.status !== "ok" && payload?.status !== "starting") throw new ApiUnavailableError(res.status); diff --git a/ui/src/api/response.test.ts b/ui/src/api/response.test.ts index b826ec2c227..96b7d0c99d6 100644 --- a/ui/src/api/response.test.ts +++ b/ui/src/api/response.test.ts @@ -24,6 +24,14 @@ describe("API responses during a restart", () => { expect(fetchMock).toHaveBeenCalledTimes(3); }); + it("keeps the gateway's Retry-After on an HTML 503", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("", { + status: 503, + headers: { "Content-Type": "text/html", "Retry-After": "20" }, + }))); + await expect(api.get("/companies")).rejects.toMatchObject({ name: "ApiUnavailableError", retryAfterMs: 20_000 }); + }); + it("does not replay a mutation after an HTML response", async () => { const fetchMock = vi.fn().mockResolvedValue(new Response("", { status: 200 })); vi.stubGlobal("fetch", fetchMock); diff --git a/ui/src/api/response.ts b/ui/src/api/response.ts index c491f304673..cb25ebdd863 100644 --- a/ui/src/api/response.ts +++ b/ui/src/api/response.ts @@ -1,18 +1,24 @@ +import { errorCopy, isTransientError, parseRetryAfter } from "./errors"; + /** A proxy or restarting server answered an API request without usable JSON. */ export class ApiUnavailableError extends Error { - constructor(public readonly status: number) { - super("Paperclip is temporarily unavailable. Please try again in a moment."); + constructor( + public readonly status: number, + /** Delay the gateway asked for with `Retry-After`, in milliseconds. */ + public readonly retryAfterMs: number | null = null, + ) { + super(errorCopy("transient").body); this.name = "ApiUnavailableError"; } } +/** + * Thin alias for `classifyError(error) === "transient"` (see `api/errors.ts`), + * kept so existing callers such as `CloudAccessGate` pick up new transient + * codes (`tenant_app_unavailable` in any status, 408/425/429) automatically. + */ export function isTemporaryApiError(error: unknown): boolean { - if (error instanceof ApiUnavailableError) return true; - if (!(error instanceof Error)) return false; - if ("status" in error && [502, 503, 504].includes(error.status as number)) return true; - // Fetch uses different network-failure messages across browsers. Do not - // classify arbitrary TypeErrors (programming bugs) or aborts as outages. - return error instanceof TypeError && /fetch|network|load failed/i.test(error.message); + return isTransientError(error); } export async function readApiJson(response: Response): Promise { @@ -22,7 +28,9 @@ export async function readApiJson(response: Response): Promise { if (!(error instanceof SyntaxError)) throw error; // An HTML fallback can have status 200 during a deployment. Never expose // its parser error or treat it as a successful, empty API response. - if (response.ok || response.status >= 500) throw new ApiUnavailableError(response.status); + if (response.ok || response.status >= 500) { + throw new ApiUnavailableError(response.status, parseRetryAfter(response.headers?.get("Retry-After") ?? null)); + } // Preserve HTTP/auth error handling even when a 4xx response has no JSON. return null as T; } diff --git a/ui/src/components/AgentConversationsSidebar.tsx b/ui/src/components/AgentConversationsSidebar.tsx index bb173be4a17..9f70efdf661 100644 --- a/ui/src/components/AgentConversationsSidebar.tsx +++ b/ui/src/components/AgentConversationsSidebar.tsx @@ -1,4 +1,5 @@ import { AgentConversationSidebar } from "./AgentConversationSidebar"; +import { queryViewKind } from "./QueryView"; import { useAgentChatNavigation, useOpenAgentChat } from "@/hooks/useAgentChatNavigation"; import { useLocation, useNavigate } from "@/lib/router"; import { useSidebar } from "@/context/SidebarContext"; @@ -46,11 +47,13 @@ export function AgentConversationsSidebar() { return agent.id)} activeId={active?.id} previews={previews} - loading={agents.isPending || chats.isPending || session.isPending} - error={agents.error ?? chats.error ?? session.error} + // Loaded chats stay listed through an outage (the banner covers it); an + // outage before the first load is a skeleton, and only a real failure is an error. + loading={[agents, chats, session].some(query => { const kind = queryViewKind(query); return kind === "loading" || kind === "reconnecting"; })} + error={[agents, chats, session].find(query => queryViewKind(query) === "error")?.error ?? null} onRetry={() => { void agents.refetch(); void chats.refetch(); void session.refetch(); }} historyLoading={historyAgents.some(result => result.isPending)} - historyError={historyAgents.find(result => result.error)?.error} + historyError={historyAgents.find(result => queryViewKind(result) === "error")?.error} onRetryHistory={() => { historyAgents.filter(result => result.isError).forEach(result => { void result.refetch(); }); }} onAddChat={openChat} onSelect={agent => { diff --git a/ui/src/components/CaseRevisionRail.tsx b/ui/src/components/CaseRevisionRail.tsx index 4226d7593da..7d7de2c546f 100644 --- a/ui/src/components/CaseRevisionRail.tsx +++ b/ui/src/components/CaseRevisionRail.tsx @@ -6,6 +6,7 @@ import { casesApi, type CaseDocumentRevision, type CaseDocumentRevisions } from import { issuesApi } from "@/api/issues"; import { queryKeys } from "@/lib/queryKeys"; import { buildLineDiff, type DiffRow } from "@/lib/line-diff"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { Card } from "@/components/ui/card"; import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@/components/ui/dialog"; @@ -180,6 +181,7 @@ export function CaseRevisionRail({ queryKey: taskId ? queryKeys.issues.documentRevisions(taskId, documentKey) : queryKeys.cases.revisions(caseIdentifier, documentKey), queryFn: () => taskId ? issuesApi.listDocumentRevisions(taskId, documentKey) : casesApi.listRevisions(caseIdentifier, documentKey), }); + const revisionsView = useQueryView(revisionsQuery); const response = revisionsQuery.data; const revisions: CaseDocumentRevision[] = Array.isArray(response) ? response.map(revision => ({ ...revision, title: revision.title ?? documentKey, createdAt: new Date(revision.createdAt).toISOString(), @@ -197,11 +199,21 @@ export function CaseRevisionRail({ } }, [revisions, selectedId]); - if (revisionsQuery.isLoading) { + if (revisionsQuery.isLoading || revisionsView.kind === "reconnecting") { return

Loading revisions…

; } - if (revisionsQuery.isError) { - return

Could not load revisions.

; + if (revisionsView.kind === "error") { + return ( +
+ +
+ ); } if (revisions.length === 0) { return

No revisions yet.

; diff --git a/ui/src/components/CloudAccessGate.test.tsx b/ui/src/components/CloudAccessGate.test.tsx index 9d62e3f31de..0c264f6c893 100644 --- a/ui/src/components/CloudAccessGate.test.tsx +++ b/ui/src/components/CloudAccessGate.test.tsx @@ -2,9 +2,12 @@ import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; -import { focusManager, QueryClient, QueryClientProvider, QueryObserver } from "@tanstack/react-query"; +import { focusManager, onlineManager, QueryClientProvider, QueryObserver, type QueryClient } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CloudAccessGate } from "./CloudAccessGate"; +import { ConnectionStatusBanner } from "./ConnectionStatusBanner"; +import { ConnectivityProvider, createConnectivityStore, type ConnectivityStore } from "@/lib/connectivity"; +import { bindConnectivity, createAppQueryClient } from "@/lib/query-client"; import { queryKeys } from "@/lib/queryKeys"; import { ApiUnavailableError } from "@/api/response"; @@ -36,16 +39,21 @@ describe("CloudAccessGate restart recovery", () => { let root: Root; let container: HTMLDivElement; let client: QueryClient; + let store: ConnectivityStore; + let unbind: () => void; + /** A single failing path, or every API path for a full outage. */ let failingPath: string | null; let failure: () => Response; const fetchMock = vi.fn(); + const callsTo = (path: string) => fetchMock.mock.calls.filter(([url]) => url === path).length; + beforeEach(() => { vi.useFakeTimers(); failingPath = null; failure = () => new Response("

Restarting

", { status: 200 }); fetchMock.mockImplementation(async (path: string) => { - if (path === failingPath) return failure(); + if (failingPath === "*" || path === failingPath) return failure(); if (!(path in responses)) throw new Error(`Unexpected request: ${path}`); return Response.json(responses[path]); }); @@ -53,11 +61,18 @@ describe("CloudAccessGate restart recovery", () => { container = document.createElement("div"); document.body.append(container); root = createRoot(container); - client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } }); + // The app's real wiring: cache events feed the connectivity store, which + // drives onlineManager and refreshes live queries on recovery. + store = createConnectivityStore({ browserOnline: true }); + client = createAppQueryClient({ connectivity: store }, { defaultOptions: { queries: { retry: false, staleTime: Infinity } } }); + unbind = bindConnectivity(client, store); }); afterEach(() => { flushSync(() => root.unmount()); + unbind(); + store.dispose(); + onlineManager.setOnline(true); client.clear(); focusManager.setFocused(undefined); container.remove(); @@ -72,7 +87,20 @@ describe("CloudAccessGate restart recovery", () => { } async function render() { - flushSync(() => root.render()); + flushSync(() => root.render( + + + + + + , + )); + await flushReact(); + } + + /** Long enough for the probe/retry backoff (1s, 2s, 5s, 10s) to reach its next step. */ + async function waitForReconnect() { + await vi.advanceTimersByTimeAsync(16_000); await flushReact(); } @@ -83,13 +111,14 @@ describe("CloudAccessGate restart recovery", () => { expect(container.textContent).not.toContain("Unexpected token"); expect(container.querySelector("[data-redirect]")).toBeNull(); expect(container.querySelector("textarea")).toBeNull(); - const callsBeforeRetry = fetchMock.mock.calls.filter(([url]) => url === path).length; + // Backoff, not a tight loop: initial try plus retries at 1s and 3s. await vi.advanceTimersByTimeAsync(4_000); - expect(fetchMock.mock.calls.filter(([url]) => url === path)).toHaveLength(callsBeforeRetry); + expect(callsTo(path)).toBeLessThanOrEqual(path === "/api/health" ? 6 : 3); + // The full-page screen already explains the outage. + expect(container.querySelector("[data-connection-status]")).toBeNull(); failingPath = null; - await vi.advanceTimersByTimeAsync(1_100); - await flushReact(); + await waitForReconnect(); expect(container.querySelector("textarea")).not.toBeNull(); expect(container.textContent).not.toContain("Reconnecting"); const callsAfterRecovery = fetchMock.mock.calls.length; @@ -98,33 +127,40 @@ describe("CloudAccessGate restart recovery", () => { expect(fetchMock.mock.calls.every(([, init]) => !init.method || init.method === "GET")).toBe(true); }); - it.each(checks)("preserves a mounted editor through an outage on %s", async (path, queryKey) => { + it.each(checks)("preserves a mounted editor through an outage that starts on %s", async (path, queryKey) => { await render(); const editor = container.querySelector("textarea")!; editor.value = "Keep my unsaved changes"; - failingPath = path; + failingPath = "*"; failure = () => new Response("Bad gateway", { status: 502 }); - await client.refetchQueries({ queryKey }); + void client.refetchQueries({ queryKey }); await flushReact(); expect(container.querySelector("textarea")).toBe(editor); - expect(editor.value).toBe("Keep my unsaved changes"); + // One blip does not flash the banner; continued trouble does. + expect(container.textContent).not.toContain("Reconnecting automatically"); + await vi.advanceTimersByTimeAsync(2_100); + await flushReact(); expect(container.textContent).toContain("Reconnecting automatically"); + expect(container.querySelector("textarea")).toBe(editor); failingPath = null; - await vi.advanceTimersByTimeAsync(5_100); - await flushReact(); + await waitForReconnect(); expect(container.querySelector("textarea")).toBe(editor); expect(editor.value).toBe("Keep my unsaved changes"); expect(container.textContent).not.toContain("Reconnecting"); + await vi.advanceTimersByTimeAsync(3_100); + await flushReact(); + expect(container.querySelector("[data-connection-status]")).toBeNull(); }); - it.each(checks)("keeps retrying %s while the tab is hidden", async (path) => { + it.each(checks)("reconnects %s as soon as a hidden tab is visible again", async (path) => { focusManager.setFocused(false); failingPath = path; await render(); expect(container.textContent).toContain("Reconnecting to Paperclip"); failingPath = null; - await vi.advanceTimersByTimeAsync(5_100); + await waitForReconnect(); + focusManager.setFocused(true); await flushReact(); expect(container.querySelector("textarea")).not.toBeNull(); }); @@ -135,12 +171,12 @@ describe("CloudAccessGate restart recovery", () => { await render(); expect(container.querySelector("textarea")).toBeNull(); expect(container.textContent).toContain("Reconnecting"); - expect(fetchMock.mock.calls.map(([path]) => path)).toEqual(checks.map(([path]) => path)); + // The session is checked once; later checks wait for the server to be ready. + expect(callsTo("/api/auth/get-session")).toBe(1); await vi.advanceTimersByTimeAsync(5_100); expect(container.querySelector("textarea")).toBeNull(); failingPath = null; - await vi.advanceTimersByTimeAsync(5_100); - await flushReact(); + await waitForReconnect(); expect(container.querySelector("textarea")).not.toBeNull(); expect(container.textContent).not.toContain("Reconnecting"); }); @@ -158,15 +194,15 @@ describe("CloudAccessGate restart recovery", () => { await render(); const editor = container.querySelector("textarea")!; editor.value = "Keep my unsaved changes"; - fetchMock.mockResolvedValueOnce(Response.json({ - status: "starting", deploymentMode: "authenticated", bootstrapStatus: "ready", - })); + failingPath = "/api/health"; + failure = () => Response.json({ status: "starting", deploymentMode: "authenticated", bootstrapStatus: "ready" }); await client.refetchQueries({ queryKey: queryKeys.health }); + await vi.advanceTimersByTimeAsync(2_100); await flushReact(); expect(container.querySelector("textarea")).toBe(editor); expect(container.textContent).toContain("Reconnecting automatically"); - await vi.advanceTimersByTimeAsync(5_100); - await flushReact(); + failingPath = null; + await waitForReconnect(); expect(container.querySelector("textarea")).toBe(editor); expect(editor.value).toBe("Keep my unsaved changes"); expect(container.textContent).not.toContain("Reconnecting"); @@ -175,6 +211,7 @@ describe("CloudAccessGate restart recovery", () => { it("offers immediate retry while waiting for the next automatic check", async () => { failingPath = "/api/health"; await render(); + await vi.advanceTimersByTimeAsync(4_000); failingPath = null; container.querySelector("button")!.click(); await flushReact(); @@ -191,7 +228,7 @@ describe("CloudAccessGate restart recovery", () => { expect(container.querySelector("[data-redirect]")).toBeNull(); }); - it("refreshes other failed reads after access checks recover", async () => { + it("refreshes other failed reads after the server recovers", async () => { const read = vi.fn().mockRejectedValueOnce(new ApiUnavailableError(503)).mockResolvedValue([]); const observer = new QueryObserver(client, { queryKey: ["companies", "test"], queryFn: read, retry: false }); const unsubscribe = observer.subscribe(() => {}); @@ -199,8 +236,7 @@ describe("CloudAccessGate restart recovery", () => { await render(); expect(read).toHaveBeenCalledTimes(1); failingPath = null; - await vi.advanceTimersByTimeAsync(5_100); - await flushReact(); + await waitForReconnect(); expect(read).toHaveBeenCalledTimes(2); expect(observer.getCurrentResult().data).toEqual([]); unsubscribe(); diff --git a/ui/src/components/CloudAccessGate.tsx b/ui/src/components/CloudAccessGate.tsx index 5ea7edf5ad9..0b9436104e5 100644 --- a/ui/src/components/CloudAccessGate.tsx +++ b/ui/src/components/CloudAccessGate.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import { RefreshCw } from "lucide-react"; import { Navigate, Outlet, useLocation } from "@/lib/router"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; @@ -7,15 +7,19 @@ import { ApiError } from "@/api/client"; import { authApi } from "@/api/auth"; import { healthApi } from "@/api/health"; import { isTemporaryApiError } from "@/api/response"; +import { useConnectivityStore } from "@/lib/connectivity"; +import { reconnectDelayFor, retryWhileTransient } from "@/lib/query-client"; import { queryKeys } from "@/lib/queryKeys"; import { BootstrapPendingPage } from "@/components/BootstrapPendingPage"; import { PaperclipLoading } from "@/components/AnimatedPaperclipIcon"; import { Card } from "@/components/ui/card"; import { Button } from "@/components/ui/button"; import { CloudSignIn } from "@/components/CloudSignIn"; +import { useSuppressConnectionBanner } from "@/components/ConnectionStatusBanner"; import { clearCloudSignInAttempt } from "@/lib/cloud-sign-in"; -const RECONNECT_INTERVAL_MS = 5_000; +/** Poll while the server reports `starting`; outages are polled by `lib/connectivity`. */ +const SERVER_STARTING_POLL_MS = 5_000; export function CloudAccessError({ temporary, @@ -26,6 +30,7 @@ export function CloudAccessError({ retrying: boolean; onRetry: () => void; }) { + useSuppressConnectionBanner(temporary); return (
- {isError ? ( + {isError ? ( // query-error-ok: set only for a real failure (useIssueExternalObjects keeps data through outages)

Couldn't load external objects.{" "} {onRetry ? ( diff --git a/ui/src/components/IssueShareSheet.tsx b/ui/src/components/IssueShareSheet.tsx index c2269bc397b..ad18db31643 100644 --- a/ui/src/components/IssueShareSheet.tsx +++ b/ui/src/components/IssueShareSheet.tsx @@ -33,6 +33,7 @@ import { } from "@/components/ui/dialog"; import { SearchableSelect, type SearchableSelectGroup } from "@/components/SearchableSelect"; import { Skeleton } from "@/components/ui/skeleton"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { agentsApi } from "@/api/agents"; import { accessApi } from "@/api/access"; import { issuesApi } from "@/api/issues"; @@ -295,9 +296,10 @@ export function IssueShareSheet({ addMutation.mutate({ subjectType, subjectId }); } - const isLoading = grantsQuery.isLoading; - const isError = grantsQuery.isError; - const isEmpty = !isLoading && !isError && activeGrants.length === 0 && implicitPrincipals.length === 0; + const grantsView = useQueryView(grantsQuery); + const isLoading = grantsQuery.isLoading || grantsView.kind === "reconnecting"; + const loadFailed = grantsView.kind === "error"; + const isEmpty = !isLoading && !loadFailed && activeGrants.length === 0 && implicitPrincipals.length === 0; return (

@@ -324,18 +326,14 @@ export function IssueShareSheet({ ))} - ) : isError ? ( -
- - - + ) : loadFailed ? ( +
+
) : isEmpty ? (

diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index 48086260041..c421a3ac64a 100644 --- a/ui/src/components/IssueThreadInteractionCard.test.tsx +++ b/ui/src/components/IssueThreadInteractionCard.test.tsx @@ -751,7 +751,7 @@ describe("IssueThreadInteractionCard", () => { expect( host.querySelector('[data-testid="interaction-action-error"]')?.textContent, - ).toBe("Request failed: 503. Try again."); + ).toBe("Paperclip is temporarily unavailable. Please try again in a moment."); }); it("surfaces a denied suggested-task acceptance instead of failing silently", async () => { diff --git a/ui/src/components/NewIssueDialog.tsx b/ui/src/components/NewIssueDialog.tsx index 0f8135cae3e..42989bae966 100644 --- a/ui/src/components/NewIssueDialog.tsx +++ b/ui/src/components/NewIssueDialog.tsx @@ -360,7 +360,6 @@ export function NewIssueDialog() { queryKey: queryKeys.instance.experimentalSettings, queryFn: () => instanceSettingsApi.getExperimental(), enabled: newIssueOpen, - retry: false, }); const currentUserId = session?.user?.id ?? session?.session?.userId ?? null; const activeProjects = useMemo(() => projects ?? [], [projects]); @@ -389,14 +388,12 @@ export function NewIssueDialog() { reuseEligible: true, }), enabled: Boolean(effectiveCompanyId) && newIssueOpen && canChooseWorktrees, - retry: false, }); const { data: privacyParent, isError: parentPrivacyError, refetch: refetchParentPrivacy } = useQuery({ queryKey: queryKeys.issues.detail(newIssueDefaults.parentId ?? ""), queryFn: () => issuesApi.get(newIssueDefaults.parentId!), enabled: newIssueOpen && Boolean(newIssueDefaults.parentId), - retry: false, }); const parentPrivacyUnresolved = isSubIssueMode && !privacyParent; const inheritsPrivateAccess = privacyParent?.visibility === "private" || privacyParent?.project?.visibility === "private"; diff --git a/ui/src/components/QueryView.test.tsx b/ui/src/components/QueryView.test.tsx new file mode 100644 index 00000000000..d3a7990337d --- /dev/null +++ b/ui/src/components/QueryView.test.tsx @@ -0,0 +1,306 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider, useQuery } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { ConnectivityProvider, createConnectivityStore, type ConnectivityStore } from "@/lib/connectivity"; +import { QueryErrorState, QueryView, queryViewKind, useQueryView, type QueryViewState } from "./QueryView"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +const unavailable = () => new ApiError("Paperclip is restarting.", 503, { error: "tenant_app_unavailable" }); +const notFound = () => new ApiError("Issue not found", 404, { error: "Issue not found" }); +const forbidden = () => new ApiError("Forbidden", 403, { error: "Forbidden" }); + +describe("queryViewKind", () => { + const base = { data: undefined, error: null, status: "pending" as const, fetchStatus: "fetching" as const }; + + it("loads, then is ready", () => { + expect(queryViewKind(base)).toBe("loading"); + expect(queryViewKind({ ...base, data: 1, status: "success", fetchStatus: "idle" })).toBe("ready"); + expect(queryViewKind({ ...base, data: 1, status: "success", fetchStatus: "fetching" })).toBe("ready"); + }); + + it("keeps data through a transient failure (stale) and shows an error only for real failures", () => { + expect(queryViewKind({ ...base, data: 1, status: "error", error: unavailable() })).toBe("stale"); + expect(queryViewKind({ ...base, data: 1, status: "error", error: notFound() })).toBe("error"); + expect(queryViewKind({ ...base, data: 1, status: "error", error: forbidden() })).toBe("error"); + }); + + it("reconnects quietly when nothing has loaded yet and the app is waiting out an outage", () => { + expect(queryViewKind({ ...base, status: "error", error: unavailable() }, "reconnecting")).toBe("reconnecting"); + expect(queryViewKind({ ...base, status: "error", error: unavailable() }, "offline")).toBe("reconnecting"); + expect(queryViewKind({ ...base, status: "error", error: new TypeError("Failed to fetch") }, "reconnecting")).toBe("reconnecting"); + expect(queryViewKind({ ...base, status: "error", error: notFound() }, "reconnecting")).toBe("error"); + expect(queryViewKind({ ...base, status: "error", error: new Error("boom") }, "reconnecting")).toBe("error"); + }); + + it("shows retryable copy when a route keeps failing while the server is reachable", () => { + // Nothing would refetch these: the probe loop only runs while the app is + // not online. A placeholder here would never fill in. + expect(queryViewKind({ ...base, status: "error", error: unavailable() })).toBe("error"); + expect(queryViewKind({ ...base, status: "error", error: new ApiError("Gateway timeout", 504, "upstream timed out") })).toBe("error"); + expect(queryViewKind({ ...base, status: "error", error: new TypeError("Failed to fetch") })).toBe("error"); + }); + + it("shows retryable copy for a route-level blip with no data, but keeps data through it", () => { + const busy = new ApiError("Too many requests", 429, { error: "rate_limited" }); + const worker = new ApiError("WORKER_UNAVAILABLE", 503, { code: "WORKER_UNAVAILABLE", message: "restarting" }); + expect(queryViewKind({ ...base, status: "error", error: busy })).toBe("error"); + expect(queryViewKind({ ...base, status: "error", error: worker })).toBe("error"); + expect(queryViewKind({ ...base, status: "error", error: busy }, "reconnecting")).toBe("reconnecting"); + expect(queryViewKind({ ...base, data: 1, status: "error", error: busy })).toBe("stale"); + expect(queryViewKind({ ...base, data: 1, status: "error", error: worker })).toBe("stale"); + }); + + it("treats a paused fetch and in-flight transient retries as an outage", () => { + expect(queryViewKind({ ...base, fetchStatus: "paused" })).toBe("reconnecting"); + expect(queryViewKind({ ...base, failureReason: unavailable() })).toBe("reconnecting"); + expect(queryViewKind({ ...base, data: 1, status: "success", fetchStatus: "paused" })).toBe("stale"); + expect(queryViewKind({ ...base, data: 1, status: "success", failureReason: unavailable() })).toBe("stale"); + }); + + it("follows the app-wide connectivity state", () => { + expect(queryViewKind({ ...base, data: 1, status: "success", fetchStatus: "fetching" }, "reconnecting")).toBe("stale"); + expect(queryViewKind({ ...base, data: 1, status: "success", fetchStatus: "idle" }, "reconnecting")).toBe("stale"); + expect(queryViewKind(base, "offline")).toBe("reconnecting"); + // A disabled query is not waiting on the network. + expect(queryViewKind({ ...base, fetchStatus: "idle" }, "reconnecting")).toBe("loading"); + }); + + it("does not treat null data as missing", () => { + expect(queryViewKind({ ...base, data: null, status: "success", fetchStatus: "idle" })).toBe("ready"); + expect(queryViewKind({ ...base, data: null, status: "error", error: unavailable() })).toBe("stale"); + }); +}); + +type Deferred = { promise: Promise; resolve: (value: T) => void; reject: (error: unknown) => void }; +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((nextResolve, nextReject) => { + resolve = nextResolve; + reject = nextReject; + }); + return { promise, resolve, reject }; +} + +describe("useQueryView and ", () => { + let container: HTMLDivElement; + let root: Root; + let client: QueryClient; + let store: ConnectivityStore; + let probe: ReturnType Promise<{ reachable: true }>>>; + let attempts: Deferred<{ title: string }>[]; + let fetchCount: number; + let views: QueryViewState<{ title: string }>[]; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + client = new QueryClient({ defaultOptions: { queries: { retry: false, networkMode: "always" } } }); + probe = vi.fn(async () => ({ reachable: true as const })); + store = createConnectivityStore({ probe, browserOnline: true }); + attempts = []; + fetchCount = 0; + views = []; + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + client.clear(); + store.dispose(); + }); + + function Harness({ staleHint = false, notFound }: { staleHint?: boolean; notFound?: React.ReactNode }) { + const query = useQuery({ + queryKey: ["thing"], + queryFn: () => { + const attempt = deferred<{ title: string }>(); + attempts.push(attempt); + fetchCount += 1; + return attempt.promise; + }, + }); + views.push(useQueryView(query)); + return ( + + {(data) =>

{data.title}

} + + ); + } + + function mount(props: { staleHint?: boolean; notFound?: React.ReactNode } = {}) { + act(() => { + root.render( + + + + + , + ); + }); + } + + const last = () => views[views.length - 1]!; + // React Query batches observer notifications on a timer, so flush macrotasks. + const settle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + it("shows a placeholder, then the data", async () => { + mount(); + expect(last().kind).toBe("loading"); + expect(container.querySelector('[data-query-view="placeholder"]')).not.toBeNull(); + attempts[0]!.resolve({ title: "Loaded" }); + await settle(); + expect(last().kind).toBe("ready"); + expect(container.querySelector('[data-testid="data"]')?.textContent).toBe("Loaded"); + }); + + it("keeps loaded data on screen when a refetch fails with a transient error", async () => { + mount({ staleHint: true }); + attempts[0]!.resolve({ title: "Loaded" }); + await settle(); + await act(async () => { + void client.refetchQueries({ queryKey: ["thing"] }); + }); + attempts[1]!.reject(unavailable()); + await settle(); + expect(last().kind).toBe("stale"); + expect(last().data).toEqual({ title: "Loaded" }); + expect(container.querySelector('[data-testid="data"]')?.textContent).toBe("Loaded"); + expect(container.querySelector('[data-query-view="stale"]')).not.toBeNull(); + expect(container.querySelector('[data-query-view="error"]')).toBeNull(); + expect(container.querySelector(".text-destructive")).toBeNull(); + }); + + it("shows a quiet placeholder when the first load fails during an outage", async () => { + // The server is down: the probe fails, so the store is reconnecting and + // will refetch every live query on recovery. + probe.mockResolvedValue({ reachable: false, retryAfterMs: null } as unknown as { reachable: true }); + mount(); + attempts[0]!.reject(unavailable()); + store.reportError(unavailable()); + await settle(); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(last().kind).toBe("reconnecting"); + expect(container.querySelector('[data-query-view="placeholder"]')).not.toBeNull(); + expect(container.querySelector('[data-query-view="error"]')).toBeNull(); + expect(container.textContent).not.toContain("tenant_app_unavailable"); + }); + + it("shows readable copy with Retry when one route keeps failing while the server is reachable", async () => { + // The probe says the server is fine, so no recovery loop will ever refetch + // this query: a placeholder would spin forever. + mount(); + attempts[0]!.reject(unavailable()); + store.reportError(unavailable()); + await settle(); + expect(store.getSnapshot().status).toBe("online"); + expect(last().kind).toBe("error"); + expect(last().errorKind).toBe("transient"); + const alert = container.querySelector('[data-query-view="error"]'); + expect(alert).not.toBeNull(); + expect(alert?.textContent).not.toContain("tenant_app_unavailable"); + expect(alert?.querySelector("button")?.textContent).toContain("Retry"); + expect(container.querySelector('[data-query-view="placeholder"]')).toBeNull(); + await act(async () => { + alert!.querySelector("button")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + expect(fetchCount).toBe(2); + attempts[1]!.resolve({ title: "Recovered" }); + await settle(); + expect(last().kind).toBe("ready"); + expect(container.querySelector('[data-testid="data"]')?.textContent).toBe("Recovered"); + }); + + it("shows readable copy and retries for a real error", async () => { + mount(); + attempts[0]!.reject(new ApiError("Request failed: 500", 500, { error: "internal_error" })); + await settle(); + expect(last().kind).toBe("error"); + expect(last().errorKind).toBe("unknown"); + const alert = container.querySelector('[data-query-view="error"]'); + expect(alert?.textContent).toContain("Couldn't load the thing"); + expect(alert?.textContent).not.toContain("internal_error"); + const retry = alert?.querySelector("button"); + expect(retry?.textContent).toContain("Retry"); + await act(async () => { + retry!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + expect(fetchCount).toBe(2); + attempts[1]!.resolve({ title: "Recovered" }); + await settle(); + expect(last().kind).toBe("ready"); + expect(container.querySelector('[data-testid="data"]')?.textContent).toBe("Recovered"); + }); + + it("renders not_found only for a real 404", async () => { + mount({ notFound:

Gone

}); + attempts[0]!.reject(notFound()); + await settle(); + expect(last().errorKind).toBe("not_found"); + expect(container.querySelector('[data-testid="missing"]')?.textContent).toBe("Gone"); + }); + + it("retry probes the server during an outage", async () => { + mount(); + attempts[0]!.reject(unavailable()); + await settle(); + // The failed query made the store suspect an outage and probe; make the probe fail so the store is reconnecting. + probe.mockResolvedValueOnce({ reachable: false, retryAfterMs: null } as unknown as { reachable: true }); + await settle(); + store.reportError(unavailable()); + await settle(); + probe.mockClear(); + act(() => last().retry()); + expect(fetchCount).toBe(2); + if (store.getSnapshot().status !== "online") expect(probe).toHaveBeenCalled(); + }); +}); + +describe("", () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + }); + + it("uses readable copy and leaves out Retry for errors a retry cannot fix", () => { + act(() => { + root.render( undefined} size="inline" />); + }); + expect(container.textContent).toContain("Forbidden"); + expect(container.querySelector("button")).toBeNull(); + + act(() => { + root.render( undefined} />); + }); + expect(container.textContent).not.toContain("access_denied"); + expect(container.textContent).toContain("permission"); + expect(container.querySelector("button")).toBeNull(); + + act(() => { + root.render( undefined} size="page" />); + }); + expect(container.textContent).not.toContain("stale_revision"); + expect(container.querySelector("button")?.textContent).toContain("Retry"); + }); +}); diff --git a/ui/src/components/QueryView.tsx b/ui/src/components/QueryView.tsx new file mode 100644 index 00000000000..0fd7bd7b2e7 --- /dev/null +++ b/ui/src/components/QueryView.tsx @@ -0,0 +1,297 @@ +/** + * One way to render a read. + * + * Every surface that shows the result of a `useQuery` faces the same question + * during a deploy or a gateway blip: the refetch failed, but the data is still + * in the cache. The answer is always the same, so it lives here: + * + * - **If data exists, render the data.** A transient failure with data is + * `stale`: a normal render, with an optional small "Updating paused" hint + * and no red text. The app-level connection banner explains the outage. + * - An outage with no data is `reconnecting`: a skeleton or a muted + * placeholder that fills in by itself when the server is back. + * - A non-transient failure (403, 404, validation, conflict, a real 500) is + * `error`: readable `describeError` copy with a Retry button. So is a + * transient failure with no data once the retry policy has given up while + * the server is reachable (a 429, one plugin worker restarting, a 504 on + * one slow route): the app-wide probe loop is not running, so nothing else + * would refetch it. + * - A `not_found` state is shown only when `classifyError` says `not_found`, + * never for an outage. + * + * `useQueryView(query)` returns the state; `` renders it with + * sensible defaults; `` is the error presentation on its own, + * for surfaces that keep their own loading and empty states. + */ + +import { useCallback, type ReactNode } from "react"; +import { AlertTriangle, RefreshCcw } from "lucide-react"; +import { classifyError, describeError, isTransientError, type ErrorKind } from "@/api/errors"; +import { PageSkeleton } from "@/components/PageSkeleton"; +import { Button } from "@/components/ui/button"; +import { Skeleton } from "@/components/ui/skeleton"; +import { useConnectivity, useConnectivityStore, type ConnectivityStatus } from "@/lib/connectivity"; +import { cn } from "@/lib/utils"; + +export type QueryViewKind = "loading" | "ready" | "stale" | "reconnecting" | "error"; + +export type QueryViewSize = "page" | "panel" | "inline"; + +/** The slice of a React Query result the view logic reads. */ +export interface QueryViewSource { + data: TData | undefined; + error: unknown; + /** `pending` until the first data arrives (also while disabled). */ + status: "pending" | "error" | "success"; + fetchStatus: "fetching" | "paused" | "idle"; + /** The error of the attempt currently being retried, before `error` is set. */ + failureReason?: unknown; + refetch: () => unknown; +} + +export interface QueryViewState { + kind: QueryViewKind; + /** The latest data, including cached data behind a `stale` or `error` kind. */ + data: TData | undefined; + error: unknown; + /** The classification of `error` when `kind` is `error`, otherwise null. */ + errorKind: ErrorKind | null; + /** Fetch again now; during an outage it also probes the server at once. */ + retry: () => void; + isFetching: boolean; +} + +function hasError(value: unknown): boolean { + return value !== null && value !== undefined; +} + +/** + * Which of the five states a query is in. Pure, so plugin hooks and tests can + * use it without React. `connectivity` is the app-wide connection status; a + * query with data is `stale` while the server is unreachable even before its + * own refetch fails. + */ +export function queryViewKind( + query: Pick, "data" | "error" | "status" | "fetchStatus" | "failureReason">, + connectivity: ConnectivityStatus = "online", +): QueryViewKind { + const hasData = query.data !== undefined; + if (hasError(query.error)) { + const transient = isTransientError(query.error); + if (hasData) return transient ? "stale" : "error"; + // With nothing to show, a transient failure is worth waiting out quietly + // only while the app-wide probe loop is running: it refetches every live + // query on recovery. While the server is reachable nothing else would + // refetch, so a route that keeps failing (a 429, one plugin worker + // restarting, a 504 on one slow endpoint) gets readable copy with a + // Retry button once the retry policy gives up. + if (transient && connectivity !== "online") return "reconnecting"; + return "error"; + } + const retryingTransient = hasError(query.failureReason) && isTransientError(query.failureReason); + if (hasData) { + return query.fetchStatus === "paused" || retryingTransient || connectivity !== "online" ? "stale" : "ready"; + } + if (query.fetchStatus === "paused" || retryingTransient) return "reconnecting"; + if (connectivity !== "online" && query.fetchStatus === "fetching") return "reconnecting"; + return "loading"; +} + +/** The view state for a query result, plus a retry that also wakes the connectivity probe. */ +export function useQueryView(query: QueryViewSource): QueryViewState { + const { status: connectivity } = useConnectivity(); + const store = useConnectivityStore(); + const { refetch } = query; + const retry = useCallback(() => { + if (store.getSnapshot().status !== "online") store.probeNow(); + void refetch(); + }, [store, refetch]); + const kind = queryViewKind(query, connectivity); + return { + kind, + data: query.data, + error: query.error ?? null, + errorKind: kind === "error" ? classifyError(query.error) : null, + retry, + isFetching: query.fetchStatus === "fetching", + }; +} + +// --- Presentation ----------------------------------------------------------------- + +export interface QueryErrorStateProps { + error: unknown; + size?: QueryViewSize; + /** What the user was doing, as a verb phrase: "load the task". */ + action?: string; + onRetry?: () => void; + /** Disables the Retry button while a retry is in flight. */ + retrying?: boolean; + className?: string; +} + +/** Errors a retry cannot fix: the Retry button is left out. */ +const NOT_RETRYABLE: ReadonlySet = new Set(["auth", "forbidden", "not_found"]); + +/** + * The standard presentation of a non-transient query error: readable copy + * from `describeError` and a Retry button. Never renders a raw error code. + */ +export function QueryErrorState({ error, size = "panel", action, onRetry, retrying = false, className }: QueryErrorStateProps) { + const kind = classifyError(error); + const { title, body } = describeError(error, { action }); + const retryButton = onRetry && !NOT_RETRYABLE.has(kind) ? ( + + ) : null; + + if (size === "inline") { + return ( + + {body} + {retryButton} + + ); + } + + if (size === "page") { + return ( +
+
+ ); + } + + return ( +
+
+ ); +} + +export interface QueryPlaceholderProps { + size?: QueryViewSize; + /** Accessible label for the busy region. */ + label?: string; + className?: string; +} + +/** A quiet placeholder for `loading` and `reconnecting`: it fills in by itself. */ +export function QueryPlaceholder({ size = "panel", label = "Loading", className }: QueryPlaceholderProps) { + if (size === "page") { + return ( +
+ +
+ ); + } + if (size === "inline") { + return ( + + + + ); + } + return ( +
+ + + +
+ ); +} + +/** The small hint above stale content, for surfaces that opt in with `staleHint`. */ +export function QueryStaleHint({ className }: { className?: string }) { + return ( +

+ Updating paused while reconnecting… +

+ ); +} + +export interface QueryViewProps { + query: QueryViewSource; + size?: QueryViewSize; + /** What the user was doing, for error copy: "load the task". */ + action?: string; + /** Shown while loading with no data. Defaults to a placeholder for the size. */ + loading?: ReactNode; + /** Shown while reconnecting with no data. Defaults to `loading`. */ + reconnecting?: ReactNode; + /** Shown when the error is a real `not_found`. Defaults to the error state. */ + notFound?: ReactNode; + /** Show a small "Updating paused" hint above stale content. Off by default. */ + staleHint?: boolean; + className?: string; + children: (data: TData, view: QueryViewState) => ReactNode; +} + +/** + * Render a query by its view state. Loaded data always renders, even while a + * refetch is failing; placeholders are quiet; only real errors show an error. + */ +export function QueryView({ + query, + size = "panel", + action, + loading, + reconnecting, + notFound, + staleHint = false, + className, + children, +}: QueryViewProps) { + const view = useQueryView(query); + switch (view.kind) { + case "loading": + return <>{loading ?? }; + case "reconnecting": + return <>{reconnecting ?? loading ?? }; + case "error": + if (view.errorKind === "not_found" && notFound !== undefined) return <>{notFound}; + return ( + + ); + case "stale": + return ( + <> + {staleHint ? : null} + {children(view.data as TData, view)} + + ); + case "ready": + return <>{children(view.data as TData, view)}; + } +} diff --git a/ui/src/components/Sidebar.production.tsx b/ui/src/components/Sidebar.production.tsx index 7a2b2a14d7b..9d59d80a53b 100644 --- a/ui/src/components/Sidebar.production.tsx +++ b/ui/src/components/Sidebar.production.tsx @@ -205,7 +205,6 @@ export function Sidebar() { ) : null} ) : null} agentsApi.list(companyId!), enabled: !!companyId, }); + // The picker keeps the last roster through an outage; it only reports a real failure. + const agentsView = useQueryView(agentsQuery); const agents = agentsQuery.data ?? []; const [pickerOpen, setPickerOpen] = useState(false); const navigate = useNavigate(); @@ -70,9 +73,9 @@ function CompanyAgentChats({ companyId, userId }: { companyId: string | null; us agents={agents} open={pickerOpen} onOpenChange={setPickerOpen} - loading={agentsQuery.isPending} - error={agentsQuery.error} - onRetry={() => { void agentsQuery.refetch(); }} + loading={agentsView.kind === "loading" || agentsView.kind === "reconnecting"} + error={agentsView.kind === "error" ? agentsQuery.error : null} + onRetry={agentsView.retry} onSelect={(agent) => { if (isMobile) setSidebarOpen(false); navigate(`/chats/${encodeURIComponent(agentRouteRef(agent))}`); diff --git a/ui/src/components/UnprefixedExecutionWorkspaceRedirect.test.tsx b/ui/src/components/UnprefixedExecutionWorkspaceRedirect.test.tsx index 9584e839bd2..ef299c0a2ed 100644 --- a/ui/src/components/UnprefixedExecutionWorkspaceRedirect.test.tsx +++ b/ui/src/components/UnprefixedExecutionWorkspaceRedirect.test.tsx @@ -5,6 +5,8 @@ import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { ConnectivityProvider, createConnectivityStore, type ConnectivityStore } from "@/lib/connectivity"; import { ExecutionWorkspaceCompanyGate, UnprefixedExecutionWorkspaceRedirect, @@ -50,20 +52,21 @@ describe("UnprefixedExecutionWorkspaceRedirect", () => { vi.clearAllMocks(); }); - function render(path: string) { + function render(path: string, options: { store?: ConnectivityStore } = {}) { root = createRoot(container); const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const tree = ( + + + + } /> + } /> + + + + ); flushSync(() => { - root.render( - - - - } /> - } /> - - - , - ); + root.render(options.store ? {tree} : tree); }); } @@ -97,14 +100,52 @@ describe("UnprefixedExecutionWorkspaceRedirect", () => { expect(container.textContent).not.toContain("/FOR/execution-workspaces"); }); - it("shows not found when the workspace cannot be resolved", async () => { - mockExecutionWorkspacesApi.get.mockRejectedValue(new Error("Execution workspace not found")); + it("shows not found when the workspace does not exist", async () => { + mockExecutionWorkspacesApi.get.mockRejectedValue(new ApiError("Execution workspace not found", 404, { error: "not_found" })); render("/execution-workspaces/missing/issues"); await vi.waitFor(() => expect(container.textContent).toContain("NOT_FOUND")); expect(container.textContent).not.toContain("DESTINATION@"); }); + it("keeps loading instead of showing not found during an outage", async () => { + mockExecutionWorkspacesApi.get.mockRejectedValue( + new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }), + ); + // The app-wide store is waiting out the outage, so the probe loop will refetch on recovery. + const store = createConnectivityStore({ browserOnline: false }); + render("/execution-workspaces/workspace-1/issues", { store }); + + await vi.waitFor(() => expect(mockExecutionWorkspacesApi.get).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(container.textContent).not.toContain("NOT_FOUND"); + expect(container.textContent).not.toContain("DESTINATION@"); + expect(container.querySelector('[data-query-view="error"]')).toBeNull(); + store.dispose(); + }); + + it("shows readable copy with a retry when the route keeps failing while the server is reachable", async () => { + mockExecutionWorkspacesApi.get.mockRejectedValue( + new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }), + ); + render("/execution-workspaces/workspace-1/issues"); + + await vi.waitFor(() => expect(container.querySelector('[data-query-view="error"]')).not.toBeNull()); + expect(container.textContent).not.toContain("NOT_FOUND"); + expect(container.textContent).not.toContain("tenant_app_unavailable"); + expect(container.textContent).toContain("Retry"); + }); + + it("shows readable copy with a retry for an unexpected failure", async () => { + mockExecutionWorkspacesApi.get.mockRejectedValue(new ApiError("internal_error", 500, { error: "internal_error" })); + render("/execution-workspaces/workspace-1/issues"); + + await vi.waitFor(() => expect(container.querySelector('[data-query-view="error"]')).not.toBeNull()); + expect(container.textContent).not.toContain("NOT_FOUND"); + expect(container.textContent).not.toContain("internal_error"); + expect(container.textContent).toContain("Retry"); + }); + it("rejects a prefixed route for a different company's workspace", async () => { mockExecutionWorkspacesApi.get.mockResolvedValue({ id: "workspace-1", companyId: PAP.id }); renderGate("/FOR/execution-workspaces/workspace-1/issues"); diff --git a/ui/src/components/UnprefixedExecutionWorkspaceRedirect.tsx b/ui/src/components/UnprefixedExecutionWorkspaceRedirect.tsx index 4f5f81af846..5c5bc814d50 100644 --- a/ui/src/components/UnprefixedExecutionWorkspaceRedirect.tsx +++ b/ui/src/components/UnprefixedExecutionWorkspaceRedirect.tsx @@ -4,26 +4,57 @@ import { useCompany } from "@/context/CompanyContext"; import { queryKeys } from "@/lib/queryKeys"; import { Navigate, Outlet, useLocation, useParams } from "@/lib/router"; import { PaperclipLoading } from "./AnimatedPaperclipIcon"; +import { QueryErrorState, useQueryView, type QueryViewState } from "./QueryView"; import { NotFoundPage } from "../pages/NotFound"; +/** + * A workspace lookup decides where a URL goes. An outage must not look like a + * missing workspace: keep the loading screen while reconnecting, show "not + * found" only for a real 404 (or a denial, which must not leak existence), + * and readable copy for anything else. + */ +function useWorkspaceLookup(workspaceId: string | undefined) { + const query = useQuery({ + queryKey: queryKeys.executionWorkspaces.detail(workspaceId ?? "__missing__"), + queryFn: () => executionWorkspacesApi.get(workspaceId!), + enabled: Boolean(workspaceId), + }); + return { workspace: query.data, view: useQueryView(query) }; +} + +function workspaceLookupGate(view: QueryViewState, companiesLoading: boolean) { + if (companiesLoading || view.kind === "loading" || view.kind === "reconnecting") return ; + if (view.kind === "error") { + if (view.errorKind === "not_found" || view.errorKind === "forbidden" || view.errorKind === "auth") { + return ; + } + return ( + + ); + } + return null; +} + /** Resolve a prefix-free workspace URL from the resource, not browsing state. */ export function UnprefixedExecutionWorkspaceRedirect() { const location = useLocation(); const { workspaceId } = useParams<{ workspaceId?: string }>(); const { companies, loading: companiesLoading } = useCompany(); - const workspaceQuery = useQuery({ - queryKey: queryKeys.executionWorkspaces.detail(workspaceId ?? "__missing__"), - queryFn: () => executionWorkspacesApi.get(workspaceId!), - enabled: Boolean(workspaceId), - retry: false, - }); + const { workspace, view } = useWorkspaceLookup(workspaceId); if (!workspaceId) return ; - if (companiesLoading || workspaceQuery.isPending) return ; - if (workspaceQuery.isError) return ; + const gate = workspaceLookupGate(view, companiesLoading); + if (gate) return gate; + if (!workspace) return ; const targetCompany = companies.find( - (company) => company.id === workspaceQuery.data.companyId, + (company) => company.id === workspace.companyId, ); if (!targetCompany) return ; @@ -42,21 +73,17 @@ export function ExecutionWorkspaceCompanyGate() { workspaceId?: string; }>(); const { companies, loading: companiesLoading } = useCompany(); - const workspaceQuery = useQuery({ - queryKey: queryKeys.executionWorkspaces.detail(workspaceId ?? "__missing__"), - queryFn: () => executionWorkspacesApi.get(workspaceId!), - enabled: Boolean(workspaceId), - retry: false, - }); + const { workspace, view } = useWorkspaceLookup(workspaceId); if (!workspaceId || !companyPrefix) return ; - if (companiesLoading || workspaceQuery.isPending) return ; - if (workspaceQuery.isError) return ; + const gate = workspaceLookupGate(view, companiesLoading); + if (gate) return gate; + if (!workspace) return ; const routeCompany = companies.find( (company) => company.issuePrefix.toUpperCase() === companyPrefix.toUpperCase(), ); - if (!routeCompany || routeCompany.id !== workspaceQuery.data.companyId) { + if (!routeCompany || routeCompany.id !== workspace.companyId) { return ; } diff --git a/ui/src/components/WorkspaceFileBrowser.tsx b/ui/src/components/WorkspaceFileBrowser.tsx index 56314f2b1bd..59ff60dd655 100644 --- a/ui/src/components/WorkspaceFileBrowser.tsx +++ b/ui/src/components/WorkspaceFileBrowser.tsx @@ -15,6 +15,8 @@ import { cn } from "@/lib/utils"; import { fileResourcesApi } from "@/api/file-resources"; import { projectsApi } from "@/api/projects"; import { ApiError } from "@/api/client"; +import { describeError } from "@/api/errors"; +import { queryViewKind } from "@/components/QueryView"; import { queryKeys } from "@/lib/queryKeys"; import { usePageVisibility } from "@/lib/page-visibility"; import { parseWorkspaceFileRef } from "@/lib/workspace-file-parser"; @@ -540,7 +542,6 @@ export function WorkspaceFileBrowser({ queryKey: companyId ? queryKeys.projects.list(companyId) : ["projects", "__none__"], queryFn: () => projectsApi.list(companyId!), enabled: source === "other" && !!companyId, - retry: false, staleTime: 30_000, }); @@ -649,7 +650,6 @@ export function WorkspaceFileBrowser({ path: folderPath, }, { signal }), enabled: queriesEnabled, - retry: false, staleTime: 15_000, refetchOnWindowFocus: false, refetchOnReconnect: false, @@ -716,7 +716,6 @@ export function WorkspaceFileBrowser({ path: spec.path || null, }, { signal }), enabled: queriesEnabled && isLazyBrowse, - retry: false, staleTime: 15_000, refetchOnWindowFocus: false, refetchOnReconnect: false, @@ -982,6 +981,21 @@ export function WorkspaceFileBrowser({ return children.map((node) => ({ ...node, depth })); } + // Loaded files stay on screen while a refetch fails; an outage before the + // first load is the skeleton. A saturated git scan is a per-route condition + // with its own copy and Retry, not an outage, even though its status is transient. + const listKind = queryViewKind(listQuery); + const listErrorStatus = listQuery.error instanceof ApiError ? listQuery.error.status : 0; + const listErrorCode = listQuery.error instanceof ApiError && listQuery.error.body && typeof listQuery.error.body === "object" + ? (listQuery.error.body as { code?: unknown }).code + : null; + const changedFilesTemporarilyUnavailable = listQuery.error != null && mode === "changed" && ( + listErrorStatus === 503 || + listErrorStatus === 504 || + listErrorCode === "workspace_git_scan_saturated" || + listErrorCode === "workspace_git_scan_timeout" + ); + let body: ReactNode; if (source === "other" && !companyId) { body = ( @@ -1007,7 +1021,7 @@ export function WorkspaceFileBrowser({ body="No same-organization project has a registered workspace to browse." /> ); - } else if (listQuery.isFetching && !data) { + } else if (!data && (listQuery.isFetching || (listKind === "reconnecting" && !changedFilesTemporarilyUnavailable))) { body = (
{Array.from({ length: 6 }).map((_, index) => ( @@ -1018,17 +1032,7 @@ export function WorkspaceFileBrowser({ ))}
); - } else if (listQuery.isError) { - const status = listQuery.error instanceof ApiError ? listQuery.error.status : 0; - const errorCode = listQuery.error instanceof ApiError && listQuery.error.body && typeof listQuery.error.body === "object" - ? (listQuery.error.body as { code?: unknown }).code - : null; - const changedFilesTemporarilyUnavailable = mode === "changed" && ( - status === 503 || - status === 504 || - errorCode === "workspace_git_scan_saturated" || - errorCode === "workspace_git_scan_timeout" - ); + } else if (listKind === "error" || (changedFilesTemporarilyUnavailable && !data)) { body = (
- {tasksQuery.isPending ? ( + {tasksView.kind === "loading" || tasksView.kind === "reconnecting" ? ( Loading tasks… - ) : tasksQuery.isError ? ( - Could not load this agent's tasks. + ) : tasksView.kind === "error" ? ( +
+ +
) : total === 0 ? ( This agent hasn't worked on any tasks yet. ) : visible.length === 0 ? ( @@ -293,9 +302,24 @@ export function AgentArtifactsPanel({ companyId, agentId }: { companyId: string; queryFn: () => listAgentArtifacts(companyId, agentId), }); - if (artifactsQuery.isPending) return Loading artifacts…; - if (artifactsQuery.isError) return Could not load this agent's artifacts.; - const { artifacts, truncated } = artifactsQuery.data; + const artifactsView = useQueryView(artifactsQuery); + + if (artifactsView.kind === "loading" || artifactsView.kind === "reconnecting") return Loading artifacts…; + if (artifactsView.kind === "error") { + return ( +
+ +
+ ); + } + const loaded = artifactsQuery.data; + if (!loaded) return null; + const { artifacts, truncated } = loaded; if (artifacts.length === 0) return This agent hasn't produced any artifacts yet.; return (
diff --git a/ui/src/components/task-side-panel/TaskAttachmentPanel.tsx b/ui/src/components/task-side-panel/TaskAttachmentPanel.tsx index adda17ea9cf..fc1dc0b9ba2 100644 --- a/ui/src/components/task-side-panel/TaskAttachmentPanel.tsx +++ b/ui/src/components/task-side-panel/TaskAttachmentPanel.tsx @@ -3,6 +3,7 @@ import { useQuery } from "@tanstack/react-query"; import { Download } from "lucide-react"; import { issuesApi } from "@/api/issues"; import { Button } from "@/components/ui/button"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { CsvPreview } from "@/components/CsvPreview"; import { isCsvFile } from "@/lib/csv-preview"; import { MarkdownBody } from "@/components/MarkdownBody"; @@ -16,7 +17,8 @@ export const TEXT_PREVIEW_MAX_BYTES = 512 * 1024; /** Bound the actual response, not just producer-supplied attachment metadata. */ export async function readTextPreview(response: Response) { - if (!response.ok) throw new Error(`Could not load file (${response.status}).`); + // Carry the status so the shared classification can tell an outage (503) from a real failure. + if (!response.ok) throw Object.assign(new Error(`Could not load file (${response.status}).`), { status: response.status }); if (!response.body) throw new Error("The file response is empty."); const reader = response.body.getReader(); const decoder = new TextDecoder("utf-8", { fatal: true }); @@ -86,16 +88,27 @@ export function TaskAttachmentPanel({ issueId, attachmentId }: { issueId: string { signal, credentials: "same-origin" }, )), enabled: Boolean(eligible), - retry: false, }); - if (attachments.isLoading) return

Loading file…

; - if (attachments.isError) return
Could not load attachment details.
; + const attachmentsView = useQueryView(attachments); + const contentView = useQueryView(content); + if (attachments.isLoading || attachmentsView.kind === "reconnecting") return

Loading file…

; + if (attachmentsView.kind === "error") { + return ( + + ); + } if (!attachment) return

File no longer available. Close this tab or choose another file.

; const downloadUrl = attachmentDownloadPath(attachment); - if (!eligible || content.isError) { + if (!eligible || contentView.kind === "error") { return (
-

{content.isError ? "Could not preview this file. Retry or download it." : "This file is too large or is not supported for text preview. Download it instead."}

+

{contentView.kind === "error" ? "Could not preview this file. Retry or download it." : "This file is too large or is not supported for text preview. Download it instead."}

{eligible ? : null}
diff --git a/ui/src/components/task-side-panel/TaskDocumentPanel.tsx b/ui/src/components/task-side-panel/TaskDocumentPanel.tsx index 3b38da9b891..d46725e8fe4 100644 --- a/ui/src/components/task-side-panel/TaskDocumentPanel.tsx +++ b/ui/src/components/task-side-panel/TaskDocumentPanel.tsx @@ -6,6 +6,7 @@ import { ApiError } from "@/api/client"; import { issuesApi } from "@/api/issues"; import { DocumentAnnotationsCountChip, IssueDocumentAnnotations } from "@/components/IssueDocumentAnnotations"; import { MarkdownBody } from "@/components/MarkdownBody"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { queryKeys } from "@/lib/queryKeys"; import { documentDisplayTitle } from "@/lib/issue-artifacts"; import { useLocation } from "@/lib/router"; @@ -34,7 +35,9 @@ export function TaskDocumentPanel({ initialData: initialDocument, }); - if (query.isLoading) { + const view = useQueryView(query); + + if (query.isLoading || view.kind === "reconnecting") { return (
@@ -42,10 +45,10 @@ export function TaskDocumentPanel({
); } - if (query.isError) { + if (view.kind === "error") { return ( -
- The document could not be loaded. Retry from the tab launcher or refresh the task. +
+
); } diff --git a/ui/src/components/task-side-panel/TaskSkillPanel.test.tsx b/ui/src/components/task-side-panel/TaskSkillPanel.test.tsx index ef2389bbcc4..bc2a8d59abb 100644 --- a/ui/src/components/task-side-panel/TaskSkillPanel.test.tsx +++ b/ui/src/components/task-side-panel/TaskSkillPanel.test.tsx @@ -6,6 +6,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ApiError } from "@/api/client"; import { companySkillsApi } from "@/api/companySkills"; +import { ConnectivityProvider, createConnectivityStore } from "@/lib/connectivity"; import { TaskSkillPanel } from "./TaskSkillPanel"; const navigate = vi.fn(); @@ -43,10 +44,40 @@ describe("TaskSkillPanel", () => { expect(container.querySelector("button")).toBeNull(); }); - it("offers retry for transient errors", async () => { + it("keeps a quiet loading state during an outage instead of an error", async () => { + vi.mocked(companySkillsApi.detail).mockRejectedValue( + new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }), + ); + // The app-wide store is waiting out the outage, so the probe loop will refetch on recovery. + const store = createConnectivityStore({ browserOnline: false }); + act(() => root.render( + + + , + )); + await vi.waitFor(() => expect(companySkillsApi.detail).toHaveBeenCalled()); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(container.textContent).toContain("Loading skill…"); + expect(container.querySelector('[data-query-view="error"]')).toBeNull(); + expect(container.textContent).not.toContain("tenant_app_unavailable"); + store.dispose(); + }); + + it("shows readable copy with Retry when the skill route keeps failing while the server is reachable", async () => { + vi.mocked(companySkillsApi.detail) + .mockRejectedValueOnce(new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" })) + .mockResolvedValue(skill); + renderPanel(); + await vi.waitFor(() => expect(container.querySelector('[data-query-view="error"]')).not.toBeNull()); + expect(container.textContent).not.toContain("tenant_app_unavailable"); + act(() => (container.querySelector("button") as HTMLButtonElement).click()); + await vi.waitFor(() => expect(container.textContent).toContain("Release helper")); + }); + + it("offers retry with readable copy for an unexpected failure", async () => { vi.mocked(companySkillsApi.detail).mockRejectedValueOnce(new Error("offline")).mockResolvedValue(skill); renderPanel(); - await vi.waitFor(() => expect(container.textContent).toContain("The skill could not be loaded")); + await vi.waitFor(() => expect(container.textContent).toContain("Couldn't load the skill")); act(() => (container.querySelector("button") as HTMLButtonElement).click()); await vi.waitFor(() => expect(container.textContent).toContain("Release helper")); expect(container.textContent).toContain("Run the release."); diff --git a/ui/src/components/task-side-panel/TaskSkillPanel.tsx b/ui/src/components/task-side-panel/TaskSkillPanel.tsx index fe5d2ba653f..741b8650fa1 100644 --- a/ui/src/components/task-side-panel/TaskSkillPanel.tsx +++ b/ui/src/components/task-side-panel/TaskSkillPanel.tsx @@ -1,8 +1,8 @@ import { useQuery } from "@tanstack/react-query"; import { ExternalLink, FileText, Loader2, Wrench } from "lucide-react"; import { companySkillsApi } from "@/api/companySkills"; -import { ApiError } from "@/api/client"; import { MarkdownBody } from "@/components/MarkdownBody"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { queryKeys } from "@/lib/queryKeys"; import { useNavigate } from "@/lib/router"; @@ -13,11 +13,11 @@ export function TaskSkillPanel({ companyId, skillId }: { companyId: string; skil const query = useQuery({ queryKey: queryKeys.companySkills.detail(companyId, skillId), queryFn: () => companySkillsApi.detail(companyId, skillId), - retry: false, refetchOnWindowFocus: true, refetchOnMount: "always", }); - if (query.isLoading) { + const view = useQueryView(query); + if (query.isLoading || view.kind === "reconnecting") { return (
@@ -25,20 +25,16 @@ export function TaskSkillPanel({ companyId, skillId }: { companyId: string; skil
); } - if (query.isError) { - const status = query.error instanceof ApiError ? query.error.status : null; - if (status === 404) { + if (view.kind === "error") { + if (view.errorKind === "not_found") { return
Skill no longer available.
; } - if (status === 403) { + if (view.errorKind === "forbidden") { return
You do not have access to this skill.
; } return ( -
-

The skill could not be loaded.

- +
+
); } diff --git a/ui/src/context/LiveUpdatesProvider.tsx b/ui/src/context/LiveUpdatesProvider.tsx index b2d0f8021e4..3d88229d4cf 100644 --- a/ui/src/context/LiveUpdatesProvider.tsx +++ b/ui/src/context/LiveUpdatesProvider.tsx @@ -50,6 +50,7 @@ import { clearIssueExecutionRun, removeLiveRunById, } from "../lib/optimistic-issue-runs"; +import { useConnectivityStore } from "../lib/connectivity"; import { queryKeys } from "../lib/queryKeys"; import { extractCompanyPrefixFromPath, toCompanyRelativePath } from "../lib/company-routes"; import { useLocation } from "../lib/router"; @@ -2054,6 +2055,7 @@ export function LiveUpdatesProvider({ children }: { children: ReactNode }) { const hasNeededSocketRecovery = useRef(!visible); const { selectedCompanyId, selectedCompany } = useCompany(); const queryClient = useQueryClient(); + const connectivity = useConnectivityStore(); const { pushToast } = useToastActions(); const location = useLocation(); const gateRef = useRef({ @@ -2195,6 +2197,7 @@ export function LiveUpdatesProvider({ children }: { children: ReactNode }) { } setConnectedScopeKey(socketScopeKey); stopPolling(); + connectivity.reportSocketStatus("open"); transcripts.open(nextSocket); if (reconnectAttempt > 0) { gateRef.current.suppressUntil = Date.now() + RECONNECT_SUPPRESS_MS; @@ -2266,6 +2269,9 @@ export function LiveUpdatesProvider({ children }: { children: ReactNode }) { transcripts.close(); if (closed) return; setConnectedScopeKey((current) => current === socketScopeKey ? null : current); + // A dropped socket may be the first sign of an outage; the store + // confirms with a health probe before it changes state. + connectivity.reportSocketStatus("closed"); startPolling(); scheduleReconnect(); }; @@ -2296,6 +2302,7 @@ export function LiveUpdatesProvider({ children }: { children: ReactNode }) { pushToast, canConnectSocket, socketAuthKey, + connectivity, socketScopeKey, transcripts, ]); diff --git a/ui/src/hooks/useIssueExternalObjects.ts b/ui/src/hooks/useIssueExternalObjects.ts index 12ac44a4f01..b8c178d60c5 100644 --- a/ui/src/hooks/useIssueExternalObjects.ts +++ b/ui/src/hooks/useIssueExternalObjects.ts @@ -1,5 +1,6 @@ import { useCallback, useMemo } from "react"; import { useQuery } from "@tanstack/react-query"; +import { queryViewKind } from "../components/QueryView"; import type { ExternalObjectMention, ExternalObjectMentionGroup, @@ -71,7 +72,6 @@ function useExternalObjectsFeature() { const query = useQuery({ queryKey: queryKeys.instance.experimentalSettings, queryFn: () => instanceSettingsApi.getExperimental(), - retry: false, }); return { isEnabled: query.data?.enableExternalObjects === true, @@ -172,12 +172,15 @@ export function useIssueExternalObjects(issueId: string | null | undefined): Iss void query.refetch(); }, [query.refetch]); + // Loaded objects stay on screen through an outage; only a real failure is + // reported as an error, and an outage before the first load counts as loading. + const viewKind = queryViewKind(query); return { isEnabled: externalObjectsFeature.isEnabled, groups, markdownReferences, - isLoading: enabled && query.isLoading, - isError: query.isError, + isLoading: enabled && (query.isLoading || viewKind === "reconnecting"), + isError: viewKind === "error", refetch, }; } diff --git a/ui/src/index.css b/ui/src/index.css index 195d5cee7b2..04b918dd2dd 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -393,6 +393,8 @@ --new-issue-visual-viewport-offset-top: 0px; --new-issue-dialog-top-gap: max(1rem, env(safe-area-inset-top)); --new-issue-dialog-bottom-gap: max(1rem, env(safe-area-inset-bottom)); + /* Connection banner offset: clear the notch/status bar under viewport-fit=cover. */ + --connection-banner-top: calc(0.5rem + env(safe-area-inset-top, 0px)); --mobile-entity-picker-visual-viewport-height: 100dvh; --mobile-entity-picker-backdrop: color-mix(in oklab, var(--foreground) 50%, transparent); diff --git a/ui/src/lib/connectivity.test.ts b/ui/src/lib/connectivity.test.ts new file mode 100644 index 00000000000..8fd2ef18907 --- /dev/null +++ b/ui/src/lib/connectivity.test.ts @@ -0,0 +1,284 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { ApiUnavailableError } from "@/api/response"; +import { createConnectivityStore, probeServerHealth, type ConnectivityStore, type ProbeResult } from "./connectivity"; + +const outage = new ApiError("Paperclip is restarting", 503, { error: "tenant_app_unavailable" }); +const down: ProbeResult = { reachable: false, retryAfterMs: null }; +const up: ProbeResult = { reachable: true }; + +describe("connectivity store", () => { + let probe: ReturnType Promise>>; + let store: ConnectivityStore; + + beforeEach(() => { + vi.useFakeTimers(); + probe = vi.fn<() => Promise>().mockResolvedValue(down); + store = createConnectivityStore({ probe, browserOnline: true }); + }); + + afterEach(() => { + store.dispose(); + vi.useRealTimers(); + }); + + const flush = () => vi.advanceTimersByTimeAsync(0); + + it("starts online", () => { + expect(store.getSnapshot()).toMatchObject({ status: "online", troubleSince: null, pendingWrites: 0 }); + }); + + it("confirms a connectivity error with a probe before reconnecting", async () => { + store.reportError(outage); + expect(store.getSnapshot().status).toBe("online"); + expect(probe).toHaveBeenCalledTimes(1); + await flush(); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(store.getSnapshot().troubleSince).not.toBeNull(); + }); + + it("stays online when the probe finds the server healthy (one busy route)", async () => { + probe.mockResolvedValue(up); + const listener = vi.fn(); + store.subscribe(listener); + store.reportError(new ApiUnavailableError(503)); + await flush(); + expect(store.getSnapshot().status).toBe("online"); + expect(listener).not.toHaveBeenCalled(); + // Repeated failures right after a clear probe do not re-probe. + store.reportError(new ApiUnavailableError(503)); + expect(probe).toHaveBeenCalledTimes(1); + }); + + it("ignores errors that are not server-wide", () => { + store.reportError(new ApiError("Too many requests", 429, null)); + store.reportError(new ApiError("Forbidden", 403, null)); + store.reportError(new ApiError("Plugin worker restarting", 503, { code: "WORKER_UNAVAILABLE", message: "x" })); + store.reportError(new TypeError("Cannot read properties of undefined")); + expect(probe).not.toHaveBeenCalled(); + }); + + it("probes with backoff 1s, 2s, 5s, 10s, then every 15s", async () => { + store.reportError(outage); + await flush(); + const probeTimes: number[] = []; + const start = Date.now(); + probe.mockImplementation(async () => { + probeTimes.push(Date.now() - start); + return down; + }); + await vi.advanceTimersByTimeAsync(1_000 + 2_000 + 5_000 + 10_000 + 15_000 + 15_000); + expect(probeTimes).toEqual([1_000, 3_000, 8_000, 18_000, 33_000, 48_000]); + }); + + it("honors a longer Retry-After", async () => { + probe.mockResolvedValueOnce({ reachable: false, retryAfterMs: 7_000 }); + store.reportError(outage); + await flush(); + await vi.advanceTimersByTimeAsync(6_999); + expect(probe).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(probe).toHaveBeenCalledTimes(2); + }); + + it("recovers on a successful probe and notifies recovery listeners once", async () => { + const recovered = vi.fn(); + store.onRecover(recovered); + store.reportError(outage); + await flush(); + probe.mockResolvedValue(up); + await vi.advanceTimersByTimeAsync(1_000); + expect(store.getSnapshot()).toMatchObject({ status: "online", troubleSince: null }); + expect(store.getSnapshot().recoveredAt).not.toBeNull(); + expect(recovered).toHaveBeenCalledTimes(1); + // No more probes once online. + await vi.advanceTimersByTimeAsync(60_000); + expect(probe).toHaveBeenCalledTimes(2); + expect(recovered).toHaveBeenCalledTimes(1); + }); + + it("recovers when any request succeeds during an outage", async () => { + const recovered = vi.fn(); + store.onRecover(recovered); + store.reportError(outage); + await flush(); + store.reportSuccess(); + expect(store.getSnapshot().status).toBe("online"); + expect(recovered).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(60_000); + expect(probe).toHaveBeenCalledTimes(1); + }); + + it("ignores a failed probe that started before a newer success", async () => { + store.reportError(outage); + await flush(); + let finishProbe: (result: ProbeResult) => void = () => undefined; + probe.mockImplementationOnce(() => new Promise((resolve) => { + finishProbe = resolve; + })); + await vi.advanceTimersByTimeAsync(1_000); + expect(probe).toHaveBeenCalledTimes(2); + + store.reportSuccess(); + expect(store.getSnapshot().status).toBe("online"); + finishProbe(down); + await flush(); + expect(store.getSnapshot().status).toBe("online"); + await vi.advanceTimersByTimeAsync(60_000); + expect(probe).toHaveBeenCalledTimes(2); + }); + + it("re-checks after a stale probe when something failed after the newer success", async () => { + let finishProbe: (result: ProbeResult) => void = () => undefined; + probe.mockImplementationOnce(() => new Promise((resolve) => { + finishProbe = resolve; + })); + store.reportError(outage); + store.reportSuccess(); + store.reportError(outage); + expect(probe).toHaveBeenCalledTimes(1); + + finishProbe(down); + await flush(); + expect(probe).toHaveBeenCalledTimes(2); + expect(store.getSnapshot().status).toBe("reconnecting"); + }); + + it("does not report recovery for a success while already online", () => { + const recovered = vi.fn(); + store.onRecover(recovered); + store.reportSuccess(); + expect(recovered).not.toHaveBeenCalled(); + }); + + it("probes immediately on probeNow during an outage", async () => { + store.reportError(outage); + await flush(); + probe.mockResolvedValue(up); + store.probeNow(); + await flush(); + expect(store.getSnapshot().status).toBe("online"); + expect(probe).toHaveBeenCalledTimes(2); + }); + + it("treats a closed socket as a suspicion and an open socket as success", async () => { + store.reportSocketStatus("closed"); + expect(probe).toHaveBeenCalledTimes(1); + await flush(); + expect(store.getSnapshot().status).toBe("reconnecting"); + store.reportSocketStatus("open"); + expect(store.getSnapshot().status).toBe("online"); + }); + + it("goes offline with the browser and confirms the server when the network returns", async () => { + store.setBrowserOnline(false); + expect(store.getSnapshot().status).toBe("offline"); + store.reportError(outage); + store.reportSuccess(); + expect(store.getSnapshot().status).toBe("offline"); + expect(probe).not.toHaveBeenCalled(); + + probe.mockResolvedValue(up); + store.setBrowserOnline(true); + expect(store.getSnapshot().status).toBe("reconnecting"); + await flush(); + expect(store.getSnapshot().status).toBe("online"); + }); + + it("keeps a starting server reconnecting until health reports ok", async () => { + store.reportServerStarting(); + expect(store.getSnapshot().status).toBe("reconnecting"); + // Other routes answering does not mean the server is ready. + store.reportSuccess(); + expect(store.getSnapshot().status).toBe("reconnecting"); + probe.mockResolvedValue(up); + await vi.advanceTimersByTimeAsync(1_000); + expect(store.getSnapshot().status).toBe("online"); + }); + + it("keeps probing when the server reports starting after a success while a probe is in flight", async () => { + let finishProbe: (result: ProbeResult) => void = () => undefined; + probe.mockImplementationOnce(() => new Promise((resolve) => { + finishProbe = resolve; + })); + store.reportError(outage); + // A health query answers `starting`: the cache reports the success first, + // then the gate reports the starting server. + store.reportSuccess(); + store.reportServerStarting(); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(probe).toHaveBeenCalledTimes(1); + + finishProbe(down); + await flush(); + expect(store.getSnapshot().status).toBe("reconnecting"); + probe.mockResolvedValue(up); + await vi.advanceTimersByTimeAsync(1_000); + expect(probe).toHaveBeenCalledTimes(2); + expect(store.getSnapshot().status).toBe("online"); + }); + + it("ignores a healthy probe that started before the server reported starting", async () => { + let finishProbe: (result: ProbeResult) => void = () => undefined; + probe.mockImplementationOnce(() => new Promise((resolve) => { + finishProbe = resolve; + })); + store.reportError(outage); + store.reportServerStarting(); + + finishProbe(up); + await flush(); + expect(store.getSnapshot().status).toBe("reconnecting"); + await vi.advanceTimersByTimeAsync(1_000); + expect(probe).toHaveBeenCalledTimes(2); + expect(store.getSnapshot().status).toBe("reconnecting"); + }); + + it("sums pending writes across sources", () => { + store.setPendingWrites("composer", 2); + store.setPendingWrites("autosave", 1); + expect(store.getSnapshot().pendingWrites).toBe(3); + store.setPendingWrites("composer", 0); + expect(store.getSnapshot().pendingWrites).toBe(1); + }); + + it("returns a stable snapshot until something changes", async () => { + const first = store.getSnapshot(); + store.reportSuccess(); + expect(store.getSnapshot()).toBe(first); + store.reportError(outage); + await flush(); + expect(store.getSnapshot()).not.toBe(first); + }); +}); + +describe("probeServerHealth", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it.each<[string, Response | Error, ProbeResult["reachable"]]>([ + ["ok health", Response.json({ status: "ok" }), true], + ["a starting server", Response.json({ status: "starting" }), false], + ["the gateway code", Response.json({ error: "tenant_app_unavailable" }, { status: 503 }), false], + ["the gateway code with a 404", Response.json({ error: "tenant_app_unavailable" }, { status: 404 }), false], + ["a proxy HTML page", new Response("Bad gateway", { status: 502 }), false], + ["an HTML fallback with 200", new Response("", { status: 200 }), false], + ["a 401 (server is up)", Response.json({ error: "Unauthorized" }, { status: 401 }), true], + ["a network failure", new TypeError("Failed to fetch"), false], + ])("reports %s", async (_label, outcome, reachable) => { + vi.stubGlobal("fetch", vi.fn(async () => { + if (outcome instanceof Error) throw outcome; + return outcome; + })); + expect((await probeServerHealth()).reachable).toBe(reachable); + }); + + it("passes Retry-After through", async () => { + vi.stubGlobal("fetch", vi.fn(async () => Response.json( + { error: "tenant_app_unavailable" }, + { status: 503, headers: { "Retry-After": "4" } }, + ))); + expect(await probeServerHealth()).toEqual({ reachable: false, retryAfterMs: 4_000 }); + }); +}); diff --git a/ui/src/lib/connectivity.ts b/ui/src/lib/connectivity.ts new file mode 100644 index 00000000000..5551ea05fca --- /dev/null +++ b/ui/src/lib/connectivity.ts @@ -0,0 +1,367 @@ +/** + * App-wide connectivity state. + * + * One small external store answers "can we reach the Paperclip server right + * now?" for the whole app. Inputs: + * + * - React Query cache events: a connectivity-level failure (network drop, proxy + * 5xx, gateway `tenant_app_unavailable`) makes the store suspicious. Cache + * success can come from another service, so only server health and the + * live-updates WebSocket confirm recovery. + * - The live-updates WebSocket opening (reachable) or closing (suspicious). + * - `navigator.onLine` and the window `online`/`offline` events. + * + * A suspicion is confirmed by probing `/api/health` before the state changes, + * so one busy route returning 503 cannot pause the whole app. While the state + * is not `online`, a single probe loop polls `/api/health` with backoff (1s, + * 2s, 5s, 10s, then every 15s; a longer `Retry-After` wins). It is the only + * outage poll in the app. + * + * `lib/query-client.ts` feeds this store into React Query's `onlineManager`, + * so queries and replayable mutations pause during an outage and resume on + * recovery instead of failing. + */ + +import { createContext, createElement, useContext, useEffect, useSyncExternalStore, type ReactNode } from "react"; +import { apiFetch } from "@/api/client"; +import { isConnectivityError, isTransientError, parseRetryAfter } from "@/api/errors"; + +export type ConnectivityStatus = "online" | "reconnecting" | "offline"; + +export interface ConnectivitySnapshot { + status: ConnectivityStatus; + /** When the current outage began (first failed request), or null while online. */ + troubleSince: number | null; + /** When the store last recovered from an outage. */ + recoveredAt: number | null; + /** Writes waiting for the connection, as reported by `usePendingWritesReporter`. */ + pendingWrites: number; +} + +export type ProbeResult = { reachable: true } | { reachable: false; retryAfterMs: number | null }; + +export interface ConnectivityStoreOptions { + /** Checks whether the server is reachable. Defaults to probing `/api/health`. */ + probe?: () => Promise; + now?: () => number; + /** Initial browser connectivity; defaults to `navigator.onLine`. */ + browserOnline?: boolean; +} + +/** Delay before each probe while reconnecting; the last value repeats. */ +export const PROBE_BACKOFF_MS = [1_000, 2_000, 5_000, 10_000, 15_000] as const; +/** Upper bound for a server-requested `Retry-After`. */ +export const MAX_PROBE_DELAY_MS = 60_000; +/** Do not re-check a suspicion this soon after a probe said the server is fine. */ +export const SUSPICION_COOLDOWN_MS = 2_000; +const PROBE_TIMEOUT_MS = 10_000; + +export interface ConnectivityStore { + getSnapshot(): ConnectivitySnapshot; + subscribe(listener: () => void): () => void; + /** Report a failed request. Only connectivity-level errors have an effect. */ + reportError(error: unknown): void; + /** Report a successful request: the server is reachable. */ + reportSuccess(): void; + /** + * Health says the server is `starting`: reachable, but not ready. Unlike an + * outage, ordinary successful requests do not end this state; only a health + * probe that reports `ok` does. + */ + reportServerStarting(): void; + reportSocketStatus(status: "open" | "closed"): void; + setBrowserOnline(online: boolean): void; + /** During an outage, probe now instead of waiting out the backoff step (a Retry button). */ + probeNow(): void; + /** Called once each time the store returns to `online` after an outage. */ + onRecover(listener: () => void): () => void; + setPendingWrites(sourceId: string, count: number): void; + /** Attach `online`/`offline` window listeners. Returns a cleanup. */ + start(): () => void; + /** Stop timers and drop listeners (tests). */ + dispose(): void; +} + +function readBrowserOnline(): boolean { + if (typeof navigator === "undefined" || typeof navigator.onLine !== "boolean") return true; + return navigator.onLine; +} + +export function createConnectivityStore(options: ConnectivityStoreOptions = {}): ConnectivityStore { + const probe = options.probe ?? probeServerHealth; + const now = options.now ?? Date.now; + const listeners = new Set<() => void>(); + const recoverListeners = new Set<() => void>(); + const pendingWriteSources = new Map(); + + let browserOnline = options.browserOnline ?? readBrowserOnline(); + let snapshot: ConnectivitySnapshot = { + status: browserOnline ? "online" : "offline", + troubleSince: browserOnline ? null : now(), + recoveredAt: null, + pendingWrites: 0, + }; + let probeTimer: ReturnType | null = null; + let probing = false; + let probeAttempt = 0; + /** First connectivity failure not yet confirmed or dismissed by a probe. */ + let suspectedSince: number | null = null; + let lastClearProbeAt = Number.NEGATIVE_INFINITY; + let serverStarting = false; + let disposed = false; + /** Bumped by every proof of reachability; a failed probe started before one is stale. */ + let recoveries = 0; + /** Bumped by every `starting` report; a reachable probe started before one is stale. */ + let startingReports = 0; + + const emit = () => { + for (const listener of [...listeners]) listener(); + }; + + const update = (next: Partial) => { + const merged = { ...snapshot, ...next }; + if ( + merged.status === snapshot.status + && merged.troubleSince === snapshot.troubleSince + && merged.recoveredAt === snapshot.recoveredAt + && merged.pendingWrites === snapshot.pendingWrites + ) return; + snapshot = merged; + emit(); + }; + + const clearProbeTimer = () => { + if (probeTimer !== null) { + clearTimeout(probeTimer); + probeTimer = null; + } + }; + + const recover = () => { + recoveries += 1; + clearProbeTimer(); + suspectedSince = null; + probeAttempt = 0; + if (snapshot.status === "online") return; + update({ status: "online", troubleSince: null, recoveredAt: now() }); + for (const listener of [...recoverListeners]) listener(); + }; + + const enterReconnecting = (since: number) => { + if (snapshot.status !== "online") return; + update({ status: "reconnecting", troubleSince: since }); + }; + + const scheduleProbe = (retryAfterMs: number | null) => { + clearProbeTimer(); + if (disposed || !browserOnline) return; + const backoff = PROBE_BACKOFF_MS[Math.min(probeAttempt, PROBE_BACKOFF_MS.length - 1)]; + const delay = retryAfterMs === null ? backoff : Math.min(Math.max(backoff, retryAfterMs), MAX_PROBE_DELAY_MS); + probeAttempt += 1; + probeTimer = setTimeout(() => { + probeTimer = null; + runProbe(); + }, delay); + }; + + const runProbe = () => { + if (disposed || probing || !browserOnline) return; + probing = true; + const recoveriesAtStart = recoveries; + const startingReportsAtStart = startingReports; + void probe() + .catch((): ProbeResult => ({ reachable: false, retryAfterMs: null })) + .then((result) => { + probing = false; + if (disposed || !browserOnline) return; + const stale = result.reachable + ? startingReports !== startingReportsAtStart + : recoveries !== recoveriesAtStart; + if (stale) { + // Newer evidence arrived while this probe was in flight (a success, + // or a `starting` report), so it wins over this result. If that left + // the app reconnecting, keep the loop alive: nothing else will + // probe. Otherwise check again only if something failed since. + if (snapshot.status !== "online") scheduleProbe(result.reachable ? null : result.retryAfterMs); + else if (suspectedSince !== null) runProbe(); + return; + } + if (result.reachable) { + if (snapshot.status === "online") lastClearProbeAt = now(); + serverStarting = false; + recover(); + return; + } + enterReconnecting(suspectedSince ?? now()); + suspectedSince = null; + scheduleProbe(result.retryAfterMs); + }); + }; + + const suspect = () => { + if (disposed || !browserOnline) return; + if (snapshot.status !== "online") return; // The probe loop is already running. + suspectedSince ??= now(); + if (probing) return; + if (now() - lastClearProbeAt < SUSPICION_COOLDOWN_MS) { + suspectedSince = null; + return; + } + runProbe(); + }; + + const reportSuccess = () => { + if (!browserOnline || serverStarting) return; + recover(); + }; + + const reportServerStarting = () => { + if (disposed || serverStarting) return; + serverStarting = true; + startingReports += 1; + if (!browserOnline) return; + enterReconnecting(now()); + if (probeTimer === null && !probing) scheduleProbe(null); + }; + + const setBrowserOnline = (online: boolean) => { + if (online === browserOnline) return; + browserOnline = online; + if (!online) { + clearProbeTimer(); + suspectedSince = null; + update({ status: "offline", troubleSince: snapshot.troubleSince ?? now() }); + return; + } + // The browser has a network again; confirm the server before trusting it. + update({ status: "reconnecting" }); + probeAttempt = 0; + clearProbeTimer(); + runProbe(); + }; + + return { + getSnapshot: () => snapshot, + subscribe(listener) { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + reportError(error) { + if (isConnectivityError(error)) suspect(); + }, + reportSuccess, + reportServerStarting, + reportSocketStatus(status) { + if (status === "open") reportSuccess(); + else suspect(); + }, + setBrowserOnline, + probeNow() { + if (snapshot.status === "online") return; + clearProbeTimer(); + runProbe(); + }, + onRecover(listener) { + recoverListeners.add(listener); + return () => { + recoverListeners.delete(listener); + }; + }, + setPendingWrites(sourceId, count) { + if (count > 0) pendingWriteSources.set(sourceId, count); + else pendingWriteSources.delete(sourceId); + let total = 0; + for (const value of pendingWriteSources.values()) total += value; + update({ pendingWrites: total }); + }, + start() { + if (typeof window === "undefined") return () => undefined; + const handleOnline = () => setBrowserOnline(true); + const handleOffline = () => setBrowserOnline(false); + window.addEventListener("online", handleOnline); + window.addEventListener("offline", handleOffline); + setBrowserOnline(readBrowserOnline()); + return () => { + window.removeEventListener("online", handleOnline); + window.removeEventListener("offline", handleOffline); + }; + }, + dispose() { + disposed = true; + clearProbeTimer(); + listeners.clear(); + recoverListeners.clear(); + }, + }; +} + +/** + * Probe `/api/health` directly (not through `healthApi`, which may trigger a + * tenant-session recovery redirect). Reachable means the server answered with + * a non-transient response; a `starting` server is not ready yet. + */ +export async function probeServerHealth(): Promise { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS); + try { + const res = await apiFetch("/api/health", { + credentials: "include", + headers: { Accept: "application/json" }, + cache: "no-store", + signal: controller.signal, + }); + const retryAfterMs = parseRetryAfter(res.headers.get("Retry-After")); + let body: unknown = null; + try { + body = await res.json(); + } catch { + // A proxy's HTML page: the app itself did not answer. + return res.ok || res.status >= 500 ? { reachable: false, retryAfterMs } : { reachable: true }; + } + if (res.ok) { + const status = body && typeof body === "object" ? (body as { status?: unknown }).status : undefined; + return status === "ok" ? { reachable: true } : { reachable: false, retryAfterMs }; + } + return isTransientError({ status: res.status, body }) + ? { reachable: false, retryAfterMs } + : { reachable: true }; + } catch { + return { reachable: false, retryAfterMs: null }; + } finally { + clearTimeout(timeout); + } +} + +/** The app's connectivity store. */ +export const connectivity = createConnectivityStore(); + +const ConnectivityContext = createContext(connectivity); + +/** Override the store for a subtree (tests, Storybook). The app uses the singleton. */ +export function ConnectivityProvider({ store, children }: { store: ConnectivityStore; children?: ReactNode }) { + return createElement(ConnectivityContext.Provider, { value: store }, children); +} + +export function useConnectivityStore(): ConnectivityStore { + return useContext(ConnectivityContext); +} + +export function useConnectivity(): ConnectivitySnapshot { + const store = useConnectivityStore(); + return useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot); +} + +/** + * Report writes that are waiting for the connection (drafts queued to resend, + * autosaves held back). The banner shows the total. Durable write queues + * feed this; `bindConnectivity` counts paused replayable mutations. + */ +export function usePendingWritesReporter(sourceId: string, count: number): void { + const store = useConnectivityStore(); + useEffect(() => { + store.setPendingWrites(sourceId, count); + }, [store, sourceId, count]); + useEffect(() => () => store.setPendingWrites(sourceId, 0), [store, sourceId]); +} diff --git a/ui/src/lib/interaction-resolution-error.test.ts b/ui/src/lib/interaction-resolution-error.test.ts index d9352e813e8..de1e9c7ad6a 100644 --- a/ui/src/lib/interaction-resolution-error.test.ts +++ b/ui/src/lib/interaction-resolution-error.test.ts @@ -147,7 +147,27 @@ describe("describeInteractionResolutionFailure", () => { it("keeps the retry prompt for a genuinely transient failure", () => { expect( describeInteractionResolutionFailure(new ApiError("Request failed: 503", 503, null), humanOnlyAudience), - ).toMatchObject({ kind: "transient", message: "Request failed: 503. Try again." }); + ).toMatchObject({ + kind: "transient", + message: "Paperclip is temporarily unavailable. Please try again in a moment.", + }); + }); + + it("never shows a raw gateway code", () => { + const failure = describeInteractionResolutionFailure( + new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }), + humanOnlyAudience, + ); + expect(failure.kind).toBe("transient"); + expect(failure.code).toBeNull(); + expect(failure.message).not.toMatch(/tenant_app_unavailable/); + expect(failure.message).toMatch(/try again/i); + }); + + it("does not echo a raw code for a non-transient failure", () => { + expect( + interactionResolutionErrorMessage(new ApiError("internal_error", 500, { error: "internal_error" }), null), + ).toBe("Couldn't submit. Try again."); }); it("falls back to generic copy when nothing explains the failure", () => { diff --git a/ui/src/lib/interaction-resolution-error.ts b/ui/src/lib/interaction-resolution-error.ts index 3babed17d5a..8320048012d 100644 --- a/ui/src/lib/interaction-resolution-error.ts +++ b/ui/src/lib/interaction-resolution-error.ts @@ -12,6 +12,7 @@ * action is permitted, it only explains the answer the server already gave. */ +import { describeError, errorCopy, isRawErrorCode, isTransientError } from "@/api/errors"; import type { InteractionAudienceDescription } from "./interaction-audience"; /** @@ -71,12 +72,15 @@ function errorStatus(error: unknown): number | null { return typeof status === "number" ? status : null; } -/** The server's human-readable reason, preserved verbatim apart from punctuation. */ +/** + * The server's human-readable reason, preserved verbatim apart from + * punctuation. A raw code (`tenant_app_unavailable`) is not a reason. + */ function serverReason(error: unknown): string | null { const body = record(record(error)?.body); const fromBody = typeof body?.error === "string" ? body.error.trim() : ""; const fromError = error instanceof Error ? error.message.trim() : ""; - const reason = fromBody || fromError; + const reason = [fromBody, fromError].find((text) => text && !isRawErrorCode(text)); if (!reason) return null; // The API writes reasons as bare clauses ("This interaction is human-only"). return /[.!?]$/.test(reason) ? reason : `${reason}.`; @@ -130,8 +134,8 @@ export function describeInteractionResolutionFailure( message: [ reason ?? (coded - ? "You are not in this card's resolver audience." - : "You do not have permission to respond to this card."), + ? errorCopy("interaction_audience_denied").body + : errorCopy("interaction_forbidden").body), responder, ] .filter(Boolean) @@ -143,14 +147,20 @@ export function describeInteractionResolutionFailure( return { kind: "settled", code, - message: reason ?? "This request is no longer waiting for a decision.", + message: reason ?? errorCopy("interaction_settled").body, }; } + // An outage or gateway blip: the server's text is a status line or a raw + // code, so use the shared copy (which already invites a retry). + if (isTransientError(error)) { + return { kind: "transient", code, message: describeError(error).body }; + } + return { kind: "transient", code, - message: reason ? `${reason} Try again.` : "Couldn't submit. Try again.", + message: reason ? `${reason} Try again.` : errorCopy("interaction_submit_failed").body, }; } diff --git a/ui/src/lib/query-client.test.ts b/ui/src/lib/query-client.test.ts new file mode 100644 index 00000000000..ab18ae063fb --- /dev/null +++ b/ui/src/lib/query-client.test.ts @@ -0,0 +1,278 @@ +import { onlineManager, QueryObserver, MutationObserver, type QueryClient } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { ApiUnavailableError } from "@/api/response"; +import { createConnectivityStore, type ConnectivityStore, type ProbeResult } from "./connectivity"; +import { + bindConnectivity, + createAppQueryClient, + reconnectDelayFor, + retryDelayFor, + shouldReportQueryError, + shouldRetryRequest, + TRANSIENT_RETRY_DELAYS_MS, + type MutationErrorToast, +} from "./query-client"; + +const gatewayDown = () => new ApiError("Paperclip is restarting", 503, { error: "tenant_app_unavailable" }); + +describe("retry policy", () => { + it("retries transient errors for about 30 seconds", () => { + const error = new ApiUnavailableError(502); + const retries = TRANSIENT_RETRY_DELAYS_MS.length; + expect(shouldRetryRequest(retries - 1, error)).toBe(true); + expect(shouldRetryRequest(retries, error)).toBe(false); + const total = TRANSIENT_RETRY_DELAYS_MS.reduce((sum, delay) => sum + delay, 0); + expect(total).toBeGreaterThanOrEqual(25_000); + expect(total).toBeLessThanOrEqual(35_000); + }); + + it.each([400, 401, 403, 404, 409, 422])("never retries a %s", (status) => { + expect(shouldRetryRequest(0, new ApiError("No", status, null))).toBe(false); + }); + + it("retries the gateway code even when it arrives with a 4xx", () => { + expect(shouldRetryRequest(0, new ApiError("x", 404, { error: "tenant_app_unavailable" }))).toBe(true); + }); + + it("retries unexpected failures briefly and aborts never", () => { + expect(shouldRetryRequest(0, new ApiError("boom", 500, null))).toBe(true); + expect(shouldRetryRequest(2, new ApiError("boom", 500, null))).toBe(false); + expect(shouldRetryRequest(0, new DOMException("Aborted", "AbortError"))).toBe(false); + }); + + it("backs off and honors Retry-After", () => { + const error = new ApiUnavailableError(503); + expect(retryDelayFor(0, error)).toBe(1_000); + expect(retryDelayFor(4, error)).toBe(15_000); + expect(retryDelayFor(9, error)).toBe(15_000); + const throttled = new ApiError("Slow down", 429, null, { retryAfterMs: 6_000 }); + expect(retryDelayFor(0, throttled)).toBe(6_000); + // A rate-limit window longer than the normal backoff is waited out in full. + const minuteWindow = new ApiError("Slow down", 429, null, { retryAfterMs: 55_000 }); + expect(retryDelayFor(0, minuteWindow)).toBe(55_000); + expect(retryDelayFor(0, new ApiUnavailableError(503, 20_000))).toBe(20_000); + expect(reconnectDelayFor(0, throttled)).toBe(6_000); + expect(reconnectDelayFor(3, new ApiUnavailableError(503))).toBe(10_000); + }); + + it("reports only unexpected query errors to Sentry", () => { + expect(shouldReportQueryError(new ApiError("boom", 500, null))).toBe(true); + expect(shouldReportQueryError(new Error("parse failure"))).toBe(true); + expect(shouldReportQueryError(gatewayDown())).toBe(false); + expect(shouldReportQueryError(new TypeError("Failed to fetch"))).toBe(false); + expect(shouldReportQueryError(new ApiError("Not found", 404, null))).toBe(false); + }); +}); + +describe("createAppQueryClient", () => { + let store: ConnectivityStore; + let probe: ReturnType Promise>>; + let client: QueryClient; + let unbind: () => void; + let notifyMutationError: ReturnType void>>; + let reportError: ReturnType void>>; + + beforeEach(() => { + vi.useFakeTimers(); + probe = vi.fn<() => Promise>().mockResolvedValue({ reachable: false, retryAfterMs: null }); + store = createConnectivityStore({ probe, browserOnline: true }); + notifyMutationError = vi.fn<(toast: MutationErrorToast) => void>(); + reportError = vi.fn<(error: unknown) => void>(); + client = createAppQueryClient({ connectivity: store, notifyMutationError, reportError }); + client.mount(); + unbind = bindConnectivity(client, store); + }); + + afterEach(() => { + unbind(); + store.dispose(); + client.unmount(); + client.clear(); + onlineManager.setOnline(true); + vi.useRealTimers(); + }); + + it("pauses a failing query during an outage and refreshes it on recovery", async () => { + let down = true; + const queryFn = vi.fn(async () => { + if (down) throw gatewayDown(); + return "fresh"; + }); + const observer = new QueryObserver(client, { queryKey: ["issue", "1"], queryFn }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(onlineManager.isOnline()).toBe(false); + + await vi.advanceTimersByTimeAsync(30_000); + // Paused, not failed, and not hammering the server. + expect(observer.getCurrentResult()).toMatchObject({ status: "pending", fetchStatus: "paused" }); + expect(queryFn).toHaveBeenCalledTimes(1); + + down = false; + probe.mockResolvedValue({ reachable: true }); + await vi.advanceTimersByTimeAsync(15_000); + expect(store.getSnapshot().status).toBe("online"); + expect(observer.getCurrentResult()).toMatchObject({ status: "success", data: "fresh" }); + unsubscribe(); + }); + + it("keeps cached data when a background refetch hits an outage", async () => { + let down = false; + const observer = new QueryObserver(client, { + queryKey: ["issue", "2"], + queryFn: async () => { + if (down) throw gatewayDown(); + return "loaded"; + }, + }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + down = true; + void observer.refetch(); + await vi.advanceTimersByTimeAsync(5_000); + expect(observer.getCurrentResult()).toMatchObject({ data: "loaded", error: null }); + unsubscribe(); + }); + + it("refreshes active queries once on recovery", async () => { + const queryFn = vi.fn(async () => "ok"); + const observer = new QueryObserver(client, { queryKey: ["list"], queryFn, staleTime: Infinity }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + expect(queryFn).toHaveBeenCalledTimes(1); + store.reportError(gatewayDown()); + await vi.advanceTimersByTimeAsync(0); + probe.mockResolvedValue({ reachable: true }); + await vi.advanceTimersByTimeAsync(1_000); + expect(queryFn).toHaveBeenCalledTimes(2); + unsubscribe(); + }); + + it("does not treat setQueryData as proof the server is back", async () => { + store.reportError(gatewayDown()); + await vi.advanceTimersByTimeAsync(0); + client.setQueryData(["optimistic"], "local"); + expect(store.getSnapshot().status).toBe("reconnecting"); + }); + + it("does not infer server recovery from an unrelated in-flight query", async () => { + let finish!: (value: null) => void; + const pending = new Promise(resolve => { finish = resolve; }); + const observer = new QueryObserver(client, { queryKey: ["npm-registry"], queryFn: () => pending }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + store.reportError(gatewayDown()); + await vi.advanceTimersByTimeAsync(0); + expect(store.getSnapshot().status).toBe("reconnecting"); + finish(null); + await vi.advanceTimersByTimeAsync(0); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(onlineManager.isOnline()).toBe(false); + probe.mockResolvedValue({ reachable: true }); + await vi.advanceTimersByTimeAsync(1_000); + expect(store.getSnapshot().status).toBe("online"); + unsubscribe(); + }); + + it("does not infer server recovery from an unrelated mutation", async () => { + store.reportError(gatewayDown()); + await vi.advanceTimersByTimeAsync(0); + const observer = new MutationObserver(client, { mutationFn: async () => "external success" }); + await observer.mutate(); + expect(store.getSnapshot().status).toBe("reconnecting"); + expect(onlineManager.isOnline()).toBe(false); + }); + + it("fails an ordinary mutation fast during an outage", async () => { + const mutationFn = vi.fn(async () => { + throw gatewayDown(); + }); + const observer = new MutationObserver(client, { mutationFn }); + await expect(observer.mutate()).rejects.toBeInstanceOf(ApiError); + expect(mutationFn).toHaveBeenCalledTimes(1); + }); + + it("pauses an idempotent mutation across an outage and resends it on reconnect", async () => { + let down = true; + const mutationFn = vi.fn(async (value: string) => { + if (down) throw gatewayDown(); + return value; + }); + const observer = new MutationObserver(client, { mutationFn, meta: { replay: "idempotent" } }); + const result = observer.mutate("title"); + await vi.advanceTimersByTimeAsync(60_000); + expect(observer.getCurrentResult()).toMatchObject({ status: "pending", isPaused: true }); + expect(mutationFn).toHaveBeenCalledTimes(1); + expect(store.getSnapshot().pendingWrites).toBe(1); + + down = false; + probe.mockResolvedValue({ reachable: true }); + await vi.advanceTimersByTimeAsync(15_000); + await expect(result).resolves.toBe("title"); + expect(mutationFn).toHaveBeenCalledTimes(2); + expect(store.getSnapshot().pendingWrites).toBe(0); + }); + + it("toasts readable copy for a failed opted-in mutation without its own handler", async () => { + const observer = new MutationObserver(client, { + mutationFn: async () => { + throw new ApiError("Title is required", 422, { error: "Title is required" }); + }, + meta: { errorToast: true }, + }); + await observer.mutate().catch(() => undefined); + expect(notifyMutationError).toHaveBeenCalledWith({ title: "Check your input", body: "Title is required" }); + }); + + it("toasts a transient failure the banner does not cover", async () => { + const observer = new MutationObserver(client, { + mutationFn: async () => { + throw new ApiError("Slow down", 429, null); + }, + meta: { errorToast: true }, + }); + await observer.mutate().catch(() => undefined); + expect(notifyMutationError).toHaveBeenCalledWith({ + title: "Connection interrupted", + body: "Paperclip is temporarily unavailable. Please try again in a moment.", + }); + }); + + it("does not toast covered transient failures, handled mutations, or mutations that did not opt in", async () => { + const fail = (error: unknown) => async () => { + throw error; + }; + store.reportError(gatewayDown()); + await vi.advanceTimersByTimeAsync(0); + expect(store.getSnapshot().status).toBe("reconnecting"); + await new MutationObserver(client, { + mutationFn: fail(gatewayDown()), + meta: { errorToast: true }, + }).mutate().catch(() => undefined); + await new MutationObserver(client, { + mutationFn: fail(new ApiError("No", 403, null)), + onError: () => undefined, + meta: { errorToast: true }, + }).mutate().catch(() => undefined); + // Surfaces that render `mutation.error` inline must not get a second copy. + await new MutationObserver(client, { + mutationFn: fail(new ApiError("No", 403, null)), + }).mutate().catch(() => undefined); + expect(notifyMutationError).not.toHaveBeenCalled(); + }); + + it("reports an unexpected query error to Sentry once per query", async () => { + const queryFn = vi.fn(async () => { + throw new ApiError("boom", 500, { error: "boom" }); + }); + const observer = new QueryObserver(client, { queryKey: ["broken"], queryFn, retry: false }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + await observer.refetch(); + expect(queryFn).toHaveBeenCalledTimes(2); + expect(reportError).toHaveBeenCalledTimes(1); + unsubscribe(); + }); +}); diff --git a/ui/src/lib/query-client.ts b/ui/src/lib/query-client.ts new file mode 100644 index 00000000000..f3a541b016f --- /dev/null +++ b/ui/src/lib/query-client.ts @@ -0,0 +1,238 @@ +/** + * The app's React Query client and its outage-aware defaults. + * + * - Queries retry transient errors with backoff for about 30 seconds and never + * retry a 4xx. `networkMode: "online"` plus `bindConnectivity` means a + * confirmed outage pauses queries instead of failing them. + * - Mutations fail fast by default (`networkMode: "always"`, no retry). A + * mutation that is safe to send twice opts in with + * `meta: { replay: "idempotent" }`: it pauses while the server is + * unreachable, retries transient errors, and resumes on reconnect. + * - `QueryCache`/`MutationCache` events feed the connectivity store; final + * query errors go to Sentry unless they are transient or client errors. + * - A mutation with `meta: { errorToast: true }` and no `onError` of its own + * gets a readable toast when it fails. Transient failures skip the toast + * while the connection banner covers them. + */ + +import { + MutationCache, + QueryCache, + QueryClient, + onlineManager, + type DefaultError, + type Mutation, + type MutationOptions, + type QueryClientConfig, +} from "@tanstack/react-query"; +import { classifyError, describeError, errorRetryAfterMs, isTransientError } from "@/api/errors"; +import { MAX_PROBE_DELAY_MS, PROBE_BACKOFF_MS, type ConnectivityStore } from "./connectivity"; + +export interface AppMutationMeta extends Record { + /** + * `"idempotent"`: sending this mutation twice has the same effect as once + * (an absolute-value PATCH, a POST with an idempotency key). It may pause + * across an outage and resend on reconnect. + */ + replay?: "idempotent"; + /** + * `true` shows the global readable error toast when this mutation has no + * `onError` of its own. Opt-in because many surfaces already render + * `mutation.error` inline, and a default toast would say it twice. + */ + errorToast?: true; +} + +declare module "@tanstack/react-query" { + interface Register { + mutationMeta: AppMutationMeta; + } +} + +/** Delay before each transient retry; together roughly 30 seconds. */ +export const TRANSIENT_RETRY_DELAYS_MS = [1_000, 2_000, 4_000, 8_000, 15_000] as const; +/** Unexpected failures (5xx other than gateway errors, parse errors) retry briefly. */ +export const UNKNOWN_RETRY_LIMIT = 2; + +/** Retry policy shared by queries and replayable mutations. */ +export function shouldRetryRequest(failureCount: number, error: unknown): boolean { + const kind = classifyError(error); + if (kind === "transient") return failureCount < TRANSIENT_RETRY_DELAYS_MS.length; + if (kind === "unknown") return failureCount < UNKNOWN_RETRY_LIMIT; + return false; +} + +/** + * Backoff for `shouldRetryRequest`; a longer server `Retry-After` wins. It is + * not shortened: retrying before the server's window ends only fails again. + * `parseRetryAfter` already bounds it. + */ +export function retryDelayFor(failureCount: number, error: unknown): number { + const base = TRANSIENT_RETRY_DELAYS_MS[Math.min(failureCount, TRANSIENT_RETRY_DELAYS_MS.length - 1)]; + const retryAfterMs = errorRetryAfterMs(error); + return retryAfterMs === null ? base : Math.max(base, retryAfterMs); +} + +/** + * For reads the app cannot open without (the access gate): retry transient + * errors without limit, never anything else. Retries pause while the + * connectivity store reports an outage, so this does not poll a down server. + */ +export function retryWhileTransient(_failureCount: number, error: unknown): boolean { + return isTransientError(error); +} + +/** + * For the few reads that must fail fast: retry a transient error at most + * `limit` times (`0` never retries) and never anything else. Prefer this over + * `retry: false` so the call site says why it opts out of the default policy. + */ +export function retryTransientOnly(limit: number): (failureCount: number, error: unknown) => boolean { + return (failureCount, error) => isTransientError(error) && failureCount < limit; +} + +/** Backoff for `retryWhileTransient`: the connectivity probe schedule, capped at 15s. */ +export function reconnectDelayFor(failureCount: number, error: unknown): number { + const base = PROBE_BACKOFF_MS[Math.min(failureCount, PROBE_BACKOFF_MS.length - 1)]; + const retryAfterMs = errorRetryAfterMs(error); + return retryAfterMs === null ? base : Math.min(Math.max(base, retryAfterMs), MAX_PROBE_DELAY_MS); +} + +/** Query errors worth a Sentry event: not outages, not denials or missing items. */ +export function shouldReportQueryError(error: unknown): boolean { + return classifyError(error) === "unknown"; +} + +export interface MutationErrorToast { + title: string; + body: string; +} + +export interface AppQueryClientDeps { + connectivity: Pick; + reportError?: (error: unknown) => void; + /** Show a toast for a failed opted-in mutation that has no handler of its own. */ + notifyMutationError?: (toast: MutationErrorToast) => void; +} + +/** + * Whether the global handler should toast for this failed mutation. A + * transient failure is skipped only while the connection banner covers it; a + * 429 or one route's worker being down leaves the app online, and the write is + * still lost. + */ +export function shouldToastMutationError( + error: unknown, + mutation: Pick, "options" | "meta">, + connectionDown: boolean, +): boolean { + if (mutation.options.onError) return false; + if (mutation.meta?.errorToast !== true) return false; + const kind = classifyError(error); + if (kind === "aborted") return false; + return kind !== "transient" || !connectionDown; +} + +class AppQueryClient extends QueryClient { + override defaultMutationOptions>(options?: T): T { + const defaulted = super.defaultMutationOptions(options); + if (options?._defaulted || defaulted.meta?.replay !== "idempotent") return defaulted; + return { + ...defaulted, + networkMode: options?.networkMode ?? "online", + retry: options?.retry ?? shouldRetryRequest, + retryDelay: options?.retryDelay ?? retryDelayFor, + }; + } +} + +/** A polling query that keeps failing reports once per window, not on every poll. */ +const QUERY_ERROR_REPORT_WINDOW_MS = 10 * 60_000; + +export function createAppQueryClient(deps: AppQueryClientDeps, config: QueryClientConfig = {}): QueryClient { + const lastReportedAt = new Map(); + const queryCache = new QueryCache({ + onError: (error, query) => { + if (!deps.reportError || !shouldReportQueryError(error)) return; + const now = Date.now(); + const last = lastReportedAt.get(query.queryHash); + if (last !== undefined && now - last < QUERY_ERROR_REPORT_WINDOW_MS) return; + lastReportedAt.set(query.queryHash, now); + deps.reportError(error); + }, + }); + const mutationCache = new MutationCache({ + onError: (error, _variables, _context, mutation) => { + const connectionDown = deps.connectivity.getSnapshot().status !== "online"; + if (!deps.notifyMutationError || !shouldToastMutationError(error, mutation, connectionDown)) return; + const { title, body } = describeError(error); + deps.notifyMutationError({ title, body }); + }, + }); + + // Retry failures are only visible as cache events, and connectivity needs + // the first failure, not the last one. + queryCache.subscribe((event) => { + if (event.type !== "updated") return; + if (event.action.type === "failed" || event.action.type === "error") deps.connectivity.reportError(event.action.error); + // Queries can read other services or return local data. Only the health + // probe and the server WebSocket can prove that Paperclip recovered. + }); + mutationCache.subscribe((event) => { + if (event.type !== "updated") return; + if (event.action.type === "failed" || event.action.type === "error") deps.connectivity.reportError(event.action.error); + }); + + return new AppQueryClient({ + ...config, + queryCache, + mutationCache, + defaultOptions: { + ...config.defaultOptions, + queries: { + retry: shouldRetryRequest, + retryDelay: retryDelayFor, + networkMode: "online", + ...config.defaultOptions?.queries, + }, + mutations: { + networkMode: "always", + retry: 0, + ...config.defaultOptions?.mutations, + }, + }, + }); +} + +/** `setPendingWrites` source for replayable mutations paused by an outage. */ +export const PAUSED_MUTATIONS_SOURCE = "paused-mutations"; + +/** + * Drive React Query's `onlineManager` from the connectivity store, refresh + * live queries once when the server comes back, and count paused replayable + * mutations as pending writes. Returns a cleanup. + */ +export function bindConnectivity(queryClient: QueryClient, store: ConnectivityStore): () => void { + const isOnline = () => store.getSnapshot().status === "online"; + onlineManager.setEventListener((setOnline) => { + setOnline(isOnline()); + return store.subscribe(() => setOnline(isOnline())); + }); + const stopRecover = store.onRecover(() => { + void queryClient.invalidateQueries({ type: "active" }, { cancelRefetch: false }); + }); + const mutationCache = queryClient.getMutationCache(); + const reportPausedMutations = () => { + const paused = mutationCache.findAll({ predicate: (mutation) => mutation.state.isPaused }).length; + store.setPendingWrites(PAUSED_MUTATIONS_SOURCE, paused); + }; + reportPausedMutations(); + const stopMutationCount = mutationCache.subscribe(reportPausedMutations); + const stopBrowserEvents = store.start(); + return () => { + stopRecover(); + stopMutationCount(); + store.setPendingWrites(PAUSED_MUTATIONS_SOURCE, 0); + stopBrowserEvents(); + }; +} diff --git a/ui/src/main.tsx b/ui/src/main.tsx index 8bc5dba71c6..52840fb7157 100644 --- a/ui/src/main.tsx +++ b/ui/src/main.tsx @@ -1,10 +1,10 @@ import { VoiceCallProvider } from "./components/voice/VoiceCallProvider"; import { useAccountIdentity } from "./api/companies-query"; import * as React from "react"; -import { StrictMode } from "react"; +import { StrictMode, useEffect } from "react"; import * as ReactDOM from "react-dom"; import { BrowserRouter } from "@/lib/router"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { QueryClientProvider } from "@tanstack/react-query"; import { App } from "./App"; import { AppErrorBoundary } from "./components/AppErrorBoundary"; import { SentryGate } from "./components/SentryGate"; @@ -16,7 +16,7 @@ import { SidebarProvider } from "./context/SidebarContext"; import { DialogProvider } from "./context/DialogContext"; import { EditorAutocompleteProvider } from "./context/EditorAutocompleteContext"; import { PrimaryAgentProvider } from "./context/PrimaryAgentProvider"; -import { ToastProvider } from "./context/ToastContext"; +import { ToastProvider, useToastActions, type ToastInput } from "./context/ToastContext"; import { ThemeProvider } from "./context/ThemeContext"; import { DesignSandboxProvider } from "./context/DesignSandboxContext"; import { TooltipProvider } from "@/components/ui/tooltip"; @@ -26,6 +26,9 @@ import { startChunkLoadRecovery } from "./lib/chunk-load-recovery"; import { startPerfMeasureReaper } from "./lib/perf-measure-reaper"; import { getOrCreatePaperclipReactRoot } from "./lib/react-root"; import { startServiceWorkerUpdates } from "./lib/service-worker-updates"; +import { connectivity } from "./lib/connectivity"; +import { bindConnectivity, createAppQueryClient } from "./lib/query-client"; +import { captureBrowserException } from "./lib/sentry"; import "@mdxeditor/editor/style.css"; import "./index.css"; @@ -46,18 +49,45 @@ window.addEventListener("load", () => { startServiceWorkerUpdates(); }); -const queryClient = new QueryClient({ - defaultOptions: { - queries: { - staleTime: 30_000, - // Explicit so cross-tab-published cache entries for resources this tab - // isn't observing get collected promptly rather than lingering. Single - // tuning point if we need to trim the cache footprint further. - gcTime: 5 * 60_000, - refetchOnWindowFocus: true, +// The global mutation error toast is raised from the MutationCache, outside +// React; the bridge below hands it the ToastProvider's pushToast. +let pushMutationErrorToast: ((input: ToastInput) => void) | null = null; + +function MutationErrorToastBridge() { + const { pushToast } = useToastActions(); + useEffect(() => { + pushMutationErrorToast = pushToast; + return () => { + if (pushMutationErrorToast === pushToast) pushMutationErrorToast = null; + }; + }, [pushToast]); + return null; +} + +// Retry, pause-on-outage, and error-reporting defaults live in +// lib/query-client.ts; app-specific cache tuning stays here. +const queryClient = createAppQueryClient( + { + connectivity, + reportError: (error) => captureBrowserException(error), + notifyMutationError: ({ title, body }) => pushMutationErrorToast?.({ title, body, tone: "error" }), + }, + { + defaultOptions: { + queries: { + staleTime: 30_000, + // Explicit so cross-tab-published cache entries for resources this tab + // isn't observing get collected promptly rather than lingering. Single + // tuning point if we need to trim the cache footprint further. + gcTime: 5 * 60_000, + refetchOnWindowFocus: true, + }, }, }, -}); +); +// Outages pause queries and replayable mutations instead of failing them, and +// live queries refresh once when the server answers again. +bindConnectivity(queryClient, connectivity); function CompanyAwareBreadcrumbProvider({ children }: { children: React.ReactNode }) { const { selectedCompany } = useCompany(); @@ -83,6 +113,7 @@ getOrCreatePaperclipReactRoot(window, rootElement).render( + diff --git a/ui/src/pages/AgentChat.test.tsx b/ui/src/pages/AgentChat.test.tsx new file mode 100644 index 00000000000..4708b55d633 --- /dev/null +++ b/ui/src/pages/AgentChat.test.tsx @@ -0,0 +1,95 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import type { Agent } from "@paperclipai/shared"; +import { ApiError } from "@/api/client"; +import { queryKeys } from "@/lib/queryKeys"; +import { AgentChat } from "./AgentChat"; + +const api = vi.hoisted(() => ({ list: vi.fn(), get: vi.fn(), getChat: vi.fn(), ensure: vi.fn(), session: vi.fn() })); +const agentId = "18c5ad50-9d70-4e24-bd82-337e1d45a414"; +vi.mock("@/api/agents", () => ({ agentsApi: { list: api.list, get: api.get } })); +vi.mock("@/api/agentChats", () => ({ agentChatsApi: { get: api.getChat, ensure: api.ensure } })); +vi.mock("@/api/auth", () => ({ authApi: { getSession: api.session } })); +vi.mock("@/context/CompanyContext", () => ({ useCompany: () => ({ selectedCompanyId: "company-a" }) })); +vi.mock("@/hooks/useAgentChatEnabled", () => ({ useAgentChatEnabled: () => ({ enabled: true, loaded: true }) })); +vi.mock("@/lib/router", () => ({ useParams: () => ({ agentRef: "18c5ad50-9d70-4e24-bd82-337e1d45a414" }) })); +vi.mock("./IssueDetail", () => ({ TaskDetailSurface: ({ conversation }: { conversation: { agent: Agent } }) =>
History for {conversation.agent.name}
})); + +let root: Root; +let container: HTMLDivElement; +let client: QueryClient; +async function settle() { + await new Promise(resolve => setTimeout(resolve, 0)); + flushSync(() => {}); +} +async function waitFor(assertion: () => void) { + for (let attempt = 0; attempt < 50; attempt++) { + try { assertion(); return; } catch (error) { if (attempt === 49) throw error; await settle(); } + } +} +function render() { + flushSync(() => root.render()); +} +async function failRoster(status: number, cached = true) { + if (cached) client.setQueryData(queryKeys.agents.list("company-a"), []); + api.list.mockRejectedValue(new ApiError(status === 429 ? "Rate limited" : "Denied", status, {})); + await client.fetchQuery({ queryKey: queryKeys.agents.list("company-a"), queryFn: api.list, staleTime: 0 }).catch(() => {}); + expect(client.getQueryState(queryKeys.agents.list("company-a"))?.status).toBe("error"); +} +beforeEach(() => { + vi.clearAllMocks(); + client = new QueryClient({ defaultOptions: { queries: { retry: false, retryOnMount: false, staleTime: Infinity, gcTime: Infinity } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "viewer-a" } }); + api.session.mockResolvedValue({ user: { id: "viewer-a" } }); + api.get.mockResolvedValue({ id: agentId, companyId: "company-a", name: "Retired helper", status: "terminated" }); + api.getChat.mockResolvedValue({ id: "existing-chat" }); + container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container); +}); +afterEach(() => { flushSync(() => root.unmount()); client.clear(); container.remove(); }); + +it.each(["cached_refetch_failed", "fresh"])("opens historical chat from an available roster (%s)", async state => { + if (state === "cached_refetch_failed") await failRoster(429); + else client.setQueryData(queryKeys.agents.list("company-a"), []); + render(); + await waitFor(() => expect(container.textContent).toContain("History for Retired helper")); + expect(api.get).toHaveBeenCalledWith(agentId, "company-a"); + expect(api.getChat).toHaveBeenCalledWith("company-a", agentId); + expect(container.textContent).not.toContain("Agent not found"); + expect(api.ensure).not.toHaveBeenCalled(); +}); + +it("offers a working retry when the historical lookup also fails", async () => { + await failRoster(429); + api.get.mockRejectedValue(new ApiError("Rate limited", 429, {})); + render(); + await waitFor(() => expect([...container.querySelectorAll("button")].some(button => button.textContent?.trim() === "Retry")).toBe(true)); + expect(container.textContent).not.toContain("Agent not found"); + api.get.mockResolvedValue({ id: agentId, companyId: "company-a", name: "Retired helper" }); + flushSync(() => [...container.querySelectorAll("button")].find(button => button.textContent?.trim() === "Retry")!.click()); + await waitFor(() => expect(container.textContent).toContain("History for Retired helper")); + expect(api.ensure).not.toHaveBeenCalled(); +}); + +it.each([[403, true], [429, false]] as const)("retains roster error controls without starting a historical lookup (%s, cached %s)", async (status, cached) => { + await failRoster(status, cached); + render(); + await waitFor(() => expect(container.querySelector('[role="alert"]')).not.toBeNull()); + expect([...container.querySelectorAll("button")].some(button => button.textContent?.trim() === "Retry")).toBe(status === 429); + expect(api.get).not.toHaveBeenCalled(); + expect(api.getChat).not.toHaveBeenCalled(); + expect(container.textContent).not.toContain("Agent not found"); +}); + +it("does not display a historical agent from another company", async () => { + await failRoster(429); + api.get.mockResolvedValue({ id: agentId, companyId: "company-b", name: "Private helper" }); + render(); + await waitFor(() => expect(api.get).toHaveBeenCalled()); + await waitFor(() => expect(container.textContent).toContain("Agent not found")); + expect(container.textContent).not.toContain("Private helper"); + expect(api.getChat).not.toHaveBeenCalled(); +}); diff --git a/ui/src/pages/AgentChat.tsx b/ui/src/pages/AgentChat.tsx index 6e8dc81d559..3e60b343062 100644 --- a/ui/src/pages/AgentChat.tsx +++ b/ui/src/pages/AgentChat.tsx @@ -3,6 +3,7 @@ import { useQuery, useQueryClient } from "@tanstack/react-query"; import { agentChatsApi } from "@/api/agentChats"; import { agentsApi } from "@/api/agents"; import { authApi } from "@/api/auth"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { useCompany } from "@/context/CompanyContext"; import { useAgentChatEnabled } from "@/hooks/useAgentChatEnabled"; import { recordAgentChatVisit } from "@/lib/recent-agent-chats"; @@ -28,13 +29,15 @@ export function AgentChat() { }); const userId = session.data?.user?.id ?? session.data?.session?.userId ?? null; + const agentsView = useQueryView(agents); const rosterAgent = agents.data?.find( (item) => item.id === agentRef || agentRouteRef(item) === agentRef, ); const historyAgent = useQuery({ queryKey: queryKeys.agents.detail(agentRef), queryFn: () => agentsApi.get(agentRef, selectedCompanyId!), - enabled: enabled && !!selectedCompanyId && agents.isSuccess && !rosterAgent && isUuidLike(agentRef), + enabled: enabled && !!selectedCompanyId && (agentsView.kind === "ready" || agentsView.kind === "stale") + && !rosterAgent && isUuidLike(agentRef), }); const agent = rosterAgent ?? (historyAgent.data?.companyId === selectedCompanyId ? historyAgent.data : undefined); const chatKey = queryKeys.agentChats.detail(selectedCompanyId, userId, agent?.id); @@ -43,6 +46,9 @@ export function AgentChat() { queryFn: () => agentChatsApi.get(selectedCompanyId!, agent!.id), enabled: enabled && !!agent && session.isFetched, }); + const sessionView = useQueryView(session); + const historyView = useQueryView(historyAgent); + const chatView = useQueryView(chat); const creating = useRef | null>(null); useEffect(() => { creating.current = null; @@ -69,7 +75,11 @@ export function AgentChat() { throw error; } }, [agent, selectedCompanyId, chat.data, client, userId]); - if (!loaded || agents.isPending || session.isPending || historyAgent.isFetching && !agent) + // An outage before anything loaded keeps the quiet loading copy; loaded + // data renders through it. + const reconnecting = agentsView.kind === "reconnecting" || sessionView.kind === "reconnecting" + || chatView.kind === "reconnecting" || (!rosterAgent && historyView.kind === "reconnecting"); + if (!loaded || agents.isPending || session.isPending || historyAgent.isFetching && !agent || reconnecting) return (

Loading conversation…

); @@ -80,11 +90,16 @@ export function AgentChat() { history remains available through task links.

); - if (agents.error || chat.error || !rosterAgent && historyAgent.error) + const failed = [agentsView, chatView, ...(rosterAgent ? [] : [historyView])].find((view) => view.kind === "error"); + if (failed) return ( -

- {(agents.error ?? chat.error ?? historyAgent.error)?.message} -

+ ); if (!agent) return

Agent not found.

; diff --git a/ui/src/pages/AgentChats.test.tsx b/ui/src/pages/AgentChats.test.tsx index 29fc4928c36..2ffac7052a7 100644 --- a/ui/src/pages/AgentChats.test.tsx +++ b/ui/src/pages/AgentChats.test.tsx @@ -174,6 +174,7 @@ describe("Chat landing", () => { state.chatsFetched = false; await render(); expect(state.navigate).not.toHaveBeenCalled(); - expect(container.querySelector('[role="alert"]')?.textContent).toContain("Couldn’t load your chats"); + expect(container.querySelector('[role="alert"]')?.textContent).toMatch(/Couldn[’']t load your chats/); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("Retry"); }); }); diff --git a/ui/src/pages/AgentChats.tsx b/ui/src/pages/AgentChats.tsx index 047d6d1d5be..59f8cb7b74e 100644 --- a/ui/src/pages/AgentChats.tsx +++ b/ui/src/pages/AgentChats.tsx @@ -2,6 +2,7 @@ import { usePrimaryAgentPresentation } from "@/components/primary-agent/PrimaryA import { useEffect, useState } from "react"; import { MessageCircle } from "lucide-react"; import { AgentAvatar } from "@/components/AgentAvatar"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { useBreadcrumbs } from "@/context/BreadcrumbContext"; import { useAgentChatNavigation, useOpenAgentChat } from "@/hooks/useAgentChatNavigation"; @@ -24,7 +25,12 @@ function AgentChatsContent({ companyId, userId, enabled, loaded, agents, chats, const recentIds = useRecentAgentChats(companyId ?? "", userId); const [openingId, setOpeningId] = useState(null); const [openError, setOpenError] = useState(null); - const blockingError = agents.error ?? session.error; + const agentsView = useQueryView(agents); + const sessionView = useQueryView(session); + const chatsView = useQueryView(chats); + // Only a real failure blocks the entry redirect; an outage waits it out below. + const blockingError = agentsView.kind === "error" ? agents.error : sessionView.kind === "error" ? session.error : null; + const reconnecting = agentsView.kind === "reconnecting" || sessionView.kind === "reconnecting" || chatsView.kind === "reconnecting"; const firstId = recentIds[0]; const firstAgent = agents.data?.find(agent => agent.id === firstId); // An empty chat with an active agent can reopen from the roster alone. A @@ -54,15 +60,16 @@ function AgentChatsContent({ companyId, userId, enabled, loaded, agents, chats, if (!loaded) return

Loading chat…

; if (!enabled) return

Agent Chat is disabled. Enable it in Experimental settings.

; if (!companyId) return

Select a company to start a conversation.

; + if (reconnecting) return

Loading chat…

; if (resolvingRecent || entryPath || primary?.loading) return

Opening chat…

; - const error = blockingError ?? (recentIds.length > 0 ? chats.error : null); + const error = blockingError ?? (recentIds.length > 0 && chatsView.kind === "error" ? chats.error : null); return

Who would you like to talk to?

Ask a question, think through an idea, or plan the next step with your team.

- {error ?

Couldn’t load your chats.

+ {error ? { agentsView.retry(); chatsView.retry(); sessionView.retry(); }} /> : agents.isPending || session.isPending ?

Loading agents…

:
{(agents.data ?? []).slice(0, 6).map(agent =>
)} - {resumeRun.isError && ( + {resumeRun.isError && ( // query-error-ok: mutation result, not a query
{resumeRun.error instanceof Error ? resumeRun.error.message : "Failed to resume run"}
)} - {retryRun.isError && ( + {retryRun.isError && ( // query-error-ok: mutation result, not a query
{retryRun.error instanceof Error ? retryRun.error.message : "Failed to retry run"}
@@ -3521,7 +3525,7 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } > {runClaudeLogin.isPending ? "Running claude login..." : "Login to Claude Code"} - {runClaudeLogin.isError && ( + {runClaudeLogin.isError && ( // query-error-ok: mutation result, not a query

{runClaudeLogin.error instanceof Error ? runClaudeLogin.error.message @@ -3662,7 +3666,7 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } ? "clearing session..." : "clear session for these tasks"} - {clearSessionsForTouchedIssues.isError && ( + {clearSessionsForTouchedIssues.isError && ( // query-error-ok: mutation result, not a query

{clearSessionsForTouchedIssues.error instanceof Error ? clearSessionsForTouchedIssues.error.message diff --git a/ui/src/pages/AgentDetail.tsx b/ui/src/pages/AgentDetail.tsx index 47492b31a53..d5fb0b41161 100644 --- a/ui/src/pages/AgentDetail.tsx +++ b/ui/src/pages/AgentDetail.tsx @@ -62,6 +62,7 @@ import { MembershipAction } from "../components/MembershipAction"; import { StarToggle } from "../components/StarToggle"; import { Identity } from "../components/Identity"; import { PageSkeleton } from "../components/PageSkeleton"; +import { QueryErrorState, queryViewKind, useQueryView } from "../components/QueryView"; import { AgentActionButtons } from "../components/AgentActionButtons"; import { InlineBanner } from "../components/InlineBanner"; import { BuiltInBundlePanel } from "../components/BuiltInBundlePanel"; @@ -822,11 +823,13 @@ export function AgentDetail() { const prepareAgentNavigation = useCallback(() => { return confirmAgentConfigNavigation(configDirty); }, [configDirty]); - const { data: agent, isLoading, error } = useQuery({ + const agentQuery = useQuery({ queryKey: [...queryKeys.agents.detail(routeAgentRef), lookupCompanyId ?? null], queryFn: () => agentsApi.get(routeAgentRef, lookupCompanyId), enabled: canFetchAgent, }); + const { data: agent, isLoading, error } = agentQuery; + const agentView = useQueryView(agentQuery); const lifecycle = useAgentLifecycleStatus(agent); const resolvedCompanyId = agent?.companyId ?? selectedCompanyId; const canonicalAgentRef = agent ? agentRouteRef(agent) : routeAgentRef; @@ -839,7 +842,6 @@ export function AgentDetail() { const { data: boardAccess } = useQuery({ queryKey: queryKeys.access.currentBoardAccess, queryFn: () => accessApi.getCurrentBoardAccess(), - retry: false, }); const canUseProviderTrace = boardAccess?.source === "local_implicit" || @@ -1168,8 +1170,10 @@ export function AgentDetail() { return () => window.removeEventListener("popstate", handlePopState, true); }, [configDirty, prepareAgentNavigation]); - if (isLoading) return ; - if (error) return

{error.message}

; + if (isLoading || agentView.kind === "reconnecting") return ; + if (agentView.kind === "error") { + return ; + } if (!agent) return null; if (!urlRunId && legacyAuditSection) { return ; @@ -1761,7 +1765,6 @@ export function AgentOverview({ queryKey: queryKeys.access.companyUserDirectory(agent.companyId), queryFn: () => accessApi.listUserDirectory(agent.companyId), enabled: hasHumanCreator, - retry: false, }); const creatorLabel = resolveAgentCreatorLabel(agent.createdByUserId, userDirectory.data?.users); return ( @@ -1785,14 +1788,14 @@ export function AgentOverview({ ) : Board} {directReportCount} - {hasHumanCreator && userDirectory.isError ? ( + {hasHumanCreator && queryViewKind(userDirectory) === "error" ? ( Could not load creator ) : creatorLabel ? ( {creatorLabel} ) : null} - {identity.isPending ? Loading… - : identity.isError ? Could not load public key + {identity.isPending || queryViewKind(identity) === "reconnecting" ? Loading… + : queryViewKind(identity) === "error" ? Could not load public key : identity.data ? ( {identity.data.keyId.slice(0, 19)}… @@ -3469,7 +3472,6 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } const { data: boardAccess } = useQuery({ queryKey: queryKeys.access.currentBoardAccess, queryFn: () => accessApi.getCurrentBoardAccess(), - retry: false, }); const canUseProviderTrace = boardAccess?.source === "local_implicit" || @@ -3485,7 +3487,6 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } queryKey: queryKeys.providerTraceMetadata(run.companyId, [run.id]), queryFn: () => heartbeatsApi.providerTraceMetadata(run.companyId, [run.id]), enabled: canUseProviderTrace, - retry: false, refetchInterval: run.status === "running" || run.status === "queued" ? 3000 : false, }); @@ -3495,7 +3496,6 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } queryKey: queryKeys.access.companyUserDirectory(run.companyId), queryFn: () => accessApi.listUserDirectory(run.companyId), enabled: Boolean(run.companyId && run.responsibleUserId), - retry: false, }); const responsibleUserName = useMemo(() => { if (!run.responsibleUserId) return null; @@ -3796,12 +3796,12 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } )} - {resumeRun.isError && ( + {resumeRun.isError && ( // query-error-ok: mutation result, not a query
{resumeRun.error instanceof Error ? resumeRun.error.message : "Failed to resume run"}
)} - {retryRun.isError && ( + {retryRun.isError && ( // query-error-ok: mutation result, not a query
{retryRun.error instanceof Error ? retryRun.error.message : "Failed to retry run"}
@@ -3842,7 +3842,7 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } > {runClaudeLogin.isPending ? "Running claude login..." : "Login to Claude Code"} - {runClaudeLogin.isError && ( + {runClaudeLogin.isError && ( // query-error-ok: mutation result, not a query

{runClaudeLogin.error instanceof Error ? runClaudeLogin.error.message @@ -3940,7 +3940,7 @@ function RunDetail({ run: initialRun, agentRouteId, adapterType, adapterConfig } ? "clearing session..." : "clear session for these tasks"} - {clearSessionsForTouchedIssues.isError && ( + {clearSessionsForTouchedIssues.isError && ( // query-error-ok: mutation result, not a query

{clearSessionsForTouchedIssues.error instanceof Error ? clearSessionsForTouchedIssues.error.message diff --git a/ui/src/pages/CaseDetail.test.tsx b/ui/src/pages/CaseDetail.test.tsx index c7a9d7930b1..c388f9392bc 100644 --- a/ui/src/pages/CaseDetail.test.tsx +++ b/ui/src/pages/CaseDetail.test.tsx @@ -7,6 +7,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CaseDetail as CaseDetailData, CaseHandoffView } from "@/api/cases"; import { ApiError } from "@/api/client"; +import { queryKeys } from "@/lib/queryKeys"; import { CaseDetail } from "./CaseDetail"; const cases = vi.hoisted(() => ({ @@ -260,6 +261,18 @@ describe("read-only CaseDetail", () => { expect(container.querySelector('a[href="/activity?entityType=case&entityId=case-1"]')?.textContent).toBe("View in Audit"); }); + it("keeps a loaded case visible after a transient background refresh fails", async () => { + render(); + await waitFor(() => expect(container.textContent).toContain("Hermes agent launch post")); + cases.get.mockRejectedValue(new ApiError("Rate limited", 429, {})); + const client = clients.at(-1)!; + await client.refetchQueries({ queryKey: queryKeys.cases.detail("PAP-C7"), exact: true }); + await waitFor(() => expect(client.getQueryState(queryKeys.cases.detail("PAP-C7"))?.status).toBe("error")); + await settle(); + expect(container.textContent).toContain("Hermes agent launch post"); + expect(container.querySelector('table[aria-label="Case fields"]')).not.toBeNull(); + expect(container.textContent).not.toContain("Case not found"); + }); it("uses the same unavailable state for missing and inaccessible cases", async () => { cases.get.mockRejectedValue(new ApiError("Denied", 403, {})); render(); await waitFor(() => expect(container.textContent).toContain("Case not found")); diff --git a/ui/src/pages/CaseDetail.tsx b/ui/src/pages/CaseDetail.tsx index 6b60c726e16..892d9218cd9 100644 --- a/ui/src/pages/CaseDetail.tsx +++ b/ui/src/pages/CaseDetail.tsx @@ -14,6 +14,7 @@ import { EmptyState } from "@/components/EmptyState"; import { EntityRow } from "@/components/EntityRow"; import { IssueReferencePill } from "@/components/IssueReferencePill"; import { PageSkeleton } from "@/components/PageSkeleton"; +import { QueryErrorState, queryViewKind, useQueryView } from "@/components/QueryView"; import { CaseDocumentPreview } from "@/components/cases/CaseDocumentPreview"; import { CaseHandoffHistory } from "@/components/cases/CaseHandoffHistory"; import { CaseStatus } from "@/components/cases/CaseStatus"; @@ -42,6 +43,7 @@ export function CaseDetail() { queryFn: () => casesApi.get(caseIdentifier!), enabled: !!caseIdentifier, }); + const caseView = useQueryView(caseQuery); const caseData = caseQuery.data; const listPath = `${caseHref()}${search}`; const childrenQuery = useQuery({ @@ -66,14 +68,16 @@ export function CaseDetail() { }, [setBreadcrumbs, listPath, caseData?.identifier, caseIdentifier]); if (!caseIdentifier) return ; - if (caseQuery.isLoading) return

; - if (caseQuery.isError || !caseData) { + if (caseQuery.isLoading || caseView.kind === "reconnecting") return
; + if (caseView.kind === "error" || !caseData) { + // "Not found" only for a real 403/404; an outage never looks like a missing case. const unavailable = caseQuery.error instanceof ApiError && [403, 404].includes(caseQuery.error.status); return
- + {unavailable + ? + : }
- {!unavailable && }
; } @@ -133,7 +137,7 @@ export function CaseDetail() { Related cases {caseData.parent &&

Parent

} {children.length > 0 &&

Children {children.length}

{children.map(child => )}
} - {childrenQuery.isError &&

Child cases couldn’t be loaded.

} + {queryViewKind(childrenQuery) === "error" &&

Child cases couldn’t be loaded.

} } {!fields.length && !caseData.documents.length && !taskDocuments.length && !caseData.attachments.length &&

No findings recorded yet.

} diff --git a/ui/src/pages/CompanySettingsPluginPage.tsx b/ui/src/pages/CompanySettingsPluginPage.tsx index 091c7d68303..7769a225c5a 100644 --- a/ui/src/pages/CompanySettingsPluginPage.tsx +++ b/ui/src/pages/CompanySettingsPluginPage.tsx @@ -2,6 +2,7 @@ import { useEffect, useMemo } from "react"; import { useParams } from "@/lib/router"; import { useBreadcrumbs } from "@/context/BreadcrumbContext"; import { useCompany } from "@/context/CompanyContext"; +import { QueryErrorState } from "@/components/QueryView"; import { PluginSlotMount, usePluginSlots } from "@/plugins/slots"; import { NotFoundPage } from "./NotFound"; @@ -25,7 +26,7 @@ export function CompanySettingsPluginPage() { ? companies.find((company) => company.id === resolvedCompanyId)?.issuePrefix ?? null : null; - const { slots, isLoading, errorMessage } = usePluginSlots({ + const { slots, isLoading, errorMessage, error, retry } = usePluginSlots({ slotTypes: ["companySettingsPage"], companyId: resolvedCompanyId, enabled: Boolean(resolvedCompanyId && settingsRoutePath), @@ -57,12 +58,9 @@ export function CompanySettingsPluginPage() { return
Loading...
; } + // Only a real failure reaches here; an outage keeps `isLoading` true above. if (errorMessage) { - return ( -
- Plugin extensions unavailable: {errorMessage} -
- ); + return ; } if (pageSlots.length > 1) { diff --git a/ui/src/pages/ExecutionWorkspaceDetail.tsx b/ui/src/pages/ExecutionWorkspaceDetail.tsx index e1d65ba2b24..ee68920b693 100644 --- a/ui/src/pages/ExecutionWorkspaceDetail.tsx +++ b/ui/src/pages/ExecutionWorkspaceDetail.tsx @@ -4,6 +4,7 @@ import { Link, Navigate, useLocation, useNavigate, useParams } from "@/lib/route import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import type { ExecutionWorkspace, Issue, Project, ProjectWorkspace, RoutineListItem, WorkspaceOperation } from "@paperclipai/shared"; import { Copy, ExternalLink, Loader2, Play, Repeat } from "lucide-react"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { Card, CardContent, CardDescription, CardHeader, CardTitle, CardAction } from "@/components/ui/card"; import { Input } from "@/components/ui/input"; @@ -800,6 +801,7 @@ export function ExecutionWorkspaceDetail() { queryFn: () => executionWorkspacesApi.get(workspaceId!), enabled: Boolean(workspaceId), }); + const workspaceView = useQueryView(workspaceQuery); const workspace = workspaceQuery.data ?? null; const projectQuery = useQuery({ @@ -979,12 +981,18 @@ export function ExecutionWorkspaceDetail() { }, }); - if (workspaceQuery.isLoading) return

Loading workspace…

; - if (workspaceQuery.error) { + if (workspaceQuery.isLoading || workspaceView.kind === "reconnecting") { + return

Loading workspace…

; + } + if (workspaceView.kind === "error") { return ( -

- {workspaceQuery.error instanceof Error ? workspaceQuery.error.message : "Failed to load workspace"} -

+ ); } if (!workspace || !form || !initialState) return null; diff --git a/ui/src/pages/GoalDetail.tsx b/ui/src/pages/GoalDetail.tsx index e922a6e7008..5bcf7a9bca6 100644 --- a/ui/src/pages/GoalDetail.tsx +++ b/ui/src/pages/GoalDetail.tsx @@ -15,6 +15,7 @@ import { StatusBadge } from "../components/StatusBadge"; import { InlineEditor } from "../components/InlineEditor"; import { EntityRow } from "../components/EntityRow"; import { PageSkeleton } from "../components/PageSkeleton"; +import { QueryErrorState, useQueryView } from "../components/QueryView"; import { cn, projectUrl } from "../lib/utils"; import { Button } from "@/components/ui/button"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; @@ -54,15 +55,13 @@ export function GoalDetail() { const { setBreadcrumbs } = useBreadcrumbs(); const queryClient = useQueryClient(); - const { - data: goal, - isLoading, - error - } = useQuery({ + const goalQuery = useQuery({ queryKey: queryKeys.goals.detail(goalId!), queryFn: () => goalsApi.get(goalId!), enabled: !!goalId }); + const { data: goal, isLoading, error } = goalQuery; + const goalView = useQueryView(goalQuery); const resolvedCompanyId = goal?.companyId ?? selectedCompanyId; const { data: allGoals } = useQuery({ @@ -135,8 +134,10 @@ export function GoalDetail() { return () => closePanel(); }, [goal]); // eslint-disable-line react-hooks/exhaustive-deps - if (isLoading) return ; - if (error) return

{error.message}

; + if (isLoading || goalView.kind === "reconnecting") return ; + if (goalView.kind === "error") { + return ; + } if (!goal) return null; return ( diff --git a/ui/src/pages/InviteLanding.tsx b/ui/src/pages/InviteLanding.tsx index 4835a004174..8b40865a98e 100644 --- a/ui/src/pages/InviteLanding.tsx +++ b/ui/src/pages/InviteLanding.tsx @@ -7,7 +7,7 @@ import { CompanyPatternIcon } from "@/components/CompanyPatternIcon"; import { useCompany } from "@/context/CompanyContext"; import { Link, useNavigate, useParams } from "@/lib/router"; import { accessApi } from "../api/access"; -import { authApi } from "../api/auth"; +import { AuthApiError, authApi, authErrorServerMessage } from "../api/auth"; import { fetchCompanyListForCurrentAccount, useCompanyListQuery } from "../api/companies-query"; import { healthApi } from "../api/health"; import { getAdapterLabel } from "../adapters/adapter-display-registry"; @@ -66,6 +66,11 @@ function getAuthErrorMessage(error: unknown) { return message.length > 0 ? message : null; } +/** A bare HTTP status from the auth API, with no server explanation. */ +function isUnexplainedAuthStatus(error: unknown, status: number) { + return error instanceof AuthApiError && error.status === status && authErrorServerMessage(error.body) === null; +} + function mapInviteAuthFeedback( error: unknown, authMode: AuthMode, @@ -90,7 +95,7 @@ function mapInviteAuthFeedback( }; } - if (authMode === "sign_in" && message === "Request failed: 401") { + if (authMode === "sign_in" && isUnexplainedAuthStatus(error, 401)) { return { tone: "error", message: @@ -98,7 +103,7 @@ function mapInviteAuthFeedback( }; } - if (authMode === "sign_up" && message === "Request failed: 422") { + if (authMode === "sign_up" && isUnexplainedAuthStatus(error, 422)) { return { tone: "info", message: `An account may already exist for ${emailLabel}. Try signing in instead.`, diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index a9adc686cb3..a57d2988b84 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -281,6 +281,8 @@ import { PopoverContent, } from "@/components/ui/popover"; import { Button } from "@/components/ui/button"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; +import { describeError, isTransientError } from "@/api/errors"; import { Sheet, SheetContent, @@ -402,7 +404,7 @@ function createRunCancelledStatusUpdateError( ): StopAndFinalizeRunError { const message = err instanceof Error - ? `Run was stopped, but updating the task failed: ${err.message}` + ? `Run was stopped, but updating the task failed: ${describeError(err).body}` : "Run was stopped, but updating the task failed. Retry the task status update."; const error = new Error(message) as StopAndFinalizeRunError; error.runCancelledBeforeStatusUpdateFailed = true; @@ -2794,7 +2796,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ } footer={grokWork || stopGrokWork.error || classicQueuedDeliveryError ? <> {grokWork &&

Runs in Grok Bot · Usage not reported. Stop Paperclip work revokes this session’s authority; remote execution may continue. Open the Grok Bot desktop app to inspect it.

} - {stopGrokWork.error &&

{stopGrokWork.error.message}

} + {stopGrokWork.error &&

{describeError(stopGrokWork.error).body}

} {classicQueuedDeliveryError &&

{classicQueuedDeliveryError}

} {footer} : footer} @@ -2894,7 +2896,6 @@ function IssueDetailActivityTab({ throw error; } }, - retry: false, placeholderData: keepPreviousDataForSameQueryTail > | null>(issueId), @@ -3333,13 +3334,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS Boolean(conversation || classicTaskInterfaceEnabled || location.hash || detailTab !== "chat"), coreIssueQuery.data, ); - const { - data: queriedIssue, - isLoading, - isPlaceholderData, - error, - refetch: refetchIssueDetail, - } = useQuery({ + const issueQuery = useQuery({ ...getIssueDetailQueryOptions(queryClient, issueId!, { placeholderIssue: issueHeaderSeed ? { @@ -3350,6 +3345,17 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS }), enabled: !!issueId && (coreIssueQuery.isFetched || !!queryClient.getQueryData(queryKeys.issues.detail(issueId))), }); + const { + data: queriedIssue, + isLoading, + isPlaceholderData, + error, + refetch: refetchIssueDetail, + } = issueQuery; + // A header seed is a placeholder, not loaded data: it must not count as + // content worth keeping on screen through a failure. + const issueView = useQueryView(isPlaceholderData ? { ...issueQuery, data: undefined } : issueQuery); + const coreIssueView = useQueryView(coreIssueQuery); useEffect(() => { if (issueId && historyExpanded) void prefetchIssueThread(queryClient, issueId); }, [issueId, historyExpanded, queryClient]); @@ -3370,9 +3376,10 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS }; // A cached header seed can paint during navigation, but must not redirect // or upload against the previous task while the requested task is loading. + // A failed refetch of an already loaded task does not unload it. const loadedIssue = !isPlaceholderData && - (!error || !!coreIssueQuery.data) && + !(error && !queriedIssue && !coreIssueQuery.data) && issue && issueId && (issue.id.toLowerCase() === issueId.toLowerCase() || @@ -3697,7 +3704,6 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS queryKey: queryKeys.access.currentBoardAccess, queryFn: () => accessApi.getCurrentBoardAccess(), enabled: !!session?.user?.id, - retry: false, }); const canManageTreeControl = Boolean( selectedCompanyId && boardAccess?.companyIds?.includes(selectedCompanyId), @@ -3721,13 +3727,11 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS queryKey: queryKeys.instance.generalSettings, queryFn: () => instanceSettingsApi.getGeneral(), enabled: !!issueId, - retry: false, }); const { data: instanceExperimentalSettings } = useQuery({ queryKey: queryKeys.instance.experimentalSettings, queryFn: () => instanceSettingsApi.getExperimental(), enabled: !!issueId, - retry: false, }); // Experimental Cases: linkify `PAP-C7` chips in this issue's comment bodies. const casesChipsEnabled = instanceExperimentalSettings?.enableCases === true; @@ -3780,13 +3784,11 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS }), enabled: treeControlOpen && !!issueId && canManageTreeControl, staleTime: 0, - retry: false, }); const { data: treeControlState, error: treeControlStateError } = useQuery({ queryKey: ["issues", "tree-control-state", issueId ?? "pending"], queryFn: () => issuesApi.getTreeControlState(issueId!), enabled: !!issueId, - retry: false, }); const { data: activeRootPauseHolds = [] } = useQuery({ queryKey: [ @@ -7212,9 +7214,28 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS ); }, [activePauseHold, issue]); - if (!issue && !accessError && (isLoading || coreIssueQuery.isPending)) + // The loaded task stays on screen while a refetch fails (the connection + // banner explains the outage); the skeleton covers an outage before the + // first load; only a real failure replaces the pane. + if ( + (!issue && !accessError && (isLoading || coreIssueQuery.isPending || coreIssueView.kind === "reconnecting")) || + (issueView.kind === "reconnecting" && (!issue || (isPlaceholderData && !coreIssueQuery.data))) + ) return ; - if (!issue && (error || coreIssueQuery.error)) return

{(error || coreIssueQuery.error)?.message}

; + if (!issue || (issueView.kind === "error" && isPlaceholderData && !coreIssueQuery.data)) { + const pageError = accessError ?? error ?? coreIssueQuery.error; + if (pageError) { + return ( + + ); + } + } if (!issue) return null; // Do not expose a file chooser on the outgoing UUID/company/interface // branch: its input can be detached before the chosen file is returned. @@ -7867,7 +7888,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS onCancelMonitor={() => cancelIssueMonitor.mutateAsync()} issue={issue} workProducts={workProducts} - checkError={checkIssueMonitorNow.error?.message} + checkError={checkIssueMonitorNow.error ? describeError(checkIssueMonitorNow.error, { action: "check the monitor" }).body : null} onCheckNow={() => checkIssueMonitorNow.mutate()} checkingNow={checkIssueMonitorNow.isPending} /> @@ -7969,7 +7990,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS : "max-w-3xl space-y-6" } > - {error && coreIssueQuery.data ? ( + {error && coreIssueQuery.data && !isTransientError(error) ? (
Some task details could not be loaded. @@ -8032,7 +8053,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS role="alert" className={cn("text-sm text-destructive", shellSectionClass)} > - {executeTreeControl.error.message} + {describeError(executeTreeControl.error).body}

)} @@ -8447,7 +8468,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS checkIssueMonitorNow.mutate()} checkingNow={checkIssueMonitorNow.isPending} /> @@ -8681,7 +8702,9 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS error={ treeControlPreviewError ? treeControlPreviewErrorCopy(treeControlPreviewError) - : executeTreeControl.error?.message + : executeTreeControl.error + ? describeError(executeTreeControl.error).body + : undefined } pending={executeTreeControl.isPending} valid={canApplyTreeControl} diff --git a/ui/src/pages/PluginManager.tsx b/ui/src/pages/PluginManager.tsx index e1cccb40603..a7cdfa7679c 100644 --- a/ui/src/pages/PluginManager.tsx +++ b/ui/src/pages/PluginManager.tsx @@ -13,6 +13,7 @@ import { useCompany } from "@/context/CompanyContext"; import { useBreadcrumbs } from "@/context/BreadcrumbContext"; import { pluginsApi } from "@/api/plugins"; import { queryKeys } from "@/lib/queryKeys"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; @@ -105,15 +106,18 @@ export function PluginManager() { ]); }, [selectedCompany?.name, setBreadcrumbs]); - const { data: plugins, isLoading, error } = useQuery({ + const pluginsQuery = useQuery({ queryKey: queryKeys.plugins.all, queryFn: () => pluginsApi.list(), }); + const { data: plugins, isLoading, error } = pluginsQuery; + const pluginsView = useQueryView(pluginsQuery); const bundledQuery = useQuery({ queryKey: queryKeys.plugins.examples, queryFn: () => pluginsApi.listBundled(), }); + const bundledView = useQueryView(bundledQuery); const invalidatePluginQueries = () => { queryClient.invalidateQueries({ queryKey: queryKeys.plugins.all }); @@ -185,8 +189,20 @@ export function PluginManager() { [installedPlugins] ); - if (isLoading) return
Loading plugins...
; - if (error) return
Failed to load plugins.
; + if (isLoading || pluginsView.kind === "reconnecting") { + return
Loading plugins...
; + } + if (pluginsView.kind === "error") { + return ( + + ); + } return (
@@ -259,10 +275,15 @@ export function PluginManager() {
)} - {bundledQuery.isLoading ? ( + {bundledQuery.isLoading || bundledView.kind === "reconnecting" ? (
Loading bundled plugins...
- ) : bundledQuery.error ? ( -
Failed to load bundled plugins.
+ ) : bundledView.kind === "error" ? ( + ) : bundledPlugins.length === 0 ? (
No bundled plugins were found in this checkout. diff --git a/ui/src/pages/ProjectDetail.tsx b/ui/src/pages/ProjectDetail.tsx index c65a04d77d7..4ee819de716 100644 --- a/ui/src/pages/ProjectDetail.tsx +++ b/ui/src/pages/ProjectDetail.tsx @@ -23,6 +23,7 @@ import { ProjectTile } from "../components/ProjectTile"; import { BudgetPolicyCard } from "../components/BudgetPolicyCard"; import { IssuesList } from "../components/IssuesList"; import { PageSkeleton } from "../components/PageSkeleton"; +import { QueryErrorState, useQueryView } from "../components/QueryView"; import { PageTabBar } from "../components/PageTabBar"; import { ProjectWorkspacesContent } from "../components/ProjectWorkspacesContent"; import { SummarySlotCard } from "../components/SummarySlotCard"; @@ -356,7 +357,7 @@ export function ProjectDetail() { }, [location.search]); const activeTab = activeRouteTab ?? pluginTabFromSearch; - const { data: project, isLoading, error } = useQuery({ + const projectQuery = useQuery({ queryKey: [...queryKeys.projects.detail(routeProjectRef), lookupCompanyId ?? null], queryFn: () => projectsApi.get(routeProjectRef, lookupCompanyId), enabled: canFetchProject, @@ -368,6 +369,8 @@ export function ProjectDetail() { ? previous : undefined, }); + const { data: project, isLoading, error } = projectQuery; + const projectView = useQueryView(projectQuery); const canonicalProjectRef = project ? projectRouteRef(project) : routeProjectRef; const projectLookupRef = project?.id ?? routeProjectRef; const resolvedCompanyId = project?.companyId ?? selectedCompanyId; @@ -404,24 +407,27 @@ export function ProjectDetail() { const pluginTabDecisionLoaded = Boolean(resolvedCompanyId) && !pluginDetailSlotsLoading; const isolatedWorkspacesEnabled = experimentalSettingsQuery.data?.enableIsolatedWorkspaces === true; const workspaceTabProjectId = project?.id ?? null; - const { data: workspaceTabIssues = [], isLoading: isWorkspaceTabIssuesLoading, error: workspaceTabIssuesError } = useQuery({ + const workspaceTabIssuesQuery = useQuery({ queryKey: workspaceTabProjectId && resolvedCompanyId ? queryKeys.issues.listByProject(resolvedCompanyId, workspaceTabProjectId) : ["issues", "__workspace-tab__", "disabled"], queryFn: () => issuesApi.list(resolvedCompanyId!, { projectId: workspaceTabProjectId! }), enabled: Boolean(resolvedCompanyId && workspaceTabProjectId && isolatedWorkspacesEnabled), }); - const { - data: workspaceTabExecutionWorkspaces = [], - isLoading: isWorkspaceTabExecutionWorkspacesLoading, - error: workspaceTabExecutionWorkspacesError, - } = useQuery({ + const { data: workspaceTabIssues = [], isLoading: isWorkspaceTabIssuesLoading } = workspaceTabIssuesQuery; + const workspaceTabIssuesView = useQueryView(workspaceTabIssuesQuery); + const workspaceTabExecutionWorkspacesQuery = useQuery({ queryKey: workspaceTabProjectId && resolvedCompanyId ? queryKeys.executionWorkspaces.list(resolvedCompanyId, { projectId: workspaceTabProjectId }) : ["execution-workspaces", "__workspace-tab__", "disabled"], queryFn: () => executionWorkspacesApi.list(resolvedCompanyId!, { projectId: workspaceTabProjectId! }), enabled: Boolean(resolvedCompanyId && workspaceTabProjectId && isolatedWorkspacesEnabled), }); + const { + data: workspaceTabExecutionWorkspaces = [], + isLoading: isWorkspaceTabExecutionWorkspacesLoading, + } = workspaceTabExecutionWorkspacesQuery; + const workspaceTabExecutionWorkspacesView = useQueryView(workspaceTabExecutionWorkspacesQuery); const workspaceSummaries = useMemo(() => { if (!project || !isolatedWorkspacesEnabled) return []; return buildProjectWorkspaceSummaries({ @@ -431,10 +437,18 @@ export function ProjectDetail() { }); }, [project, isolatedWorkspacesEnabled, workspaceTabIssues, workspaceTabExecutionWorkspaces]); const showWorkspacesTab = isolatedWorkspacesEnabled && workspaceSummaries.length > 0; + const workspaceTabReconnecting = + workspaceTabIssuesView.kind === "reconnecting" || workspaceTabExecutionWorkspacesView.kind === "reconnecting"; const workspaceTabDecisionLoaded = experimentalSettingsQuery.isFetched && - (!isolatedWorkspacesEnabled || (!isWorkspaceTabIssuesLoading && !isWorkspaceTabExecutionWorkspacesLoading)); - const workspaceTabError = (workspaceTabIssuesError ?? workspaceTabExecutionWorkspacesError) as Error | null; + (!isolatedWorkspacesEnabled || + (!isWorkspaceTabIssuesLoading && !isWorkspaceTabExecutionWorkspacesLoading && !workspaceTabReconnecting)); + // Only a real failure shows an error; loaded summaries stay through an outage. + const workspaceTabFailure = workspaceTabIssuesView.kind === "error" + ? workspaceTabIssuesView + : workspaceTabExecutionWorkspacesView.kind === "error" + ? workspaceTabExecutionWorkspacesView + : null; useEffect(() => { if (!project?.companyId || project.companyId === selectedCompanyId) return; @@ -688,8 +702,10 @@ export function ProjectDetail() { return ; } - if (isLoading) return ; - if (error) return

{error.message}

; + if (isLoading || projectView.kind === "reconnecting") return ; + if (projectView.kind === "error") { + return ; + } if (!project) return null; const showLeftProjectNotice = projectMembershipState === "left" && !dismissedLeftProjectIds.has(project.id); @@ -879,8 +895,13 @@ export function ProjectDetail() { {activeTab === "workspaces" ? ( workspaceTabDecisionLoaded ? ( - workspaceTabError ? ( -

{workspaceTabError.message}

+ workspaceTabFailure ? ( + ) : ( projectsApi.get(routeProjectRef, lookupCompanyId), enabled: canFetchProject, }); + const projectView = useQueryView(projectQuery); const project = projectQuery.data ?? null; const workspace = useMemo( @@ -391,12 +393,18 @@ export function ProjectWorkspaceDetail() { }, }); - if (projectQuery.isLoading) return

Loading workspace…

; - if (projectQuery.error) { + if (projectQuery.isLoading || projectView.kind === "reconnecting") { + return

Loading workspace…

; + } + if (projectView.kind === "error") { return ( -

- {projectQuery.error instanceof Error ? projectQuery.error.message : "Failed to load workspace"} -

+ ); } if (!project || !workspace || !form || !initialState) { diff --git a/ui/src/pages/SkillStudio.tsx b/ui/src/pages/SkillStudio.tsx index 91fa0707ddf..c47a1a8f456 100644 --- a/ui/src/pages/SkillStudio.tsx +++ b/ui/src/pages/SkillStudio.tsx @@ -3,6 +3,7 @@ import { interactionReadinessRefetchInterval } from "@/lib/issue-thread-interact import { SkillBinaryFile } from "../components/SkillBinaryFile"; import { SkillSourceProvenance } from "../components/SkillSourceProvenance"; import { AgentIdentity } from "@/components/AgentIdentity"; +import { QueryErrorState, useQueryView } from "@/components/QueryView"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { @@ -299,6 +300,7 @@ export function SkillStudio() { queryFn: () => companySkillsApi.detail(companyId, skillId), enabled: Boolean(companyId && skillId && !isCreateMode), }); + const detailView = useQueryView(detailQuery); const forkDetailQuery = useQuery({ queryKey: queryKeys.companySkills.detail(companyId, forkFromSkillId ?? ""), queryFn: () => companySkillsApi.detail(companyId, forkFromSkillId!), @@ -362,10 +364,22 @@ export function SkillStudio() { /> ); } - if (detailQuery.isLoading) { + if (detailQuery.isLoading || detailView.kind === "reconnecting") { return ; } - if (detailQuery.isError || !detailQuery.data) { + // "Not found" only for a real 404; an outage never looks like a missing skill. + if (detailView.kind === "error" && detailView.errorKind !== "not_found") { + return ( + + ); + } + if (detailView.kind === "error" || !detailQuery.data) { return ; } diff --git a/ui/src/pages/Workspaces.test.tsx b/ui/src/pages/Workspaces.test.tsx index 2522896c3ba..1f82b2c9666 100644 --- a/ui/src/pages/Workspaces.test.tsx +++ b/ui/src/pages/Workspaces.test.tsx @@ -6,6 +6,7 @@ import { createRoot, type Root } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { WorkspaceOverviewItem, WorkspaceOverviewResponse } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; import { Workspaces } from "./Workspaces"; const mockExecutionWorkspacesApi = vi.hoisted(() => ({ @@ -241,4 +242,28 @@ describe("Workspaces", () => { expect(container.textContent).toContain("/issues"); expect(mockExecutionWorkspacesApi.listOverview).not.toHaveBeenCalled(); }); + + it("shows readable copy with Retry instead of redirecting when the feature flag cannot be read", async () => { + mockInstanceSettingsApi.getExperimental.mockRejectedValue( + new ApiError("Too many requests", 429, { error: "rate_limited" }), + ); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + await flushQueries(); + + expect(container.querySelector('[data-testid="navigate"]')).toBeNull(); + const alert = container.querySelector('[data-query-view="error"]'); + expect(alert).not.toBeNull(); + expect(alert?.textContent).not.toContain("rate_limited"); + expect(alert?.querySelector("button")?.textContent).toContain("Retry"); + expect(mockExecutionWorkspacesApi.listOverview).not.toHaveBeenCalled(); + }); }); diff --git a/ui/src/pages/Workspaces.tsx b/ui/src/pages/Workspaces.tsx index a2feba67c49..000a53615d8 100644 --- a/ui/src/pages/Workspaces.tsx +++ b/ui/src/pages/Workspaces.tsx @@ -8,6 +8,7 @@ import { instanceSettingsApi } from "../api/instanceSettings"; import { ProjectWorkspacesContent } from "../components/ProjectWorkspacesContent"; import { SummarySlotCard } from "../components/SummarySlotCard"; import { PageSkeleton } from "../components/PageSkeleton"; +import { QueryErrorState, useQueryView } from "../components/QueryView"; import { useBreadcrumbs } from "../context/BreadcrumbContext"; import { useCompany } from "../context/CompanyContext"; import type { ProjectWorkspaceSummary } from "../lib/project-workspaces-tab"; @@ -83,6 +84,7 @@ export function Workspaces() { queryKey: queryKeys.instance.experimentalSettings, queryFn: () => instanceSettingsApi.getExperimental(), }); + const experimentalSettingsView = useQueryView(experimentalSettingsQuery); const isolatedWorkspacesEnabled = experimentalSettingsQuery.data?.enableIsolatedWorkspaces === true; const overviewQuery = useInfiniteQuery({ @@ -94,6 +96,7 @@ export function Workspaces() { getNextPageParam: (lastPage) => lastPage.nextOffset ?? undefined, enabled: Boolean(selectedCompanyId && isolatedWorkspacesEnabled), }); + const overviewView = useQueryView(overviewQuery); useEffect(() => { setBreadcrumbs([{ label: "Workspaces" }]); @@ -107,13 +110,30 @@ export function Workspaces() { const groups = useMemo(() => buildProjectWorkspaceGroups(overviewItems), [overviewItems]); const firstPage = overviewPages[0] ?? null; const totalWorkspaceCount = firstPage?.total ?? overviewItems.length; - const dataLoading = overviewQuery.isLoading; + const dataLoading = overviewQuery.isLoading || overviewView.kind === "reconnecting"; const error = overviewQuery.error as Error | null; - if (experimentalSettingsQuery.isLoading) return ; + // An outage before the settings load must not redirect away as "disabled". + if (experimentalSettingsQuery.isLoading || experimentalSettingsView.kind === "reconnecting") { + return ; + } + // Nor must a failed settings load: with no settings there is no flag to read. + if (experimentalSettingsView.kind === "error" && experimentalSettingsQuery.data === undefined) { + return ( + + ); + } if (!isolatedWorkspacesEnabled) return ; if (dataLoading) return ; - if (error) return

{error.message}

; + if (overviewView.kind === "error") { + return ; + } return (
diff --git a/ui/src/plugins/bridge.ts b/ui/src/plugins/bridge.ts index fcdf35af453..183658c1d4a 100644 --- a/ui/src/plugins/bridge.ts +++ b/ui/src/plugins/bridge.ts @@ -35,6 +35,8 @@ import type { } from "@paperclipai/shared"; import { pluginsApi } from "@/api/plugins"; import { ApiError } from "@/api/client"; +import { isTransientError } from "@/api/errors"; +import { retryDelayFor, shouldRetryRequest } from "@/lib/query-client"; import { useToastActions, type ToastInput } from "@/context/ToastContext"; import { useSidebar } from "@/context/SidebarContext"; import { isGlobalPath, normalizeCompanyPrefix } from "@/lib/company-routes"; @@ -367,6 +369,13 @@ export function usePluginData( const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [refreshCounter, setRefreshCounter] = useState(0); + /** + * The identity of the request whose successful response `data` holds, or + * null. Data is kept through an outage only for that same request: once + * `key`, `params`, the company, or the render environment change, the old + * value belongs to a different request and must not stand in for the new one. + */ + const dataRequestKey = useRef(null); // Stable serialization for params change detection const paramsKey = serializeParams(params); @@ -374,9 +383,8 @@ export function usePluginData( useEffect(() => { let cancelled = false; let retryTimer: ReturnType | null = null; - let retryCount = 0; - const maxRetryCount = 2; - const retryableCodes: PluginBridgeErrorCode[] = ["WORKER_UNAVAILABLE", "TIMEOUT"]; + let failureCount = 0; + const requestKey = [pluginId, companyId ?? "", key, paramsKey, renderEnvironmentKey].join("\u0000"); setLoading(true); const request = () => { pluginsApi @@ -389,6 +397,7 @@ export function usePluginData( ) .then((response) => { if (!cancelled) { + dataRequestKey.current = requestKey; setData(response.data as T); setError(null); setLoading(false); @@ -397,19 +406,28 @@ export function usePluginData( .catch((err: unknown) => { if (cancelled) return; - const bridgeError = extractBridgeError(err); - if (retryableCodes.includes(bridgeError.code) && retryCount < maxRetryCount) { - retryCount += 1; + // The same policy as every other read: transient failures (a worker + // restarting, a gateway blip) retry with backoff, client errors do not. + if (shouldRetryRequest(failureCount, err)) { + const delay = retryDelayFor(failureCount, err); + failureCount += 1; retryTimer = setTimeout(() => { retryTimer = null; if (!cancelled) request(); - }, 150 * retryCount); + }, delay); return; } - setError(bridgeError); - setData(null); setLoading(false); + if (isTransientError(err) && dataRequestKey.current === requestKey) { + // Keep the last good data for this same request through an + // outage. The app-level connection banner explains why it is not + // updating. + return; + } + dataRequestKey.current = null; + setError(extractBridgeError(err)); + setData(null); }); }; diff --git a/ui/src/plugins/launchers.tsx b/ui/src/plugins/launchers.tsx index 19f052df4e4..c4bc54230d8 100644 --- a/ui/src/plugins/launchers.tsx +++ b/ui/src/plugins/launchers.tsx @@ -25,6 +25,8 @@ import type { } from "@paperclipai/shared"; import { pluginsApi, type PluginUiContribution } from "@/api/plugins"; import { authApi } from "@/api/auth"; +import { describeError, isTransientError } from "@/api/errors"; +import { queryViewKind } from "@/components/QueryView"; import { Button } from "@/components/ui/button"; import { useNavigate, useLocation } from "@/lib/router"; import { queryKeys } from "@/lib/queryKeys"; @@ -68,9 +70,12 @@ type UsePluginLaunchersFilters = { }; type UsePluginLaunchersResult = { + /** The last loaded launchers. Kept through a transient failure so chrome never disappears. */ launchers: ResolvedPluginLauncher[]; contributionsByPluginId: Map; + /** True while nothing has loaded yet, including while reconnecting after an outage. */ isLoading: boolean; + /** Readable copy for a non-transient failure, or null. Never set for an outage. */ errorMessage: string | null; }; @@ -127,11 +132,6 @@ const supportedLauncherBounds = new Set( const PluginLauncherRuntimeContext = createContext(null); -function getErrorMessage(error: unknown): string { - if (error instanceof Error && error.message) return error.message; - return "Unknown error"; -} - function buildLauncherHostContext( context: PluginLauncherContext, renderEnvironment: PluginRenderEnvironmentContext | null, @@ -290,11 +290,14 @@ export function usePluginLaunchers( filters: UsePluginLaunchersFilters, ): UsePluginLaunchersResult { const queryEnabled = filters.enabled ?? true; - const { data, isLoading, error } = useQuery({ + const query = useQuery({ queryKey: queryKeys.plugins.uiContributions, queryFn: () => pluginsApi.listUiContributions(), enabled: queryEnabled, }); + const { data, isLoading, error } = query; + // Shared with `usePluginSlots`: an outage is never reported as an error here. + const viewKind = queryViewKind(query); const placementZonesKey = useMemo( () => [...filters.placementZones].sort().join("|"), @@ -344,11 +347,14 @@ export function usePluginLaunchers( return rows; }, [data, filters.entityType, placementZonesKey]); + // Same as `usePluginSlots`: a transient failure never reports an error, + // even after the retry policy gives up while the server is reachable. + const transientFailure = viewKind === "error" && isTransientError(error); return { launchers, contributionsByPluginId, - isLoading: queryEnabled && isLoading, - errorMessage: error ? getErrorMessage(error) : null, + isLoading: queryEnabled && (isLoading || viewKind === "reconnecting" || transientFailure), + errorMessage: viewKind === "error" && !transientFailure ? describeError(error).body : null, }; } @@ -780,40 +786,28 @@ type PluginLauncherOutletProps = { entityType?: PluginUiSlotEntityType | null; className?: string; itemClassName?: string; - errorClassName?: string; - /** - * `hidden` suppresses the inline error and keeps rendering the last loaded - * launchers, so ambient chrome (the sidebar) stays quiet while the server is - * unreachable. - */ - errorBehavior?: "inline" | "hidden"; }; +/** + * Renders the plugin launchers for a host placement. Like `PluginSlotOutlet`, + * an outlet never shows a fetch error: it renders the last loaded launchers, + * or nothing if none have loaded yet. + */ export function PluginLauncherOutlet({ placementZones, context, entityType, className, itemClassName, - errorClassName, - errorBehavior = "inline", }: PluginLauncherOutletProps) { const { activateLauncher } = usePluginLauncherRuntime(); - const { launchers, contributionsByPluginId, errorMessage } = usePluginLaunchers({ + const { launchers, contributionsByPluginId } = usePluginLaunchers({ placementZones, entityType, companyId: context.companyId, enabled: !!context.companyId, }); - if (errorMessage && errorBehavior === "inline") { - return ( -
- Plugin launchers unavailable: {errorMessage} -
- ); - } - if (launchers.length === 0) return null; return ( diff --git a/ui/src/plugins/outlet-errors.test.tsx b/ui/src/plugins/outlet-errors.test.tsx index 90c6ce9343c..0ec2ff30bb5 100644 --- a/ui/src/plugins/outlet-errors.test.tsx +++ b/ui/src/plugins/outlet-errors.test.tsx @@ -4,11 +4,12 @@ import { createRoot, type Root } from "react-dom/client"; import { MemoryRouter } from "react-router-dom"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; import type { PluginUiContribution } from "@/api/plugins"; import { pluginsApi } from "@/api/plugins"; import { queryKeys } from "@/lib/queryKeys"; -import { PluginLauncherOutlet, PluginLauncherProvider } from "./launchers"; -import { _resetPluginModuleLoader, PluginSlotOutlet, registerPluginReactComponent } from "./slots"; +import { PluginLauncherOutlet, PluginLauncherProvider, usePluginLaunchers } from "./launchers"; +import { _resetPluginModuleLoader, PluginSlotOutlet, registerPluginReactComponent, usePluginSlots } from "./slots"; vi.mock("@/context/CompanyContext", () => ({ useCompany: () => ({ selectedCompany: null }) })); @@ -29,20 +30,33 @@ let root: Root; let container: HTMLDivElement; let client: QueryClient; -async function render(errorBehavior?: "inline" | "hidden") { +function HookProbe() { + const slots = usePluginSlots({ slotTypes: ["sidebar"], companyId: context.companyId }); + const launchers = usePluginLaunchers({ placementZones: ["sidebar"], companyId: context.companyId }); + return ( + + {`slots:${slots.isLoading ? "loading" : "settled"}:${slots.errorMessage ?? "-"}|launchers:${launchers.isLoading ? "loading" : "settled"}:${launchers.errorMessage ?? "-"}`} + + ); +} + +async function render() { await act(async () => root.render( - - - + + + + , )); } +const probeText = () => container.querySelector('[data-testid="probe"]')?.textContent ?? ""; + beforeEach(() => { Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); // Module and session fetches stay pending; components are registered directly. @@ -50,25 +64,41 @@ beforeEach(() => { vi.stubGlobal("fetch", vi.fn(() => new Promise(() => {}))); client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } }); container = document.createElement("div"); document.body.append(container); root = createRoot(container); - vi.spyOn(pluginsApi, "listUiContributions").mockRejectedValue(new TypeError("Failed to fetch")); }); afterEach(async () => { await act(async () => root.unmount()); container.remove(); client.clear(); _resetPluginModuleLoader(); vi.unstubAllGlobals(); vi.restoreAllMocks(); }); -it("shows inline errors by default when contributions fail to load", async () => { +it("collapses silently and stays loading when contributions cannot be fetched during an outage", async () => { + vi.spyOn(pluginsApi, "listUiContributions").mockRejectedValue(new TypeError("Failed to fetch")); await render(); - await vi.waitFor(() => expect(container.textContent).toContain("unavailable")); - expect(container.textContent).toContain("Plugin extensions unavailable: Failed to fetch"); - expect(container.textContent).toContain("Plugin launchers unavailable: Failed to fetch"); + await vi.waitFor(() => expect(client.getQueryState(queryKeys.plugins.uiContributions)?.status).toBe("error")); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); + expect(container.textContent).not.toContain("unavailable"); + expect(container.textContent).not.toContain("Failed to fetch"); + expect(container.querySelector(".text-destructive")).toBeNull(); + // Nothing has loaded yet: the hooks report loading, not an error, so pages show a placeholder. + expect(probeText()).toBe("slots:loading:-|launchers:loading:-"); }); -it("hides errors and keeps the last loaded slots and launchers when errorBehavior is hidden", async () => { +it("reports a readable message, never a raw code, for a real failure", async () => { + vi.spyOn(pluginsApi, "listUiContributions").mockRejectedValue(new ApiError("access_denied", 403, { error: "access_denied" })); + await render(); + await vi.waitFor(() => expect(client.getQueryState(queryKeys.plugins.uiContributions)?.status).toBe("error")); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); + expect(container.textContent).not.toContain("access_denied"); + expect(container.querySelector(".text-destructive")).toBeNull(); + expect(probeText()).toContain("slots:settled:You don’t have permission to do that."); + expect(probeText()).toContain("launchers:settled:You don’t have permission to do that."); +}); + +it("keeps the last loaded slots and launchers when a refetch fails", async () => { + vi.spyOn(pluginsApi, "listUiContributions").mockRejectedValue(new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" })); registerPluginReactComponent("fixture.sidebar", "SidebarItem", () => Fixture sidebar item); registerPluginReactComponent("fixture.sidebar", "SidebarPanel", () => Fixture sidebar panel); client.setQueryData(queryKeys.plugins.uiContributions, [contribution]); - await render("hidden"); + await render(); expect(container.textContent).toContain("Fixture sidebar item"); expect(container.textContent).toContain("Fixture sidebar panel"); @@ -82,4 +112,6 @@ it("hides errors and keeps the last loaded slots and launchers when errorBehavio expect(container.textContent).toContain("Fixture sidebar item"); expect(container.textContent).toContain("Fixture sidebar panel"); expect(container.textContent).toContain("Fixture launcher"); + // The slots probe stays "loading" only because the fixture module fetch never settles. + expect(probeText()).toMatch(/^slots:(?:loading|settled):-\|launchers:settled:-$/); }); diff --git a/ui/src/plugins/slots.tsx b/ui/src/plugins/slots.tsx index 689f033b591..6f0752b35ef 100644 --- a/ui/src/plugins/slots.tsx +++ b/ui/src/plugins/slots.tsx @@ -21,6 +21,7 @@ import { Component, createElement, + useCallback, useEffect, useMemo, useRef, @@ -39,6 +40,8 @@ import type { } from "@paperclipai/shared"; import { pluginsApi, type PluginUiContribution } from "@/api/plugins"; import { authApi } from "@/api/auth"; +import { describeError, isTransientError } from "@/api/errors"; +import { queryViewKind } from "@/components/QueryView"; import { queryKeys } from "@/lib/queryKeys"; import { cn } from "@/lib/utils"; import { @@ -115,9 +118,18 @@ type SlotFilters = { }; type UsePluginSlotsResult = { + /** The last loaded slots. Kept through a transient failure so chrome never disappears. */ slots: ResolvedPluginSlot[]; + /** True while nothing has loaded yet, including while reconnecting after an outage. */ isLoading: boolean; + /** + * Readable copy for a non-transient failure (`describeError`), or null. A + * transient failure never sets this: outlets collapse or render the last + * loaded slots, and the app-level connection banner explains the outage. + */ errorMessage: string | null; + error: unknown; + retry: () => void; }; /** @@ -153,11 +165,6 @@ function requiresEntityType(slotType: PluginUiSlotType): boolean { return slotType === "detailTab" || slotType === "taskDetailView" || slotType === "contextMenuItem" || slotType === "commentAnnotation" || slotType === "commentContextMenuItem" || slotType === "projectSidebarItem" || slotType === "toolbarButton"; } -function getErrorMessage(error: unknown): string { - if (error instanceof Error && error.message) return error.message; - return "Unknown error"; -} - /** * Registers a React component export for a plugin UI slot. */ @@ -649,11 +656,18 @@ function usePluginModuleLoader(contributions: PluginUiContribution[] | undefined */ export function usePluginSlots(filters: SlotFilters): UsePluginSlotsResult { const queryEnabled = filters.enabled ?? true; - const { data, isLoading: isQueryLoading, error } = useQuery({ + const query = useQuery({ queryKey: queryKeys.plugins.uiContributions, queryFn: () => pluginsApi.listUiContributions(), enabled: queryEnabled, }); + const { data, isLoading: isQueryLoading, error, refetch } = query; + // One failed fetch backs every outlet on the page, so an outage must not + // fan out into red boxes: only a real (non-transient) failure is reported. + const viewKind = queryViewKind(query); + const retry = useCallback(() => { + void refetch(); + }, [refetch]); const slotTypesKey = useMemo(() => [...filters.slotTypes].sort().join("|"), [filters.slotTypes]); @@ -695,12 +709,19 @@ export function usePluginSlots(filters: SlotFilters): UsePluginSlotsResult { }, [data, slots]); usePluginModuleLoader(contributions); const modulesLoaded = contributions ? aggregateLoadState(contributions) === "loaded" : true; - const isLoading = queryEnabled && (isQueryLoading || !modulesLoaded); + // Plugin chrome never shows red for a transient failure, even one that + // outlasts the retry policy while the server is reachable: with nothing + // loaded the outlets stay collapsed, and the next mount or focus refetches. + const transientFailure = viewKind === "error" && isTransientError(error); + const reportedError = viewKind === "error" && !transientFailure ? error : null; + const isLoading = queryEnabled && (isQueryLoading || viewKind === "reconnecting" || transientFailure || !modulesLoaded); return { slots, isLoading, - errorMessage: error ? getErrorMessage(error) : null, + errorMessage: reportedError ? describeError(reportedError).body : null, + error: reportedError, + retry, }; } @@ -905,40 +926,29 @@ type PluginSlotOutletProps = { entityType?: PluginUiSlotEntityType | null; className?: string; itemClassName?: string; - errorClassName?: string; missingBehavior?: "hidden" | "placeholder"; - /** - * `hidden` suppresses the inline error and keeps rendering the last loaded - * slots, so ambient chrome (the sidebar) stays quiet while the server is - * unreachable. - */ - errorBehavior?: "inline" | "hidden"; }; +/** + * Renders the plugin slots for a host location. An outlet never shows a + * fetch error: while the contributions cannot be loaded it renders the last + * loaded slots, or nothing if none have loaded yet. Failures are reported on + * the plugin settings pages, where someone can act on them. + */ export function PluginSlotOutlet({ slotTypes, context, entityType, className, itemClassName, - errorClassName, missingBehavior = "hidden", - errorBehavior = "inline", }: PluginSlotOutletProps) { - const { slots, errorMessage } = usePluginSlots({ + const { slots } = usePluginSlots({ slotTypes, entityType, companyId: context.companyId, }); - if (errorMessage && errorBehavior === "inline") { - return ( -
- Plugin extensions unavailable: {errorMessage} -
- ); - } - if (slots.length === 0) return null; return ( diff --git a/ui/src/plugins/usePluginData.test.tsx b/ui/src/plugins/usePluginData.test.tsx new file mode 100644 index 00000000000..0a09529a657 --- /dev/null +++ b/ui/src/plugins/usePluginData.test.tsx @@ -0,0 +1,123 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { PluginBridgeContext, usePluginData, type PluginBridgeContextValue } from "./bridge"; + +const mockPluginsApi = vi.hoisted(() => ({ bridgeGetData: vi.fn() })); +vi.mock("@/api/plugins", () => ({ pluginsApi: mockPluginsApi })); +// The hook shares the app-wide retry policy; fail fast here so a transient +// error reaches the keep-or-clear decision without a 30-second backoff. +vi.mock("@/lib/query-client", () => ({ shouldRetryRequest: () => false, retryDelayFor: () => 0 })); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +const unavailable = () => new ApiError("Paperclip is restarting.", 503, { error: "tenant_app_unavailable" }); + +const bridge: PluginBridgeContextValue = { + pluginId: "files", + hostContext: { + companyId: "company-1", + companyPrefix: null, + projectId: null, + entityId: null, + entityType: null, + userId: null, + renderEnvironment: null, + }, +}; + +type Snapshot = { data: unknown; loading: boolean; error: string | null }; + +describe("usePluginData", () => { + let container: HTMLDivElement; + let root: Root; + let snapshots: Snapshot[]; + + function Consumer({ path }: { path: string }) { + const { data, loading, error } = usePluginData<{ content: string }>("file-content", { path }); + snapshots.push({ data, loading, error: error?.code ?? null }); + return null; + } + + function render(path: string) { + act(() => { + root.render( + + + , + ); + }); + } + + const last = () => snapshots[snapshots.length - 1]!; + const settle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + snapshots = []; + mockPluginsApi.bridgeGetData.mockReset(); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + }); + + it("does not show the previous request's data when a new request fails transiently", async () => { + mockPluginsApi.bridgeGetData.mockResolvedValueOnce({ data: { content: "alpha" } }); + render("a.txt"); + await settle(); + expect(last().data).toEqual({ content: "alpha" }); + + mockPluginsApi.bridgeGetData.mockRejectedValueOnce(unavailable()); + render("b.txt"); + await settle(); + expect(mockPluginsApi.bridgeGetData).toHaveBeenLastCalledWith( + "files", + "file-content", + { path: "b.txt" }, + "company-1", + null, + ); + expect(last().loading).toBe(false); + expect(last().data).toBeNull(); + expect(last().error).not.toBeNull(); + }); + + it("keeps data through a transient failure of the same request", async () => { + mockPluginsApi.bridgeGetData.mockResolvedValueOnce({ data: { content: "alpha" } }); + let refresh!: () => void; + function Refreshing() { + const result = usePluginData<{ content: string }>("file-content", { path: "a.txt" }); + refresh = result.refresh; + snapshots.push({ data: result.data, loading: result.loading, error: result.error?.code ?? null }); + return null; + } + act(() => { + root.render( + + + , + ); + }); + await settle(); + expect(last().data).toEqual({ content: "alpha" }); + + mockPluginsApi.bridgeGetData.mockRejectedValueOnce(unavailable()); + act(() => refresh()); + await settle(); + expect(mockPluginsApi.bridgeGetData).toHaveBeenCalledTimes(2); + expect(last()).toEqual({ data: { content: "alpha" }, loading: false, error: null }); + }); +}); diff --git a/ui/storybook/stories/recovery/cloud-access-recovery.stories.tsx b/ui/storybook/stories/recovery/cloud-access-recovery.stories.tsx index b7c9fd04085..6be4a306494 100644 --- a/ui/storybook/stories/recovery/cloud-access-recovery.stories.tsx +++ b/ui/storybook/stories/recovery/cloud-access-recovery.stories.tsx @@ -1,5 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { CloudAccessError } from "@/components/CloudAccessGate"; +import { ConnectionStatusMessage } from "@/components/ConnectionStatusBanner"; const meta: Meta = { id: "app-connection-recovery", @@ -23,3 +24,17 @@ type Story = StoryObj; export const WaitingForServer: Story = {}; export const CheckingConnection: Story = { args: { retrying: true } }; export const AccessCheckFailed: Story = { args: { temporary: false } }; + +/** The app-level banner shown over an open board. */ +export const BannerReconnecting: Story = { + render: () => , +}; +export const BannerReconnectingWithPendingWrites: Story = { + render: () => , +}; +export const BannerOffline: Story = { + render: () => , +}; +export const BannerBackOnline: Story = { + render: () => , +};