Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/features/instance/applications/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,15 @@ tell it from a gateway's. So [`onDeployComponentSubmit`](../../../integrations/a
turns those into `SSEInconclusiveError`, which every deploy caller already answers by checking rather than resending.
`streamOperation` stays neutral: it puts the `status` on `SSEUnsupportedError` and leaves the decision to the caller,
because a read (`get_deployment`) may fall back on anything.

## Who reports a failed deploy

Every caller of [`useDeployComponentMutation`](../../../integrations/api/instance/applications/deployComponent.ts)
shows its own failure. The mutation opts out of the global error toast and reports the failure to RUM itself, with a
mutation-level `console.error` that still runs after the caller unmounts. A new caller has to render or toast the
rejection, or the person sees nothing.

Callers await `mutateAsync` rather than reporting from `mutate()` callbacks, which React Query drops once the caller
unmounts. Following a rollout keeps a deploy pending for up to 30 minutes, so its outcome often lands after the form
or modal that started it is gone. A failure that lands while its progress panel or import form is still on screen is
shown there only; one that lands later goes to a toast, with the deployment record when the failure named one.
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { deploymentRecordPath } from '@/features/instance/applications/hooks/useDeploymentRecordAction';
import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage';
import { Link, useParams } from '@tanstack/react-router';
import { CircleIcon } from 'lucide-react';

// The instance's own SSH keys fix this, so the failure links to them.
const SSH_ACCESS_FAILURE = /Failed to deploy private repository\b|SSH access failed/i;

export function DeployFailure({
title,
message,
deploymentId,
onNavigateAway,
}: {
title?: string;
message: string;
/** Only where the host shows no record link of its own. */
deploymentId?: string;
/** Called when a guidance link navigates away — lets a hosting modal close itself first. */
onNavigateAway?: () => void;
}) {
const params: { clusterId?: string; instanceId?: string; organizationId?: string } = useParams({ strict: false });

return (
<div role="alert" className="flex flex-col gap-2 text-sm text-destructive">
<div className="flex items-start gap-2">
<CircleIcon className="mt-0.5 size-4 shrink-0" />
<div className="flex flex-col gap-1 break-words">
{title && <span className="font-medium">{title}</span>}
<span>{message}</span>
</div>
</div>
{SSH_ACCESS_FAILURE.test(message) && (
<Link
to={buildAbsoluteLinkToPage(params, 'config/ssh-keys')}
onClick={onNavigateAway}
className="self-start pl-6 underline"
>
Manage SSH keys
</Link>
)}
{deploymentId && (
<Link
to={deploymentRecordPath(params, deploymentId)}
onClick={onNavigateAway}
className="self-start pl-6 underline"
>
View this deployment's record
</Link>
)}
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,10 @@ import { CertificationDecision } from '@/integrations/api/instance/applications/
import { RolloutNode, RolloutSnapshot } from '@/integrations/api/instance/applications/restartJob';
import { cn } from '@/lib/cn';
import { errorText } from '@/lib/errorText';
import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage';
import { Link, useParams } from '@tanstack/react-router';
import {
CheckIcon,
CircleDashedIcon,
CircleIcon,
Loader2Icon,
ShieldCheckIcon,
TriangleAlertIcon,
XIcon,
} from 'lucide-react';
import { CheckIcon, CircleDashedIcon, Loader2Icon, ShieldCheckIcon, TriangleAlertIcon, XIcon } from 'lucide-react';
import { useEffect, useRef } from 'react';
import { DeployFailure } from './DeployFailure';
import {
describeCertification,
describeDecision,
Expand All @@ -36,16 +28,6 @@ const PHASE_LABELS: Record<string, string> = {
staged: 'Staged',
};

/**
* A deploy that failed because the *instance* couldn't reach a private repository over SSH is
* fixed in Config > SSH Keys (install a key, grant it access to the repo, add the git host to
* `known_hosts`), so link the user there instead of leaving them to find it. Harper's message
* already spells out the remediation, and this UI is now the only place the failure surfaces:
* it's an instance configuration state, not a Studio bug, so `shouldKeepEvent` drops the RUM
* event (which also kept the customer's private repo URL out of Error Tracking).
*/
const SSH_ACCESS_FAILURE = /Failed to deploy private repository\b|SSH access failed/i;

function PhaseIcon({ status, isCurrent }: { status?: PhaseStatus; isCurrent: boolean }) {
if (status === 'done') {
return <CheckIcon className="size-4 text-success" />;
Expand Down Expand Up @@ -164,21 +146,7 @@ export function DeployProgress({
{phasesFailed && state.decision && <DecisionBlock decision={state.decision} />}

{phasesFailed && state.error && !state.decision && (
<div className="flex flex-col gap-2 text-sm text-destructive">
<div className="flex items-start gap-2">
<CircleIcon className="mt-0.5 size-4 shrink-0" />
<span>{state.error}</span>
</div>
{SSH_ACCESS_FAILURE.test(state.error) && (
<Link
to={buildAbsoluteLinkToPage(params, 'config/ssh-keys')}
onClick={onNavigateAway}
className="self-start pl-6 underline"
>
Manage SSH keys
</Link>
)}
</div>
<DeployFailure message={state.error} onNavigateAway={onNavigateAway} />
)}

{state.lifecycle === 'inconclusive' && !state.rollout && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,27 @@ import { Input } from '@/components/ui/input';
import { Separator } from '@/components/ui/separator';
import { Switch } from '@/components/ui/switch';
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
import { DeployFailure } from '@/features/instance/applications/components/DeployProgress/DeployFailure';
import { useStagedDeploymentsAvailable } from '@/features/instance/applications/hooks/useSupportsDeploymentSSE';
import { GitMergeIcon, LinkIcon, PackageIcon, RocketIcon } from 'lucide-react';
import { useCallback } from 'react';
import { Control, FormState, UseFormSetValue, UseFormWatch } from 'react-hook-form';
import { z } from 'zod';
import { ImportAuthorization } from './ImportAuthorization';
import { NewApplicationSchema } from './schema';
import { ImportFailure } from './useImportApplication';

export function ImportInstructions({
control,
formState,
importFailure,
isImportingApplication,
setValue,
watch,
}: {
control: Control<z.infer<typeof NewApplicationSchema>>;
formState: FormState<z.infer<typeof NewApplicationSchema>>;
importFailure?: ImportFailure;
isImportingApplication: boolean;
setValue: UseFormSetValue<z.infer<typeof NewApplicationSchema>>;
watch: UseFormWatch<z.infer<typeof NewApplicationSchema>>;
Expand Down Expand Up @@ -180,6 +184,14 @@ export function ImportInstructions({

<Separator className="bg-border" />

{importFailure && (
<DeployFailure
title={importFailure.title}
message={importFailure.message}
deploymentId={importFailure.deploymentId}
/>
)}

<Button
className="w-full"
disabled={!formState.isValid || isImportingApplication}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@
* @vitest-environment jsdom
*/
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { ReactNode } from 'react';
import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest';

const importState = vi.hoisted(() => ({
importFailure: undefined as { title: string; message: string; deploymentId?: string } | undefined,
}));

vi.mock('@tanstack/react-router', async (importOriginal) => ({
...(await importOriginal<typeof import('@tanstack/react-router')>()),
useRouteContext: () => ({ organization: { name: 'Acme' } }),
useParams: () => ({ organizationId: 'org-1', instanceId: 'ins-1' }),
Link: ({ children, to }: { children?: ReactNode; to?: string }) => <a href={to}>{children}</a>,
}));
vi.mock('@/features/instance/applications/hooks/useEditorView', () => ({
useEditorView: () => ({ rootEntries: [{ name: 'taken-app', path: 'taken-app', project: 'taken-app', entries: [] }] }),
Expand All @@ -15,7 +22,11 @@ vi.mock('./useCreateFromTemplate', () => ({
useCreateFromTemplate: () => ({ isCreatingFromTemplate: false, createFromTemplate: vi.fn() }),
}));
vi.mock('./useImportApplication', () => ({
useImportApplication: () => ({ isImportingApplication: false, importApplication: vi.fn() }),
useImportApplication: () => ({
isImportingApplication: false,
importApplication: vi.fn(),
importFailure: importState.importFailure,
}),
}));
vi.mock('./useCheckCLI', () => ({ useCheckCLI: () => ({ checkCLI: vi.fn() }) }));
// These read the instance's SSH keys and auth, which aren't what these tests exercise.
Expand All @@ -35,7 +46,10 @@ beforeAll(() => {
Element.prototype.scrollIntoView ??= () => undefined;
});

afterEach(cleanup);
afterEach(() => {
cleanup();
importState.importFailure = undefined;
});

function button(name: RegExp) {
return screen.getByRole('button', { name }) as HTMLButtonElement;
Expand Down Expand Up @@ -113,4 +127,47 @@ describe('NewApplication', () => {
expect(await screen.findByText(message)).toBeTruthy();
expect((screen.getByRole('button', { name: /Create from Template/ }) as HTMLButtonElement).disabled).toBe(true);
});

it("keeps a failed import's reason beside the Import button", async () => {
importState.importFailure = {
title: 'Failed to import acme',
message: 'This repository has no package.json in its root folder.',
};
render(<NewApplication />);

await switchTo(/Import/);

const alert = await screen.findByRole('alert');
expect(alert.textContent).toContain('Failed to import acme');
expect(alert.textContent).toContain('This repository has no package.json in its root folder.');
expect(screen.queryByRole('link', { name: 'Manage SSH keys' })).toBeNull();
});

it('links a failed private-repository import to SSH keys', async () => {
importState.importFailure = {
title: 'Failed to import acme',
message: 'Failed to deploy private repository ssh://github.com/acme/widgets.git: SSH access failed.',
};
render(<NewApplication />);

await switchTo(/Import/);

expect((await screen.findByRole('link', { name: 'Manage SSH keys' })).getAttribute('href'))
.toBe('/org-1/instance/ins-1/config/ssh-keys');
});

it('links a failed import to the deployment record it left', async () => {
importState.importFailure = {
title: 'Failed to stage acme',
message: 'Not every node staged it',
deploymentId: 'd1',
};
render(<NewApplication />);

await switchTo(/Import/);

expect((await screen.findByRole('alert')).textContent).toContain('Failed to stage acme');
expect(screen.getByRole('link', { name: "View this deployment's record" }).getAttribute('href'))
.toBe('/org-1/instance/ins-1/config/deployments/d1');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export function NewApplication() {
const [isReloading, setIsReloading] = useState(false);

const { isCreatingFromTemplate, createFromTemplate } = useCreateFromTemplate(setIsReloading);
const { isImportingApplication, importApplication } = useImportApplication(setIsReloading);
const { isImportingApplication, importApplication, importFailure } = useImportApplication(setIsReloading);
const { checkCLI } = useCheckCLI(setIsReloading);

const submitForm = useCallback((formData: z.infer<typeof NewApplicationSchema>) => {
Expand Down Expand Up @@ -162,6 +162,7 @@ export function NewApplication() {
<ImportInstructions
control={control}
formState={formState}
importFailure={importFailure}
isImportingApplication={isImportingApplication}
setValue={setValue}
watch={watch}
Expand Down
Loading
Loading