diff --git a/DESIGN.md b/DESIGN.md index 57ab2ebbf..e3e11c5a6 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -28,6 +28,16 @@ The `/apis` route is a **custom, in-house API explorer** ([`src/features/instanc - **Try-it-out is credentialed cross-origin `fetch`** (`credentials: 'include'`), the same model Swagger used — which is why the **CORS warning + one-click enable** flow in [`APIDocs.tsx`](src/features/instance/apis/APIDocs.tsx) matters and must be preserved. Auth offers Cookie (session, default) / Basic / Bearer; Basic/Bearer add an `Authorization` header. - **Persistence**: the server + auth selections persist to `localStorage` under `LocalStorageKeys.ApiExplorerSettings`, **keyed by entity id**, so a credential set for one instance never applies to another. +## Operations console (the instance/cluster "Operations" tab) + +[`src/features/instance/operations/`](src/features/instance/operations/) sends one freeform operations-API body through the entity's own `getInstanceClient` client (direct, basic, or proxied — whatever every other page uses). The rules, each pinned by a test there: + +- **The typed text is the request body.** It is parsed only to validate it and read `operation`; re-serializing the parse would round integers past 2^53 (a primary key) before Harper saw them. Axios sends a string body that parses as JSON as-is, only trimmed. +- **At most one send.** It sets `skipGatewayRetry`, because the shared client replays a 502/503/504 for any body whose `operation` reads by name, and a component's `get_…` operation is no proof that it only reads. Only a request Studio refused before sending counts as unsent. Anything that reached Harper marks data stale, an error answer included, because an arbitrary operation can change something before it fails; and a request abandoned mid-flight (the page unmounted) is reported as possibly applied. +- **Only Harper's own reads skip the confirmation.** [`isBuiltInRead`](src/features/instance/operations/operationKinds.ts) is the restart gate's naming rule ([`isReadOperation`](src/integrations/api/isReadOperation.ts)) _and_ membership in the role editor's catalog of Harper's operations, because a component can register a mutating `get_…`. Everything else — unknown names, every `sql` — confirms, showing the exact body. It guards against a mistyped paste; Harper's `verifyPerms` is what authorizes each operation. +- **Operations Studio has to know about are not sent blind.** A `restart` is tracked with `markRestarting` exactly like the Restart button's (released at once on a 4xx; a 5xx can be a gateway losing the answer to a restart that ran), so the page's reads are held (an instance) or their connectivity errors muted (a cluster, which restarts one member) until `waitUntilAnsweringAfterRestart` sees the target again. `logout` is refused: it would end the session the rest of Studio is still using. Any other operation with such a side effect needs the same treatment rather than a raw send. +- **Nothing leaves the page.** Failures render inline and never pass through `console.error` or the global React Query handlers (RUM reports the former, and bodies carry passwords and secrets). The editor buffer is not persisted, and the console remounts per entity so a confirmation written for one instance can never be sent to another. After a possible write, entity queries are marked stale with `refetchType: 'none'`, never refetched into whatever the operation just did. The download is abandoned once progress passes 5 MB (XHR reports progress about every 50 ms, so this bounds, not caps, what arrives), and display truncates like the API explorer. + ## Cloud authentication presentation All cloud authentication routes use `AuthLayout`; keep form submission, validation, OAuth targets, and error handling in the existing route components/hooks. `AuthInput` keeps `FormControl` directly around the input so label/error associations and React Hook Form refs reach the DOM. React 19 passes `ref` through props; browser tests verify first-invalid-field focus. Password selectors must use an exact label match because the reveal button also has “password” in its accessible name. @@ -39,7 +49,7 @@ The wide hero artwork spans both columns beneath the form card in three independ [`restartTracker.ts`](src/lib/restart/restartTracker.ts) is the one record of which clusters and instances are restarting, so a planned outage doesn't read as a failure. Read it before adding anything that takes an instance down. - **Two sources.** Studio's own Harper `restart` runs mark and release their targets (`markRestarting`, released in `finally`) — central manager never hears about these. Container ops are central manager's: every `getClusterInfo` fetch mirrors its `RESTARTING`/`STARTING` statuses into the tracker — including a one-node cluster whose only member is restarting under an instance-only op, which leaves the cluster's own status `RUNNING`/`PARTIAL`. A new flow that takes an instance down must use one of the two, or its failures toast like before. -- **Reads are held; everything else is refused.** Every `getInstanceClient` client holds a read (`get_`/`search_`/`describe_`/`list_`/`read_`…) while its entity is `down` and sends it on recovery, but refuses a write at once with `ERR_TARGET_RESTARTING` — a write queued for minutes could land after the user gave up on it, or retried it ([`restartGate.ts`](src/lib/restart/restartGate.ts)). Token minting counts as a write on purpose, so a route guard's Fabric Connect attempt falls through to sign-in instead of stalling. Requests that drive or watch a restart must pass `skipRestartGate: true`, or they wait on themselves. A held read is cancelled if the entity's connection generation changes before release: credentials are read at send time, so it would otherwise go out as whoever connected meanwhile. +- **Reads are held; everything else is refused.** Every `getInstanceClient` client holds a read (`get_`/`search_`/`describe_`/`list_`/`read_`…) while its entity is `down` and sends it on recovery, but refuses a write at once with `ERR_TARGET_RESTARTING` — a write queued for minutes could land after the user gave up on it, or retried it ([`restartGate.ts`](src/lib/restart/restartGate.ts); the read rule is [`isReadOperation`](src/integrations/api/isReadOperation.ts), shared with the operations console). Token minting counts as a write on purpose, so a route guard's Fabric Connect attempt falls through to sign-in instead of stalling. Requests that drive or watch a restart must pass `skipRestartGate: true`, or they wait on themselves. A held read is cancelled if the entity's connection generation changes before release: credentials are read at send time, so it would otherwise go out as whoever connected meanwhile. - **Central manager's operations proxy refuses a transitional target outright** — `400 "Cluster is not active"` / `"Instance is not active"` for any `INACTIVE_STATES` status, which includes a _rolling_ restart's `RESTARTING` cluster — and relays the host manager's connection errors as a `500` carrying the raw error text. That is why a proxied client holds for marks with `proxyRefuses`, and why `isConnectivityFailure` counts those two shapes. The set also covers ERROR, FAILED and the terminal states, which no restart releases: a poll through the proxy must be gated on [`isOperationsProxyRefused`](src/components/ui/utils/badgeStatus.tsx), not `isStoppedOrTransitioning`, or it 400s every tick for as long as the page is open (HarperFast/studio#1569). - **Only query errors are muted, and only connectivity-class ones.** Mutations still toast; a 403 or a Harper 500 during a restart still toasts. - **Detection is per tab.** A page learns of a container op started elsewhere only from a cluster-record fetch, and the in-cluster studio pages don't poll the cluster, so they don't notice one started in another tab. @@ -52,6 +62,29 @@ The wide hero artwork spans both columns beneath the form card in three independ - **On 5.2+ the first instance is still written and restarted alone.** Only once it is back up on the new config is the write fanned out with `replicated: true` from that instance — even when it was the last instance to restart, because the fan-out also reaches live members Studio does not restart (not RUNNING, or failed earlier), which the per-instance path leaves on the old config. Fanning out first would leave an unproven config pending on every node, which a Cancel could no longer contain. - **A later instance skips its write only when the fan-out acknowledged that exact node** — its CM `replicationFqdn` (lower-cased) appears among the successful `replicated[]` entries. Fabric names each Harper node after that FQDN (host-manager sets `NODE_HOSTNAME` and `REPLICATION_HOSTNAME` to it). A count of acknowledgements is not proof: the origin's replication peers are not CM's restart targets, so a stale peer can stand in for a missing one. A target with no `replicationFqdn`, a rejected fan-out (the instance client rejects any response with a failed peer), or a response with no usable `replicated[]` all keep the per-instance write. +## Instance clients replay only reads + +Every `getInstanceClient` client replays a 502/503/504 up to three times (5s/10s/20s), but only for a request [`isReadRequest`](src/integrations/api/isReadOperation.ts) accepts: a GET, or a POST whose body's `operation` is a read by Harper's naming — the restart gate's rule, one predicate for both ([`retryGatewayErrors.ts`](src/integrations/api/retryGatewayErrors.ts)). A write is sent once and its first 5xx is final, because no gateway status proves the write was not applied: Harper itself answers 503 after handing an operation to a worker that exits or overruns. Do not add an exemption for a "usually safe" status or a write that looks idempotent; a caller whose write is safe to repeat retries it itself, where it can say why. + +## A write with no verdict never reads as failed + +The same rule decides what a failure says. [`isUnansweredWrite`](src/integrations/api/unansweredWrite.ts) is a request `isReadRequest` rejects, answered 502/503/504 or not at all (`ERR_NETWORK`, a timeout). [`describeError`](src/react-query/queryClient.ts) checks it before reading any body and returns "Outcome unknown … check whether it took effect before you try again", so the global mutation toast and every `describeError` caller say so with no copy of their own. Gate on status, never body (a gateway's body describes the gateway), and keep it to display: rewriting `error.message` would defeat `shouldKeepEvent`, which recognises timeouts and `Network Error` by message. + +- **A caller that adds its own failure toast to a mutation** asks [`shouldToastOwnFailure`](src/react-query/queryClient.ts) first, so "Failed to …" never sits beside the global warning. A caller with no mutation behind it (rename, the chat tools), or one whose mutation opts out of the global toast (the deploy mutation behind import, redeploy and staged-release activation), shows `describeError`'s text itself, under a title from [`deployFailureTitle`](src/features/instance/applications/modals/reportDeployHealth.ts) for a deploy ("Couldn't confirm … was redeployed", never "Failed to …"), and a loop that tallies one write per target (cluster restart, rolling config, licenses) counts these apart from failures. +- **Do not re-wrap an AxiosError into a plain `Error`** for a write: the predicate needs its `config`. `secrets.ts` passes an unanswered one through as is. +- **A restart with no verdict stays marked until its target answers again or the watch gives up**, as an accepted one does, because a gateway can lose the answer to a restart that ran. The Restart button holds the mark while it watches, and the cluster loop waits on that member before restarting the next. +- **These toasts are never collapsed** onto a shared id: each warns about a different write, and a read timing out alongside one must not replace it through `request-timeout`. +- **A read that is costly to repeat** stays a read here and opts out of the replay instead (`package_component`, `skipGatewayRetry`). + +## A restart Harper accepted never reads as failed + +Harper's answer to the restart request is its only verdict on the restart: `restart` answers before it tears down (harper `bin/restart.ts` schedules the teardown 50ms out so the reply can go first) and `restart_service` once its job has started (`executeJob`). So [`restartInstance`](src/integrations/api/instance/status/restartInstance.ts) rejects with the request's own error only when that request failed; after a 2xx, a target Studio never sees answer the `user_info` watch successfully rejects with [`RestartUnconfirmedError`](src/integrations/api/instance/status/restartUnconfirmed.ts), which reads "Not back yet", never "Failed", and is never a reason to send the restart again. + +- **The watch outlasts the usual recovery time** (one settle, then two probe schedules), and `onSlowToReturn` fires once between them so a caller can say it is still waiting. Every restart mark uses `RESTART_MARK_TTL_MS`, derived from the same constants, so no mark lapses while its watch still runs; the probes skip the gateway replay for the same reason. +- **Only a successful probe counts as back, never a 401/403.** Through central manager's operations proxy a 403 is CM refusing the caller before it contacts the instance (`notAllowed()` in its `HDBInstance.js`), so it says nothing about the target; reading it as "back" would report a restart Studio never saw finish. That is also why the copy says Studio "couldn't confirm it came back", not that the target never answered. +- **A one-at-a-time loop stops at a member that is not back** (cluster restart, rolling config update): restarting the next one could leave no member serving. Both loops also wait on a member whose restart got no verdict, and stop if that follow-up watch gives up too: it may have restarted and still be down, or, in a config update, restarted onto a configuration that broke it. Either report names the members it left alone, and it, like the no-verdict warning, outranks Cancel, which would otherwise hide the warning not to restart again; the report then says which members Cancel left. A Cancel clicked during a member's configuration write stops before its restart, and the report says the configuration may be pending there. On a 5.2+ cluster the config update never fans out from a canary that is not back, since it never proved it can run the configuration; a stop after the fan-out keeps the note that the members it left may already have the new configuration saved. +- **The chat tools return `success: true`** for it: their action was carried out, and `false` invites the agent to repeat it. + ## Cluster upgrade picker — which versions an existing cluster is offered [`buildUpgradeVersionOptions`](src/features/clusters/upsert/lib/buildUpgradeVersionOptions.ts) is the one place that decides, and its test file is the spec. Two rules, and they differ on purpose: diff --git a/src/features/auth/describeAuthFailure.ts b/src/features/auth/describeAuthFailure.ts index c866db7ba..fd957b168 100644 --- a/src/features/auth/describeAuthFailure.ts +++ b/src/features/auth/describeAuthFailure.ts @@ -1,3 +1,4 @@ +import { NO_RESPONSE_CODES } from '@/integrations/api/unansweredWrite'; import { describeError } from '@/react-query/queryClient'; import { AxiosError } from 'axios'; @@ -15,24 +16,15 @@ export const OUTCOME_UNKNOWN_MESSAGE = "We didn't get an answer from the server, so we can't tell whether that went through."; // 503 alone: the server is declining requests, so it very likely never processed this one and a -// plain retry is honest. (`curryRetryGatewayErrors` also classes 502/504 as retryable, but it is -// installed on instance clients only, never on `apiClient` — nothing retries these calls -// automatically, and the retry this copy invites is the user's own.) +// plain retry is honest. (`curryRetryGatewayErrors` replays 502/503/504, but only for reads and +// only on instance clients, never on `apiClient` — nothing retries these calls automatically, and +// the retry this copy invites is the user's own.) const TRANSIENT_STATUSES: ReadonlySet = new Set([503]); // Every form using this submits a non-idempotent POST, and each of these means the request had // already been handed upstream, so the write may have applied (RFC 9110 §9.2.2 — a retry is only // safe once you know it did not). Repeating blindly turns a completed sign-up into a 409 with the // verification mail already sent (#1668). const OUTCOME_UNKNOWN_STATUSES: ReadonlySet = new Set([502, 504]); -// Axios reports a request that got no response as ERR_NETWORK — including a CORS rejection, and -// including a connection that dropped *after* the POST was applied — so it cannot be described as -// "we couldn't reach the server". It is the same unknown outcome. ECONNABORTED/ETIMEDOUT are the -// timeout pair; which one axios uses depends on `transitional.clarifyTimeoutError`. -const OUTCOME_UNKNOWN_CODES: ReadonlySet = new Set([ - AxiosError.ERR_NETWORK, - AxiosError.ETIMEDOUT, - AxiosError.ECONNABORTED, -]); /** * Our own copy for a failure the server's own words shouldn't answer, or `undefined` to defer. @@ -64,7 +56,7 @@ export function describeRetryableAuthFailure(rawErr: unknown, outcomeUnknownReco // ERR_CANCELED is a local abort that reached nothing, and a config fault (ERR_INVALID_URL, // ERR_BAD_OPTION) never left the browser — none of those is ours to describe. const code = axiosErr?.code; - return typeof code === 'string' && OUTCOME_UNKNOWN_CODES.has(code) ? outcomeUnknown : undefined; + return typeof code === 'string' && NO_RESPONSE_CODES.has(code) ? outcomeUnknown : undefined; } /** Display text for an auth-form rejection, preferring whatever the server said. */ diff --git a/src/features/cluster/domains/Management.test.tsx b/src/features/cluster/domains/Management.test.tsx index b2e99421d..2e460097e 100644 --- a/src/features/cluster/domains/Management.test.tsx +++ b/src/features/cluster/domains/Management.test.tsx @@ -3,7 +3,7 @@ */ import { MutationCache, onlineManager, QueryCache, QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { act, fireEvent, render, waitFor, within } from '@testing-library/react'; -import { AxiosError } from 'axios'; +import { AxiosError, type InternalAxiosRequestConfig } from 'axios'; import { PropsWithChildren, Suspense } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -30,6 +30,7 @@ vi.mock('@/hooks/usePermissions', () => ({ const { instancePost } = vi.hoisted(() => ({ instancePost: vi.fn() })); vi.mock('@/config/getInstanceClient', () => ({ getInstanceClient: () => ({ post: instancePost }) })); +import { describeUnansweredWrites, OUTCOME_UNKNOWN_TITLE } from '@/integrations/api/unansweredWrite'; import { mutationErrorHandler, queryErrorHandler } from '@/react-query/queryClient'; import { toast } from 'sonner'; @@ -1121,3 +1122,49 @@ describe('DomainsManagement — the Remove Domain tooltip', () => { expect(pendingCell.querySelectorAll('[tabindex]')).toHaveLength(0); }); }); + +describe('validating pending domains', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('says a validation that got no answer may have gone through, and refreshes the list to show it', async () => { + get.mockImplementation((url: string) => + url.startsWith('/Domain/') + ? Promise.resolve({ + data: [ + { id: 'dom-1', domain: 'a.example.com', status: 'PENDING_VALIDATION' }, + { id: 'dom-2', domain: 'b.example.com', status: 'PENDING_VALIDATION' }, + ], + }) + : Promise.resolve({ data: { id: 'clu-1', organizationId: 'org-1', status: 'RUNNING', domainIds: [] } }) + ); + post.mockImplementation((url: string) => { + const config = { method: 'post', url } as InternalAxiosRequestConfig; + return Promise.reject( + url.includes('dom-1') + ? new AxiosError('timeout of 60000ms exceeded', AxiosError.ECONNABORTED, config) + : new AxiosError('Request failed with status code 400', AxiosError.ERR_BAD_REQUEST, config, undefined, { + status: 400, + statusText: '', + data: 'TXT record not found', + headers: {}, + config, + }), + ); + }); + const { container } = renderManagement(); + const validateButton = () => container.querySelector('button[accesskey="r"]'); + await waitFor(() => expect(validateButton()?.disabled).toBe(false)); + const fetchesBefore = domainListFetches(); + + fireEvent.click(validateButton()!); + + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(toast.error).toHaveBeenCalledWith(OUTCOME_UNKNOWN_TITLE, { + description: `${describeUnansweredWrites(1)} 1 other domain failed to validate.`, + id: 'validatingDomains', + }); + expect(domainListFetches()).toBeGreaterThan(fetchesBefore); + }); +}); diff --git a/src/features/cluster/domains/Management.tsx b/src/features/cluster/domains/Management.tsx index 0fc8204d6..d8a71da46 100644 --- a/src/features/cluster/domains/Management.tsx +++ b/src/features/cluster/domains/Management.tsx @@ -40,6 +40,7 @@ import { getOrganizationDomainsQueryOptions } from '@/features/organization/quer import { useOrganizationRolePermissions } from '@/hooks/usePermissions'; import { useRefreshClick } from '@/hooks/useRefreshClick'; import { SchemaOrganizationDomain } from '@/integrations/api/api.patch'; +import { describeUnansweredWrites, isUnansweredWrite, OUTCOME_UNKNOWN_TITLE } from '@/integrations/api/unansweredWrite'; import { pluralize } from '@/lib/pluralize'; import { describeError } from '@/react-query/queryClient'; import { zodResolver } from '@hookform/resolvers/zod'; @@ -235,6 +236,7 @@ export function DomainsManagement() { const id = 'validatingDomains'; let checked = 0; let failed = 0; + let unanswered = 0; for (const pendingDomain of pendingDomains) { try { toast.loading(message, { @@ -242,11 +244,24 @@ export function DomainsManagement() { id, }); await validateDomainInOrganization(pendingDomain.id); - } catch { - failed += 1; + } catch (error) { + if (isUnansweredWrite(error)) { + unanswered += 1; + } else { + failed += 1; + } } } - if (failed > 0) { + if (unanswered > 0) { + await refetch(); + toast.error(OUTCOME_UNKNOWN_TITLE, { + description: [ + describeUnansweredWrites(unanswered), + failed > 0 && `${pluralize(failed, 'other domain', 'other domains')} failed to validate.`, + ].filter(Boolean).join(' '), + id, + }); + } else if (failed > 0) { toast.error('Validation failed!', { description: `Please make sure the TXT record has been put in place. You may need to wait a bit for the DNS change to propagate.`, diff --git a/src/features/clusters/components/ClusterCard.tsx b/src/features/clusters/components/ClusterCard.tsx index 9c10485be..596ecd766 100644 --- a/src/features/clusters/components/ClusterCard.tsx +++ b/src/features/clusters/components/ClusterCard.tsx @@ -28,6 +28,7 @@ import { excludeFalsy } from '@/lib/arrays/excludeFalsy'; import { LocalStorageKeys } from '@/lib/storage/localStorageKeys'; import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage'; import { getOperationsUrlForCluster } from '@/lib/urls/getOperationsUrlForCluster'; +import { shouldToastOwnFailure } from '@/react-query/queryClient'; import { useQueryClient } from '@tanstack/react-query'; import { Link, useRouter } from '@tanstack/react-router'; import { @@ -146,17 +147,19 @@ export function ClusterCard({ item: summary }: { item: ClusterListItem }) { }); setIsTerminateClusterModalOpen(false); }, - onError: () => { - toast.error('Error', { - description: isSelfManaged - ? `Failed to remove cluster: ${cluster.name}` - : `Failed to terminate cluster: ${cluster.name}.`, - duration: 5000, - action: { - label: 'Dismiss', - onClick: () => toast.dismiss(), - }, - }); + onError: (error) => { + if (shouldToastOwnFailure(error)) { + toast.error('Error', { + description: isSelfManaged + ? `Failed to remove cluster: ${cluster.name}` + : `Failed to terminate cluster: ${cluster.name}.`, + duration: 5000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } setIsTerminateClusterModalOpen(false); }, }); diff --git a/src/features/instance/InstanceNavBar.test.tsx b/src/features/instance/InstanceNavBar.test.tsx new file mode 100644 index 000000000..89cc61d68 --- /dev/null +++ b/src/features/instance/InstanceNavBar.test.tsx @@ -0,0 +1,42 @@ +/** @vitest-environment jsdom */ +import { InstanceNavBar } from '@/features/instance/InstanceNavBar'; +import { cleanup, render, screen } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const { permission } = vi.hoisted(() => ({ permission: { canManage: true } })); + +vi.mock('@/hooks/usePermissions', () => ({ useInstanceManagePermission: () => permission.canManage })); +vi.mock('@/config/useInstanceClient', () => ({ useInstanceClientIdParams: () => ({}) })); +vi.mock('@/components/Breadcrumbs', () => ({ Breadcrumbs: () => null })); +vi.mock('@tanstack/react-query', async (importOriginal) => ({ + ...(await importOriginal()), + useQuery: () => ({ data: undefined }), +})); +vi.mock('@tanstack/react-router', () => ({ + Link: ({ to, children, 'aria-label': ariaLabel }: { to: string; children: ReactNode; 'aria-label'?: string }) => ( + {children} + ), + useLoaderData: () => ({ version: '5.3.0' }), + useParams: () => ({ organizationId: 'org-1', clusterId: 'clu-1', instanceId: 'ins-1' }), +})); + +afterEach(cleanup); + +describe('InstanceNavBar', () => { + it('offers the operations console to a super user', () => { + permission.canManage = true; + render(); + + expect(screen.getByRole('link', { name: 'Operations' }).getAttribute('href')) + .toBe('/org-1/clu-1/instance/ins-1/operations'); + }); + + it('hides it from anyone else', () => { + permission.canManage = false; + render(); + + expect(screen.queryByRole('link', { name: 'Operations' })).toBeNull(); + expect(screen.getByRole('link', { name: 'Databases' })).toBeTruthy(); + }); +}); diff --git a/src/features/instance/InstanceNavBar.tsx b/src/features/instance/InstanceNavBar.tsx index 2bf185988..8581cb185 100644 --- a/src/features/instance/InstanceNavBar.tsx +++ b/src/features/instance/InstanceNavBar.tsx @@ -18,7 +18,16 @@ import { wasAReleasedBeforeB } from '@/lib/string/wasAReleasedBeforeB'; import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage'; import { useQuery } from '@tanstack/react-query'; import { Link, useLoaderData, useParams } from '@tanstack/react-router'; -import { DatabaseIcon, GaugeIcon, Menu, NotepadTextIcon, PackageIcon, ServerIcon, SettingsIcon } from 'lucide-react'; +import { + DatabaseIcon, + GaugeIcon, + Menu, + NotepadTextIcon, + PackageIcon, + ServerIcon, + SettingsIcon, + SquareTerminalIcon, +} from 'lucide-react'; import { ReactNode, useMemo } from 'react'; interface Link { @@ -67,6 +76,11 @@ export function InstanceNavBar() { name: 'APIs', icon: , }, + canManage && { + to: buildAbsoluteLinkToPage(params, 'operations'), + name: 'Operations', + icon: , + }, canManage && statusAvailable && { to: buildAbsoluteLinkToPage(params, 'status'), icon: , diff --git a/src/features/instance/applications/components/Chat/tools/createApp/execute.ts b/src/features/instance/applications/components/Chat/tools/createApp/execute.ts index 1f6e53f5a..b8bdf84f0 100644 --- a/src/features/instance/applications/components/Chat/tools/createApp/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/createApp/execute.ts @@ -1,6 +1,7 @@ import { templates } from '@/features/instance/applications/components/NewApplication/templates'; import { addComponent } from '@/integrations/api/instance/applications/addComponent'; import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; +import { RestartUnconfirmedError } from '@/integrations/api/instance/status/restartUnconfirmed'; import { findBy } from '@/lib/arrays/findBy'; import { emitToListeners } from '@/lib/events/listener'; import { toKebabCase } from '@/lib/string/to-kebab-case'; @@ -8,6 +9,7 @@ import { inputSchema } from '@harperfast/agent-tools/tools/createApp/inputSchema import { toast } from 'sonner'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError, toastToolFailure } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -37,9 +39,10 @@ export async function execute( template: template.npm || template.githubUrl, }); } catch (err) { + toastToolFailure(toastId, err); return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } @@ -58,9 +61,20 @@ export async function execute( replicated: instanceClientParams.entityType === 'cluster', }); } catch (err) { + toastToolFailure(toastId, err); + if (err instanceof RestartUnconfirmedError) { + // The app exists either way; `success: false` would invite creating it again. + return { + success: true, + message: `App "${name}" created. Harper accepted the restart that loads it, but Studio couldn't confirm ` + + "the instance came back, so it may still be starting. Don't create the app or restart again until " + + 'its status shows it is back.', + webURL: baseURL, + }; + } return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } diff --git a/src/features/instance/applications/components/Chat/tools/deleteTableRecords/execute.ts b/src/features/instance/applications/components/Chat/tools/deleteTableRecords/execute.ts index cb41f2ff9..683337bb5 100644 --- a/src/features/instance/applications/components/Chat/tools/deleteTableRecords/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/deleteTableRecords/execute.ts @@ -3,6 +3,7 @@ import { queryClient } from '@/react-query/queryClient'; import { inputSchema } from '@harperfast/agent-tools/tools/deleteTableRecords/inputSchema'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -24,7 +25,7 @@ export async function execute( } catch (err) { return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } } diff --git a/src/features/instance/applications/components/Chat/tools/describeToolError.ts b/src/features/instance/applications/components/Chat/tools/describeToolError.ts new file mode 100644 index 000000000..31c027455 --- /dev/null +++ b/src/features/instance/applications/components/Chat/tools/describeToolError.ts @@ -0,0 +1,13 @@ +import { isUnansweredWrite, UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; +import { describeError } from '@/react-query/queryClient'; +import { toast } from 'sonner'; + +export function describeToolError(err: unknown): string { + return isUnansweredWrite(err) ? UNANSWERED_WRITE_MESSAGE : String(err); +} + +/** A loading toast never dismisses itself, so a failed tool must replace its progress toast. */ +export function toastToolFailure(toastId: string | number, err: unknown) { + const { title, description } = describeError(err); + toast.error(title, { id: toastId, description, duration: 10_000 }); +} diff --git a/src/features/instance/applications/components/Chat/tools/dropComponentFile/execute.ts b/src/features/instance/applications/components/Chat/tools/dropComponentFile/execute.ts index fe9715563..0d022447e 100644 --- a/src/features/instance/applications/components/Chat/tools/dropComponentFile/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/dropComponentFile/execute.ts @@ -2,6 +2,7 @@ import { dropComponent } from '@/integrations/api/instance/applications/dropComp import { inputSchema } from '@harperfast/agent-tools/tools/dropComponentFile/inputSchema'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -19,7 +20,7 @@ export async function execute( } catch (err) { return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } } diff --git a/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts b/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts index 27b766e17..6ec859174 100644 --- a/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts @@ -29,6 +29,13 @@ export async function execute( }; } + if (url.endsWith('/operations')) { + return { + success: true, + description: 'sending raw requests from the operations API console', + }; + } + if (url.endsWith('/status')) { return { success: true, diff --git a/src/features/instance/applications/components/Chat/tools/insertTableRecords/execute.ts b/src/features/instance/applications/components/Chat/tools/insertTableRecords/execute.ts index fd6af4be4..dabdcf37c 100644 --- a/src/features/instance/applications/components/Chat/tools/insertTableRecords/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/insertTableRecords/execute.ts @@ -3,6 +3,7 @@ import { queryClient } from '@/react-query/queryClient'; import { inputSchema } from '@harperfast/agent-tools/tools/insertTableRecords/inputSchema'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -24,7 +25,7 @@ export async function execute( } catch (err) { return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } } diff --git a/src/features/instance/applications/components/Chat/tools/restartHTTPService/execute.ts b/src/features/instance/applications/components/Chat/tools/restartHTTPService/execute.ts index fb054f514..d2e5abb91 100644 --- a/src/features/instance/applications/components/Chat/tools/restartHTTPService/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/restartHTTPService/execute.ts @@ -1,8 +1,10 @@ import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; +import { RestartUnconfirmedError } from '@/integrations/api/instance/status/restartUnconfirmed'; import { inputSchema } from '@harperfast/agent-tools/tools/restartHTTPService/inputSchema'; import { toast } from 'sonner'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError, toastToolFailure } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -20,9 +22,20 @@ export async function execute( replicated: instanceClientParams.entityType === 'cluster', }); } catch (err) { + toastToolFailure(toastId, err); + if (err instanceof RestartUnconfirmedError) { + // `success` is whether the tool's own action was carried out, and Harper accepted the restart. + return { + success: true, + message: "Harper accepted the restart, but Studio couldn't confirm the HTTP service came back, so it may " + + "still be starting. Don't restart it again until its status shows it is back, or it goes down a " + + 'second time.', + webURL: baseURL, + }; + } return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } diff --git a/src/features/instance/applications/components/Chat/tools/restartTools.test.ts b/src/features/instance/applications/components/Chat/tools/restartTools.test.ts new file mode 100644 index 000000000..a3aae6441 --- /dev/null +++ b/src/features/instance/applications/components/Chat/tools/restartTools.test.ts @@ -0,0 +1,101 @@ +import { + RESTART_UNCONFIRMED_MESSAGE, + RESTART_UNCONFIRMED_TITLE, + RestartUnconfirmedError, +} from '@/integrations/api/instance/status/restartUnconfirmed'; +import { AxiosError } from 'axios'; +import { toast } from 'sonner'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ExecuteParams } from '../types/executeParams'; +import { execute as createApp } from './createApp/execute'; +import { execute as restartHTTPService } from './restartHTTPService/execute'; + +vi.mock('@/integrations/api/instance/status/restartInstance', () => ({ restartInstance: vi.fn() })); +vi.mock('@/integrations/api/instance/applications/addComponent', () => ({ addComponent: vi.fn() })); +vi.mock('@/lib/events/listener', () => ({ emitToListeners: vi.fn() })); +vi.mock('sonner', () => ({ + toast: { loading: vi.fn(() => 'tool-toast'), error: vi.fn(), success: vi.fn() }, +})); + +import { addComponent } from '@/integrations/api/instance/applications/addComponent'; +import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; + +function params(input: T): ExecuteParams { + return { + input, + instanceClientParams: { entityType: 'instance', entityId: 'ins-1' } as ExecuteParams['instanceClientParams'], + baseURL: 'https://ins-1.example.com', + params: {}, + }; +} + +const notBackYet = () => new RestartUnconfirmedError(new AxiosError('Network Error', AxiosError.ERR_NETWORK)); + +function refused() { + const config = { method: 'post', data: { operation: 'restart_service' } } as never; + return new AxiosError('Request failed with status code 403', AxiosError.ERR_BAD_REQUEST, config, undefined, { + status: 403, + statusText: '', + data: { error: 'This operation is restricted to super users' }, + headers: {}, + config, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(addComponent).mockResolvedValue({} as never); +}); + +describe('restartHTTPService', () => { + it('tells the agent a restart Harper accepted is not back yet, and not to restart it again', async () => { + vi.mocked(restartInstance).mockRejectedValue(notBackYet()); + + const output = await restartHTTPService(params({})); + + expect(output.success).toBe(true); + expect(output.message).not.toMatch(/^Error/); + expect(output.message).toContain("Don't restart it again"); + expect(restartInstance).toHaveBeenCalledOnce(); + expect(toast.error).toHaveBeenCalledWith( + RESTART_UNCONFIRMED_TITLE, + expect.objectContaining({ + id: 'tool-toast', + description: RESTART_UNCONFIRMED_MESSAGE, + }), + ); + }); + + it('still reports a refused restart as an error, and clears its progress toast', async () => { + vi.mocked(restartInstance).mockRejectedValue(refused()); + + const output = await restartHTTPService(params({})); + + expect(output.success).toBe(false); + expect(output.message).toMatch(/^Error: /); + expect(toast.error).toHaveBeenCalledWith('Error', expect.objectContaining({ id: 'tool-toast' })); + }); +}); + +describe('createApp', () => { + it('keeps the fact that the app was created when its restart is not back yet', async () => { + vi.mocked(restartInstance).mockRejectedValue(notBackYet()); + + const output = await createApp(params({ name: 'My App', type: 'vanilla' as const })); + + expect(output.success).toBe(true); + expect(output.message).toMatch(/^App "my-app" created\. Harper accepted the restart/); + expect(output.message).toContain("Don't create the app or restart again"); + expect(addComponent).toHaveBeenCalledOnce(); + }); + + it('clears its progress toast when the app could not be created', async () => { + vi.mocked(addComponent).mockRejectedValue(new Error('Component my-app already exists')); + + const output = await createApp(params({ name: 'My App', type: 'vanilla' as const })); + + expect(output.success).toBe(false); + expect(restartInstance).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ id: 'tool-toast' })); + }); +}); diff --git a/src/features/instance/applications/components/Chat/tools/setComponentFile/execute.ts b/src/features/instance/applications/components/Chat/tools/setComponentFile/execute.ts index 5b5b02af1..614ee120c 100644 --- a/src/features/instance/applications/components/Chat/tools/setComponentFile/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/setComponentFile/execute.ts @@ -4,6 +4,7 @@ import { queryClient } from '@/react-query/queryClient'; import { inputSchema } from '@harperfast/agent-tools/tools/setComponentFile/inputSchema'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -27,7 +28,7 @@ export async function execute( } catch (err) { return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } } diff --git a/src/features/instance/applications/components/Chat/tools/updateTableRecords/execute.ts b/src/features/instance/applications/components/Chat/tools/updateTableRecords/execute.ts index 730c8fbda..fe8fe2302 100644 --- a/src/features/instance/applications/components/Chat/tools/updateTableRecords/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/updateTableRecords/execute.ts @@ -3,6 +3,7 @@ import { queryClient } from '@/react-query/queryClient'; import { inputSchema } from '@harperfast/agent-tools/tools/updateTableRecords/inputSchema'; import { z } from 'zod'; import { ExecuteParams } from '../../types/executeParams'; +import { describeToolError } from '../describeToolError'; import { Output } from './output'; export async function execute( @@ -24,7 +25,7 @@ export async function execute( } catch (err) { return { success: false, - message: `Error: ${err}`, + message: `Error: ${describeToolError(err)}`, }; } } diff --git a/src/features/instance/applications/components/NewApplication/useImportApplication.test.ts b/src/features/instance/applications/components/NewApplication/useImportApplication.test.ts index e7d4e149c..9ce0e9a2f 100644 --- a/src/features/instance/applications/components/NewApplication/useImportApplication.test.ts +++ b/src/features/instance/applications/components/NewApplication/useImportApplication.test.ts @@ -52,6 +52,7 @@ vi.mock('@/integrations/api/instance/applications/deployComponentStream', () => import { deployComponentStream } from '@/integrations/api/instance/applications/deployComponentStream'; import { SSEInconclusiveError } from '@/integrations/api/sse/errors'; +import { UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; import { mutationErrorHandler } from '@/react-query/queryClient'; import { MutationCache, QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { act, renderHook, waitFor } from '@testing-library/react'; @@ -260,6 +261,37 @@ describe('useImportApplication', () => { expect(mocks.post).toHaveBeenCalledWith('/', expect.objectContaining({ activate: false }), expect.anything()); }); + it.each([ + [false, "Couldn't confirm my-app was imported"], + [true, "Couldn't confirm my-app was staged"], + ])('never calls a deploy that got no verdict failed (stage: %s)', async (stageOnly, title) => { + mocks.canStage.enabled = stageOnly; + const config = { + headers: new AxiosHeaders(), + method: 'post', + data: JSON.stringify({ operation: 'deploy_component' }), + } as InternalAxiosRequestConfig; + mocks.post.mockRejectedValue(new AxiosError('timeout of 300000ms exceeded', AxiosError.ECONNABORTED, config)); + const { result } = renderImport(); + + act(() => result.current.importApplication({ contents: { ...contents, stageOnly }, project: 'my-app' })); + + await waitFor(() => expect(result.current.importFailure).toEqual({ title, message: UNANSWERED_WRITE_MESSAGE })); + expect(vi.mocked(toast.error)).not.toHaveBeenCalled(); + // It may have imported, so the tree the message says to check is reloaded. + expect(mocks.editorView.reloadRootEntries).toHaveBeenCalledOnce(); + }); + + it('reloads nothing after an import Harper refused', async () => { + mocks.post.mockRejectedValue(serverFailure({ error: 'Component my-app could not be loaded' })); + const { result } = renderImport(); + + act(() => result.current.importApplication({ contents, project: 'my-app' })); + + await waitFor(() => expect(result.current.importFailure).toBeDefined()); + expect(mocks.editorView.reloadRootEntries).not.toHaveBeenCalled(); + }); + describe('a rollout that settles after the form has gone', () => { function deployThenFollow(job: () => Promise) { mocks.post.mockImplementation((_url: string, body: { operation: string }) => diff --git a/src/features/instance/applications/components/NewApplication/useImportApplication.ts b/src/features/instance/applications/components/NewApplication/useImportApplication.ts index 83ab4cd48..876b7c191 100644 --- a/src/features/instance/applications/components/NewApplication/useImportApplication.ts +++ b/src/features/instance/applications/components/NewApplication/useImportApplication.ts @@ -6,12 +6,17 @@ import { useStagedDeploymentsAvailable, useSupportsDeploymentSSE, } from '@/features/instance/applications/hooks/useSupportsDeploymentSSE'; -import { reportDeployOutcome, reportStaged } from '@/features/instance/applications/modals/reportDeployHealth'; +import { + deployFailureTitle, + reportDeployOutcome, + reportStaged, +} from '@/features/instance/applications/modals/reportDeployHealth'; import { useDeployComponentMutation } from '@/integrations/api/instance/applications/deployComponent'; import { DeployComponentResult } from '@/integrations/api/instance/applications/deployComponentStream'; import { deployFailureDetails } from '@/integrations/api/instance/applications/deployOutcome'; import { DeployRolloutError } from '@/integrations/api/instance/applications/restartJob'; import { SSEInconclusiveError } from '@/integrations/api/sse/errors'; +import { isUnansweredWrite } from '@/integrations/api/unansweredWrite'; import { invalidateEntityQueries } from '@/react-query/invalidateEntityQueries'; import { describeError } from '@/react-query/queryClient'; import { useQueryClient } from '@tanstack/react-query'; @@ -149,7 +154,12 @@ export function useImportApplication( }); return; } - const title = `Failed to ${stage ? 'stage' : 'import'} ${project}`; + const title = deployFailureTitle(error, project, stage ? 'stage' : 'import'); + if (isUnansweredWrite(error)) { + // It may have imported, so what the message says to check must be current. + void invalidateEntityQueries(queryClient, entityId); + void reloadRootEntries(); + } const { message } = describeError(error); if (formEntity.current === entityId) { setImportFailure({ entityId, failure: { title, message, deploymentId } }); @@ -174,7 +184,17 @@ export function useImportApplication( duration: 5_000, }); } - }, [mutateAsync, sseDeploy, canStage, instanceParams, runHealthCheck, onImported, recordAction]); + }, [ + mutateAsync, + sseDeploy, + canStage, + instanceParams, + runHealthCheck, + onImported, + recordAction, + queryClient, + reloadRootEntries, + ]); const importApplication = useCallback((args: ImportArgs) => void runImport(args), [runImport]); diff --git a/src/features/instance/applications/hooks/useRenameFiles.test.tsx b/src/features/instance/applications/hooks/useRenameFiles.test.tsx new file mode 100644 index 000000000..62565e7d1 --- /dev/null +++ b/src/features/instance/applications/hooks/useRenameFiles.test.tsx @@ -0,0 +1,63 @@ +/** @vitest-environment jsdom */ +import { dropComponent } from '@/integrations/api/instance/applications/dropComponent'; +import { OUTCOME_UNKNOWN_TITLE, UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; +import { renderHook } from '@testing-library/react'; +import { AxiosError, type InternalAxiosRequestConfig } from 'axios'; +import { toast } from 'sonner'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useRenameFiles } from './useRenameFiles'; + +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => ({ entityId: 'ins-1', entityType: 'instance' }), +})); +vi.mock('@/features/instance/applications/hooks/useEditorView', () => ({ + useEditorView: () => ({ reloadRootEntries: vi.fn(), setFocusedItem: vi.fn(), setSelectedItems: vi.fn() }), +})); +vi.mock('@/integrations/api/instance/applications/getComponentFile', () => ({ + getComponentFile: vi.fn().mockResolvedValue({ message: 'Y29udGVudHM=' }), +})); +vi.mock('@/integrations/api/instance/applications/setComponentFile', () => ({ + setComponentFile: vi.fn().mockResolvedValue({}), +})); +vi.mock('@/integrations/api/instance/applications/dropComponent', () => ({ dropComponent: vi.fn() })); +vi.mock('sonner', () => ({ toast: { loading: vi.fn(() => 'rename-toast'), error: vi.fn(), success: vi.fn() } })); + +const dropSent = { + method: 'post', + data: JSON.stringify({ operation: 'drop_component' }), +} as InternalAxiosRequestConfig; + +function rename() { + const { result } = renderHook(() => useRenameFiles()); + return result.current([{ from: 'app/old.js', to: 'app/new.js' }]); +} + +describe('useRenameFiles', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('says a rename whose last write got no answer may have gone through', async () => { + vi.mocked(dropComponent).mockRejectedValue( + new AxiosError('timeout of 60000ms exceeded', AxiosError.ECONNABORTED, dropSent), + ); + + await expect(rename()).resolves.toBe(false); + + expect(toast.error).toHaveBeenCalledWith( + OUTCOME_UNKNOWN_TITLE, + expect.objectContaining({ id: 'rename-toast', description: UNANSWERED_WRITE_MESSAGE }), + ); + }); + + it('still says a refused rename failed', async () => { + vi.mocked(dropComponent).mockRejectedValue(new Error('Unable to locate project')); + + await expect(rename()).resolves.toBe(false); + + expect(toast.error).toHaveBeenCalledWith( + 'Rename Failed', + expect.objectContaining({ id: 'rename-toast', description: 'Unable to locate project' }), + ); + }); +}); diff --git a/src/features/instance/applications/hooks/useRenameFiles.tsx b/src/features/instance/applications/hooks/useRenameFiles.tsx index 67380b1a8..64a458428 100644 --- a/src/features/instance/applications/hooks/useRenameFiles.tsx +++ b/src/features/instance/applications/hooks/useRenameFiles.tsx @@ -4,7 +4,9 @@ import { useEditorView } from '@/features/instance/applications/hooks/useEditorV import { dropComponent } from '@/integrations/api/instance/applications/dropComponent'; import { getComponentFile } from '@/integrations/api/instance/applications/getComponentFile'; import { setComponentFile } from '@/integrations/api/instance/applications/setComponentFile'; +import { isUnansweredWrite } from '@/integrations/api/unansweredWrite'; import { pluralize } from '@/lib/pluralize'; +import { describeError } from '@/react-query/queryClient'; import { useCallback } from 'react'; import { ExternalToast, toast } from 'sonner'; @@ -98,9 +100,13 @@ export function useRenameFiles() { stepForward(); } } catch (error) { - toast.error('Rename Failed', { - id: toastConfig.id, + const { title, description } = isUnansweredWrite(error) ? describeError(error) : { + title: 'Rename Failed', description: error instanceof Error ? error.message : 'An unexpected error occurred while renaming.', + }; + toast.error(title, { + id: toastConfig.id, + description, duration: 10000, action: { label: 'OK', onClick: () => undefined }, }); diff --git a/src/features/instance/applications/modals/RedeployApplicationModal.test.tsx b/src/features/instance/applications/modals/RedeployApplicationModal.test.tsx index 4d93c6d35..f071e1cf7 100644 --- a/src/features/instance/applications/modals/RedeployApplicationModal.test.tsx +++ b/src/features/instance/applications/modals/RedeployApplicationModal.test.tsx @@ -51,6 +51,7 @@ import { deployComponentStream, } from '@/integrations/api/instance/applications/deployComponentStream'; import { SSEOperationError } from '@/integrations/api/sse/errors'; +import { UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; import { setWatchedValue } from '@/lib/events/watcher'; import { mutationErrorHandler } from '@/react-query/queryClient'; import { MutationCache, QueryClient, QueryClientProvider } from '@tanstack/react-query'; @@ -190,6 +191,77 @@ describe('RedeployApplicationModal', () => { }); }); + describe('a redeploy that got no verdict', () => { + const unanswered = () => + new AxiosError( + 'timeout of 300000ms exceeded', + AxiosError.ECONNABORTED, + { + headers: new AxiosHeaders(), + method: 'post', + data: JSON.stringify({ operation: 'deploy_component' }), + } as InternalAxiosRequestConfig, + ); + const COMPONENTS = { queryKey: ['ins-1', 'get_components'], refetchType: 'active' }; + let invalidate: ReturnType; + beforeEach(() => { + invalidate = vi.spyOn(QueryClient.prototype, 'invalidateQueries'); + }); + + it("replaces a buffered redeploy's progress toast without calling it failed, and refreshes the list", async () => { + mocks.sse.enabled = false; + mocks.post.mockRejectedValue(unanswered()); + await renderOpenModal(); + + await submit(); + + await waitFor(() => expect(vi.mocked(toast.error)).toHaveBeenCalledOnce()); + expect(vi.mocked(toast.error)).toHaveBeenCalledWith("Couldn't confirm my-app was redeployed", { + id: 'loading-toast', + description: UNANSWERED_WRITE_MESSAGE, + }); + expect(invalidate).toHaveBeenCalledWith(COMPONENTS); + }); + + it('says so in the open progress panel, and refreshes the list', async () => { + vi.mocked(deployComponentStream).mockRejectedValue(unanswered()); + await renderOpenModal(); + + await submit(); + + expect((await screen.findByRole('alert')).textContent).toContain(UNANSWERED_WRITE_MESSAGE); + expect(vi.mocked(toast.error)).not.toHaveBeenCalled(); + expect(invalidate).toHaveBeenCalledWith(COMPONENTS); + }); + + it('refreshes nothing after a redeploy Harper refused', async () => { + vi.mocked(deployComponentStream).mockRejectedValue(new SSEOperationError(FAILURE, { phase: 'load' })); + await renderOpenModal(); + + await submit(); + + expect((await screen.findByRole('alert')).textContent).toContain(FAILURE); + expect(invalidate).not.toHaveBeenCalledWith(COMPONENTS); + }); + + it('toasts a streamed one that lands after the modal was closed without calling it failed', async () => { + const pending = deferred(); + vi.mocked(deployComponentStream).mockReturnValue(pending.promise); + await renderOpenModal(); + await submit(); + + await act(async () => { + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }); + }); + await act(async () => pending.reject(unanswered())); + + await waitFor(() => expect(vi.mocked(toast.error)).toHaveBeenCalledOnce()); + expect(vi.mocked(toast.error)).toHaveBeenCalledWith("Couldn't confirm my-app was redeployed", { + description: UNANSWERED_WRITE_MESSAGE, + }); + }); + }); + describe('a rollout that settles after the modal was closed', () => { function deployThenFollow(job: () => Promise) { vi.mocked(deployComponentStream).mockResolvedValue({ message: 'ok', deployment_id: 'd1', restartJobId: 'job-1' }); diff --git a/src/features/instance/applications/modals/RedeployApplicationModal.tsx b/src/features/instance/applications/modals/RedeployApplicationModal.tsx index ae8cd5e6a..c82b17dd6 100644 --- a/src/features/instance/applications/modals/RedeployApplicationModal.tsx +++ b/src/features/instance/applications/modals/RedeployApplicationModal.tsx @@ -24,6 +24,7 @@ import { DeployComponentResult } from '@/integrations/api/instance/applications/ import { deployFailureDetails } from '@/integrations/api/instance/applications/deployOutcome'; import { DeployRolloutError } from '@/integrations/api/instance/applications/restartJob'; import { SSEInconclusiveError } from '@/integrations/api/sse/errors'; +import { isUnansweredWrite } from '@/integrations/api/unansweredWrite'; import { attemptToRestoreFocus } from '@/lib/attemptToRestoreFocus'; import { setWatchedValue, useWatchedValue } from '@/lib/events/watcher'; import { describeError } from '@/react-query/queryClient'; @@ -32,7 +33,13 @@ import { RefreshCwIcon } from 'lucide-react'; import { useCallback, useEffect, useRef, useState } from 'react'; import { useForm } from 'react-hook-form'; import { toast } from 'sonner'; -import { deployNotes, reportDeployOutcome, reportDeployResult, reportStaged } from './reportDeployHealth'; +import { + deployFailureTitle, + deployNotes, + reportDeployOutcome, + reportDeployResult, + reportStaged, +} from './reportDeployHealth'; export function RedeployApplicationModal() { const { value: isModalOpen, trigger } = useWatchedValue('ShowRedeployApplicationModal', false); @@ -134,8 +141,9 @@ export function RedeployApplicationModal() { return; } const rolloutFailed = error instanceof DeployRolloutError; - if (rolloutFailed) { - // Live on the node that received it. + // Live on the node that received it, or, with no verdict, it may have deployed: either way the + // list the message says to check must be current. + if (rolloutFailed || isUnansweredWrite(error)) { invalidateComponents(); } const { message } = describeError(error); @@ -146,7 +154,7 @@ export function RedeployApplicationModal() { } const title = rolloutFailed ? `${project} did not reach every node` - : `Failed to ${stage ? 'stage' : 'redeploy'} ${project}`; + : deployFailureTitle(error, project, stage ? 'stage' : 'redeploy'); toast.error(title, { description: message, action: recordAction(deployFailureDetails(error).deploymentId) }); return; } @@ -204,7 +212,10 @@ export function RedeployApplicationModal() { invalidateComponents(); return; } - toast.error(`Failed to ${stage ? 'stage' : 'redeploy'} ${project}`, { + if (isUnansweredWrite(error)) { + invalidateComponents(); + } + toast.error(deployFailureTitle(error, project, stage ? 'stage' : 'redeploy'), { id: toastId, description: describeError(error).message, action, diff --git a/src/features/instance/applications/modals/RenameFileModal.tsx b/src/features/instance/applications/modals/RenameFileModal.tsx index 2170399df..f4b04ba9d 100644 --- a/src/features/instance/applications/modals/RenameFileModal.tsx +++ b/src/features/instance/applications/modals/RenameFileModal.tsx @@ -22,9 +22,11 @@ import { useEditorView } from '@/features/instance/applications/hooks/useEditorV import { useRenameFiles } from '@/features/instance/applications/hooks/useRenameFiles'; import { dropComponent } from '@/integrations/api/instance/applications/dropComponent'; import { setComponentFile } from '@/integrations/api/instance/applications/setComponentFile'; +import { isUnansweredWrite } from '@/integrations/api/unansweredWrite'; import { attemptToRestoreFocus } from '@/lib/attemptToRestoreFocus'; import { setWatchedValue, useWatchedValue } from '@/lib/events/watcher'; import { renameFileInPath } from '@/lib/string/paths/renameFileInPath'; +import { describeError } from '@/react-query/queryClient'; import { zodResolver } from '@hookform/resolvers/zod'; import { PencilIcon } from 'lucide-react'; import { useCallback, useEffect, useState } from 'react'; @@ -123,9 +125,11 @@ export function RenameFileModal() { await dropComponent({ ...instanceParams, project: openedEntry.project, file: relative(openedEntry.path) }); await reloadRootEntries(); } catch (error) { - toast.error('Rename Failed', { + const { title, description } = isUnansweredWrite(error) ? describeError(error) : { + title: 'Rename Failed', description: error instanceof Error ? error.message : 'Could not remove the original directory.', - }); + }; + toast.error(title, { description }); setIsPending(false); return; } diff --git a/src/features/instance/applications/modals/reportDeployHealth.test.ts b/src/features/instance/applications/modals/reportDeployHealth.test.ts index 66e51cb99..11bfacdd8 100644 --- a/src/features/instance/applications/modals/reportDeployHealth.test.ts +++ b/src/features/instance/applications/modals/reportDeployHealth.test.ts @@ -5,6 +5,8 @@ vi.mock('sonner', () => ({ toast })); import { DeployComponentResult } from '@/integrations/api/instance/applications/deployComponentStream'; import { DeployRolloutError } from '@/integrations/api/instance/applications/restartJob'; +import { UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; +import { AxiosError, type InternalAxiosRequestConfig } from 'axios'; import { deployNotes, reportDeployFailure, @@ -120,6 +122,18 @@ describe('reportDeployFailure', () => { reportDeployFailure('web', new Error('404: no such deployment'), 'activate'); expect(toast.error).toHaveBeenCalledWith('Failed to activate web', { description: '404: no such deployment' }); }); + + it('never calls an activation that got no verdict failed', () => { + const sent = { method: 'post', data: '{"operation":"deploy_component"}' } as InternalAxiosRequestConfig; + reportDeployFailure( + 'web', + new AxiosError('timeout of 300000ms exceeded', AxiosError.ECONNABORTED, sent), + 'activate', + ); + expect(toast.error).toHaveBeenCalledWith("Couldn't confirm web was activated", { + description: UNANSWERED_WRITE_MESSAGE, + }); + }); }); describe('deployNotes', () => { diff --git a/src/features/instance/applications/modals/reportDeployHealth.ts b/src/features/instance/applications/modals/reportDeployHealth.ts index 55a5c90d0..2c5de6f71 100644 --- a/src/features/instance/applications/modals/reportDeployHealth.ts +++ b/src/features/instance/applications/modals/reportDeployHealth.ts @@ -5,6 +5,7 @@ import { import { ComponentHealthResult } from '@/features/instance/applications/hooks/useComponentHealthCheck'; import { DeployComponentResult } from '@/integrations/api/instance/applications/deployComponentStream'; import { DeployRolloutError } from '@/integrations/api/instance/applications/restartJob'; +import { isUnansweredWrite, UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; import { toast } from 'sonner'; interface DeployNotes { @@ -135,20 +136,38 @@ export function reportDeployResult( toast.success(success.title, { ...options, description: withNotes(success.description, notes) }); } +type DeployVerb = 'import' | 'stage' | 'redeploy' | 'activate'; + +const DONE: Record = { + import: 'imported', + stage: 'staged', + redeploy: 'redeployed', + activate: 'activated', +}; + +/** Never "Failed" for a deploy that got no verdict: it may be live, and its message says to check first. */ +export function deployFailureTitle(error: unknown, project: string, verb: DeployVerb): string { + return isUnansweredWrite(error) ? `Couldn't confirm ${project} was ${DONE[verb]}` : `Failed to ${verb} ${project}`; +} + /** A deploy that failed: a rollout failure says the release is live where it was received, not that nothing landed. */ export function reportDeployFailure( project: string, error: unknown, - verb: string, + verb: DeployVerb, options: DeployToastOptions = {}, ): void { if (error instanceof DeployRolloutError) { toast.error(`${project} did not reach every node`, { ...options, description: error.message }); return; } - toast.error(`Failed to ${verb} ${project}`, { + toast.error(deployFailureTitle(error, project, verb), { ...options, - description: error instanceof Error ? error.message : undefined, + description: isUnansweredWrite(error) + ? UNANSWERED_WRITE_MESSAGE + : error instanceof Error + ? error.message + : undefined, }); } diff --git a/src/features/instance/config/deployments/components/ActivateStagedRelease.test.tsx b/src/features/instance/config/deployments/components/ActivateStagedRelease.test.tsx index 99576d143..40fc2a564 100644 --- a/src/features/instance/config/deployments/components/ActivateStagedRelease.test.tsx +++ b/src/features/instance/config/deployments/components/ActivateStagedRelease.test.tsx @@ -1,8 +1,10 @@ /** @vitest-environment jsdom */ import { Deployment } from '@/integrations/api/instance/deployments/types'; import { SSEInconclusiveError } from '@/integrations/api/sse/errors'; +import { UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { AxiosError, type InternalAxiosRequestConfig } from 'axios'; import { afterEach, describe, expect, it, vi } from 'vitest'; const state = vi.hoisted(() => ({ @@ -27,7 +29,7 @@ vi.mock('@/features/instance/applications/hooks/useDeploymentRecordAction', () = useDeploymentRecordAction: () => () => undefined, })); vi.mock('@/features/instance/applications/components/DeployProgress/DeployProgress', () => ({ - DeployProgress: () =>
progress
, + DeployProgress: ({ state }: { state: { error?: string } }) =>
{state.error ?? 'progress'}
, })); vi.mock('@/features/instance/applications/modals/reportDeployHealth', () => ({ reportDeployOutcome: state.reportDeployOutcome, @@ -134,6 +136,16 @@ describe('ActivateStagedRelease', () => { consoleError.mockRestore(); }); + it('says in its progress panel that an activation with no verdict may have gone through', async () => { + const sent = { method: 'post', data: '{"operation":"deploy_component"}' } as InternalAxiosRequestConfig; + const unanswered = new AxiosError('timeout of 300000ms exceeded', AxiosError.ECONNABORTED, sent); + state.mutateAsync.mockRejectedValue(unanswered); + renderButton(); + confirm(); + expect(await screen.findByText(UNANSWERED_WRITE_MESSAGE)).toBeTruthy(); + expect(state.reportDeployFailure).toHaveBeenCalledWith('web', unanswered, 'activate', { action: undefined }); + }); + it('offers nothing where staging is not known to exist', () => { state.canStage = false; renderButton(); diff --git a/src/features/instance/config/deployments/components/ActivateStagedRelease.tsx b/src/features/instance/config/deployments/components/ActivateStagedRelease.tsx index d68b10344..b0c90c735 100644 --- a/src/features/instance/config/deployments/components/ActivateStagedRelease.tsx +++ b/src/features/instance/config/deployments/components/ActivateStagedRelease.tsx @@ -25,6 +25,7 @@ import { deployFailureDetails } from '@/integrations/api/instance/applications/d import { DeployRolloutError } from '@/integrations/api/instance/applications/restartJob'; import { Deployment } from '@/integrations/api/instance/deployments/types'; import { SSEInconclusiveError } from '@/integrations/api/sse/errors'; +import { isUnansweredWrite, UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite'; import { invalidateEntityQueries } from '@/react-query/invalidateEntityQueries'; import { useIsMutating, useQueryClient } from '@tanstack/react-query'; import { RocketIcon } from 'lucide-react'; @@ -84,7 +85,13 @@ export function ActivateStagedRelease({ deployment }: { deployment: Deployment } } stream.markSettled( 'error', - error instanceof DeployRolloutError ? undefined : error instanceof Error ? error.message : String(error), + error instanceof DeployRolloutError + ? undefined + : isUnansweredWrite(error) + ? UNANSWERED_WRITE_MESSAGE + : error instanceof Error + ? error.message + : String(error), { failure: error }, ); reportDeployFailure(project, error, 'activate', { diff --git a/src/features/instance/config/secrets/SecretGrantsEditor.tsx b/src/features/instance/config/secrets/SecretGrantsEditor.tsx index 9525adb40..fd233d066 100644 --- a/src/features/instance/config/secrets/SecretGrantsEditor.tsx +++ b/src/features/instance/config/secrets/SecretGrantsEditor.tsx @@ -9,7 +9,7 @@ import { Badge } from '@/components/ui/badge'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { ComponentGrantCombobox } from '@/features/instance/secrets/ComponentGrantCombobox'; import { useGrantSecret, useRevokeSecret } from '@/integrations/api/instance/secrets/secrets'; -import { describeError } from '@/react-query/queryClient'; +import { describeError, shouldToastOwnFailure } from '@/react-query/queryClient'; import { XIcon } from 'lucide-react'; import { useCallback, useState } from 'react'; import { toast } from 'sonner'; @@ -41,7 +41,9 @@ export function SecretGrantsEditor({ setGrants(response.grants); onChanged?.(); } catch (error) { - toast.error(describeError(error).message); + if (shouldToastOwnFailure(error)) { + toast.error(describeError(error).message); + } // Rethrow so the combobox keeps the typed text for a retry instead of clearing it. throw error; } @@ -53,7 +55,9 @@ export function SecretGrantsEditor({ setGrants(response.grants); onChanged?.(); } catch (error) { - toast.error(describeError(error).message); + if (shouldToastOwnFailure(error)) { + toast.error(describeError(error).message); + } } }, [revokeSecret, instanceParams, name, onChanged]); diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index 3548f9b3d..6d1444560 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -70,6 +70,7 @@ import { keyBy } from '@/lib/keyBy'; import { onClickStopPropagation } from '@/lib/onClickStopPropagation'; import { pluralize } from '@/lib/pluralize'; import { Row } from '@/lib/table'; +import { shouldToastOwnFailure } from '@/react-query/queryClient'; import { zodResolver } from '@hookform/resolvers/zod'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { Link, useParams, useSearch } from '@tanstack/react-router'; @@ -645,7 +646,9 @@ export function DatabaseTableView({ refreshTable(); }, onError: (error) => { - toast.error(error instanceof Error ? error.message : 'Failed to cleanup orphan blobs'); + if (shouldToastOwnFailure(error)) { + toast.error(error instanceof Error ? error.message : 'Failed to cleanup orphan blobs'); + } }, }); }, [cleanupOrphanBlobs, databaseName, instanceParams, refreshTable]); diff --git a/src/features/instance/databases/modals/recordEditorJson.test.ts b/src/features/instance/databases/modals/recordEditorJson.test.ts index ff73cc132..900cb01c7 100644 --- a/src/features/instance/databases/modals/recordEditorJson.test.ts +++ b/src/features/instance/databases/modals/recordEditorJson.test.ts @@ -34,6 +34,13 @@ describe('tryParseRecordJson', () => { expect(parsed.ok === false && parsed.error.message).not.toMatch(/position|line \d+ column/i); }); + it('drops it from trailing-content errors too, which V8 words "after JSON at position"', () => { + const parsed = tryParseRecordJson('{"a": 1}}'); + + expect(parsed.ok === false && parsed.error.message).toBe('Unexpected non-whitespace character after JSON'); + expect(parsed.ok === false && parsed.error.location).toEqual({ lineNumber: 1, column: 9 }); + }); + // These go to the ops API as `records`, so a primitive parses cleanly and then fails on the // wire in the server's wording. Say it here, where the user can act on it. it('rejects JSON that is well-formed but could never be a record', () => { diff --git a/src/features/instance/databases/modals/recordEditorJson.ts b/src/features/instance/databases/modals/recordEditorJson.ts index a44659450..657cc91bb 100644 --- a/src/features/instance/databases/modals/recordEditorJson.ts +++ b/src/features/instance/databases/modals/recordEditorJson.ts @@ -32,15 +32,24 @@ export function tryParseRecordJson(content: string): ParsedRecordJson { // record rather than an editor the user emptied. return { ok: false, error: { message: 'The editor is empty, so there is nothing to save.' } }; } - let value: unknown; + const parsed = tryParseJson(content); + if (!parsed.ok) { + return parsed; + } + const shapeError = recordShapeError(parsed.value); + return shapeError + ? { ok: false, error: { message: shapeError } } + : { ok: true, value: parsed.value as RecordJsonValue }; +} + +/** Parse `content` as any JSON value; on failure, the parser's reason and (when the engine says) where. */ +export function tryParseJson(content: string): { ok: true; value: unknown } | { ok: false; error: RecordJsonError } { try { - value = JSON.parse(content); + return { ok: true, value: JSON.parse(content) }; } catch (error) { const message = error instanceof Error ? error.message : String(error); return { ok: false, error: { message: withoutEngineLocation(message), location: locate(message, content) } }; } - const shapeError = recordShapeError(value); - return shapeError ? { ok: false, error: { message: shapeError } } : { ok: true, value: value as RecordJsonValue }; } /** One-line rendering of a {@link RecordJsonError}, for a toast description. */ @@ -99,7 +108,7 @@ function isRecord(value: unknown): value is Record { function withoutEngineLocation(message: string): string { return message .replace(/^JSON\.parse:\s*/, '') - .replace(/\s*in JSON at position \d+.*$/i, '') + .replace(/\s*(?:in JSON )?at position \d+.*$/i, '') .replace(/\s*at line \d+ column \d+ of the JSON data\.?$/i, '') .trim(); } diff --git a/src/features/instance/operations/index.test.tsx b/src/features/instance/operations/index.test.tsx new file mode 100644 index 000000000..bf279f143 --- /dev/null +++ b/src/features/instance/operations/index.test.tsx @@ -0,0 +1,387 @@ +/** @vitest-environment jsdom */ +import { OperationsIndex } from '@/features/instance/operations/index'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import axios, { AxiosError, CanceledError, type InternalAxiosRequestConfig } from 'axios'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { state, invalidateEntityQueries, markRestarting, releaseRestart, restartSettles } = vi.hoisted(() => { + const releaseRestart = vi.fn(); + const restartSettles = { resolve: () => {}, promise: Promise.resolve() } as { + resolve: () => void; + promise: Promise; + }; + return { + state: { + canManage: true, + user: { username: 'HDB_ADMIN' } as { username: string } | null, + entityId: 'ins-1', + }, + invalidateEntityQueries: vi.fn(), + releaseRestart, + markRestarting: vi.fn(() => releaseRestart), + restartSettles, + }; +}); + +vi.mock('@/lib/monaco/MonacoEditor', () => ({ + Editor: ({ value, onChange }: { value?: string; onChange?: (value: string | undefined) => void }) => ( +