Skip to content

Commit 4714670

Browse files
manaswinidasmanaswinidas
authored
fix(frontend): surface usePipelineVersionTemplate errors in run detail routers (#13291)
RunDetailsRouter and RecurringRunDetailsRouter both call usePipelineVersionTemplate but only destructure isLoading and data, silently dropping query failures. When the pipeline version fetch fails the user sees no feedback and falls through to a legacy or broken view. Destructure isError/error from the hook and wire them into updateBanner via useEffect, matching the error-surfacing pattern established in PR #13217 for other useQuery consumers. Tests added for both routers verifying the error banner fires when getPipelineVersion rejects. Made-with: Cursor Made-with: Cursor Signed-off-by: manaswinidas <dasmanaswini10@gmail.com>
1 parent d142fc4 commit 4714670

4 files changed

Lines changed: 173 additions & 33 deletions

File tree

‎frontend/src/pages/RecurringRunDetailsRouter.test.tsx‎

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,8 @@ describe('RecurringRunDetailsRouter', () => {
127127
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
128128
},
129129
};
130-
const pipelineVersion: V2beta1PipelineVersion = {
131-
pipeline_id: TEST_PIPELINE_ID,
132-
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
133-
pipeline_spec: {
134-
apiVersion: 'argoproj.io/v1alpha1',
135-
kind: 'Workflow',
136-
metadata: { name: 'from-version' },
137-
spec: { arguments: { parameters: [{ name: 'output' }] } },
138-
},
139-
};
140130

141131
getRecurringRunSpy.mockResolvedValue(recurringRun);
142-
getPipelineVersionSpy.mockResolvedValue(pipelineVersion);
143132

144133
render(
145134
<CommonTestWrapper>
@@ -148,12 +137,9 @@ describe('RecurringRunDetailsRouter', () => {
148137
);
149138

150139
await waitFor(() => {
151-
expect(getPipelineVersionSpy).toHaveBeenCalledWith(
152-
TEST_PIPELINE_ID,
153-
TEST_PIPELINE_VERSION_ID,
154-
);
155140
expect(screen.getByTestId('recurring-run-details-v2')).toBeInTheDocument();
156141
});
142+
expect(getPipelineVersionSpy).not.toHaveBeenCalled();
157143
});
158144

159145
it('renders RecurringRunDetailsV2FC when FUNCTIONAL_COMPONENT flag is enabled', async () => {
@@ -330,6 +316,62 @@ describe('RecurringRunDetailsRouter', () => {
330316
});
331317
});
332318

319+
it('shows error banner when pipeline version template fetch fails', async () => {
320+
const recurringRun: V2beta1RecurringRun = {
321+
recurring_run_id: TEST_RECURRING_RUN_ID,
322+
pipeline_version_reference: {
323+
pipeline_id: TEST_PIPELINE_ID,
324+
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
325+
},
326+
};
327+
getRecurringRunSpy.mockResolvedValue(recurringRun);
328+
getPipelineVersionSpy.mockRejectedValue(new Error('Version not found'));
329+
330+
const props = generateProps();
331+
render(
332+
<CommonTestWrapper>
333+
<RecurringRunDetailsRouter {...props} />
334+
</CommonTestWrapper>,
335+
);
336+
337+
await waitFor(() => {
338+
expect(props.updateBanner).toHaveBeenCalledWith(
339+
expect.objectContaining({
340+
message: expect.stringContaining('failed to retrieve pipeline version template'),
341+
mode: 'error',
342+
}),
343+
);
344+
});
345+
});
346+
347+
it('does not show error banner when inline pipeline_spec is present and getPipelineVersion rejects', async () => {
348+
vi.spyOn(features, 'isFeatureEnabled').mockImplementation(
349+
(featureKey) => featureKey === features.FeatureKey.V2_ALPHA,
350+
);
351+
const recurringRun: V2beta1RecurringRun = {
352+
recurring_run_id: TEST_RECURRING_RUN_ID,
353+
pipeline_spec: v2PipelineSpec,
354+
pipeline_version_reference: {
355+
pipeline_id: TEST_PIPELINE_ID,
356+
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
357+
},
358+
};
359+
getRecurringRunSpy.mockResolvedValue(recurringRun);
360+
getPipelineVersionSpy.mockRejectedValue(new Error('Version not found'));
361+
362+
const props = generateProps();
363+
render(
364+
<CommonTestWrapper>
365+
<RecurringRunDetailsRouter {...props} />
366+
</CommonTestWrapper>,
367+
);
368+
369+
await waitFor(() => {
370+
expect(screen.getByTestId('recurring-run-details-v2')).toBeInTheDocument();
371+
});
372+
expect(props.updateBanner).not.toHaveBeenCalled();
373+
});
374+
333375
it('shows loading message while recurring run data is being fetched', () => {
334376
getRecurringRunSpy.mockReturnValue(new Promise(() => {}));
335377

‎frontend/src/pages/RecurringRunDetailsRouter.tsx‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,15 @@ export default function RecurringRunDetailsRouter(props: PageProps) {
6262
const pipelineId = v2RecurringRun?.pipeline_version_reference?.pipeline_id;
6363
const pipelineVersionId = v2RecurringRun?.pipeline_version_reference?.pipeline_version_id;
6464

65-
const { isLoading: templateStrIsLoading, data: templateStrFromPipelineVersion } =
66-
usePipelineVersionTemplate(pipelineId, pipelineVersionId);
65+
const {
66+
isLoading: templateStrIsLoading,
67+
isError: templateStrIsError,
68+
error: templateStrError,
69+
data: templateStrFromPipelineVersion,
70+
} = usePipelineVersionTemplate(
71+
pipelineManifest ? undefined : pipelineId,
72+
pipelineManifest ? undefined : pipelineVersionId,
73+
);
6774

6875
const templateString = pipelineManifest ?? templateStrFromPipelineVersion;
6976

@@ -86,6 +93,26 @@ export default function RecurringRunDetailsRouter(props: PageProps) {
8693
return undefined;
8794
}, [getRecurringRunError, recurringRunError, recurringRunId, updateBanner]);
8895

96+
useEffect(() => {
97+
if (templateStrIsError && templateStrError && !getRecurringRunError) {
98+
let cancelled = false;
99+
errorToMessage(templateStrError).then((msg) => {
100+
if (!cancelled) {
101+
updateBanner({
102+
message:
103+
'Error: failed to retrieve pipeline version template. Click Details for more information.',
104+
mode: 'error',
105+
additionalInfo: msg,
106+
});
107+
}
108+
});
109+
return () => {
110+
cancelled = true;
111+
};
112+
}
113+
return undefined;
114+
}, [templateStrIsError, templateStrError, getRecurringRunError, updateBanner]);
115+
89116
if (getRecurringRunSuccess && v2RecurringRun && templateString) {
90117
const isV2Pipeline = WorkflowUtils.isPipelineSpec(templateString);
91118
if (isV2Pipeline) {

‎frontend/src/pages/RunDetailsRouter.test.tsx‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -264,28 +264,16 @@ describe('RunDetailsRouter', () => {
264264
},
265265
};
266266
getRunSpy.mockResolvedValue(v2Run);
267-
getPipelineVersionSpy.mockResolvedValue({
268-
pipeline_id: TEST_PIPELINE_ID,
269-
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
270-
pipeline_spec: {
271-
apiVersion: 'argoproj.io/v1alpha1',
272-
kind: 'Workflow',
273-
metadata: { name: 'from-version' },
274-
spec: { arguments: { parameters: [{ name: 'output' }] } },
275-
},
276-
} as V2beta1PipelineVersion);
267+
277268
render(
278269
<CommonTestWrapper>
279270
<RunDetailsRouter {...generateProps()} />
280271
</CommonTestWrapper>,
281272
);
282273
await waitFor(() => {
283-
expect(getPipelineVersionSpy).toHaveBeenCalledWith(
284-
TEST_PIPELINE_ID,
285-
TEST_PIPELINE_VERSION_ID,
286-
);
287274
expect(screen.getByTestId('run-details-v2')).toBeInTheDocument();
288275
});
276+
expect(getPipelineVersionSpy).not.toHaveBeenCalled();
289277
});
290278

291279
it('renders EnhancedRunDetails when getRun fails', async () => {
@@ -308,6 +296,59 @@ describe('RunDetailsRouter', () => {
308296
});
309297
});
310298

299+
it('shows error banner when pipeline version template fetch fails', async () => {
300+
const runWithVersionRef: V2beta1Run = {
301+
run_id: TEST_RUN_ID,
302+
pipeline_version_reference: {
303+
pipeline_id: TEST_PIPELINE_ID,
304+
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
305+
},
306+
};
307+
getRunSpy.mockResolvedValue(runWithVersionRef);
308+
getPipelineVersionSpy.mockRejectedValue(new Error('Version not found'));
309+
310+
const props = generateProps();
311+
render(
312+
<CommonTestWrapper>
313+
<RunDetailsRouter {...props} />
314+
</CommonTestWrapper>,
315+
);
316+
317+
await waitFor(() => {
318+
expect(props.updateBanner).toHaveBeenCalledWith(
319+
expect.objectContaining({
320+
message: expect.stringContaining('failed to retrieve pipeline version template'),
321+
mode: 'error',
322+
}),
323+
);
324+
});
325+
});
326+
327+
it('does not show error banner when inline pipeline_spec is present and getPipelineVersion rejects', async () => {
328+
const v2Run: V2beta1Run = {
329+
run_id: TEST_RUN_ID,
330+
pipeline_spec: v2PipelineSpec,
331+
pipeline_version_reference: {
332+
pipeline_id: TEST_PIPELINE_ID,
333+
pipeline_version_id: TEST_PIPELINE_VERSION_ID,
334+
},
335+
};
336+
getRunSpy.mockResolvedValue(v2Run);
337+
getPipelineVersionSpy.mockRejectedValue(new Error('Version not found'));
338+
339+
const props = generateProps();
340+
render(
341+
<CommonTestWrapper>
342+
<RunDetailsRouter {...props} />
343+
</CommonTestWrapper>,
344+
);
345+
346+
await waitFor(() => {
347+
expect(screen.getByTestId('run-details-v2')).toBeInTheDocument();
348+
});
349+
expect(props.updateBanner).not.toHaveBeenCalled();
350+
});
351+
311352
it('fetches template from pipeline version when run has no inline spec', async () => {
312353
const runWithVersionRef: V2beta1Run = {
313354
run_id: TEST_RUN_ID,

‎frontend/src/pages/RunDetailsRouter.tsx‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
* limitations under the License.
1515
*/
1616

17+
import { useEffect } from 'react';
1718
import * as JsYaml from 'js-yaml';
1819
import { useQuery } from '@tanstack/react-query';
1920
import { V2beta1Run } from 'src/apisv2beta1/run';
2021
import { RouteParams } from 'src/components/Router';
2122
import { Apis } from 'src/lib/Apis';
23+
import { errorToMessage } from 'src/lib/Utils';
2224
import * as WorkflowUtils from 'src/lib/v2/WorkflowUtils';
2325
import { RouteComponentProps } from 'react-router-dom';
2426
import EnhancedRunDetails, { RunDetailsProps } from 'src/pages/RunDetails';
@@ -30,6 +32,7 @@ import { queryKeys } from 'src/hooks/queryKeys';
3032
export default function RunDetailsRouter(
3133
props: RunDetailsProps & RouteComponentProps<RunDetailsV2Params>,
3234
) {
35+
const { updateBanner } = props;
3336
const runId = props.match.params[RouteParams.runId];
3437
let pipelineManifest: string | undefined;
3538

@@ -50,8 +53,35 @@ export default function RunDetailsRouter(
5053
const pipelineId = v2Run?.pipeline_version_reference?.pipeline_id;
5154
const pipelineVersionId = v2Run?.pipeline_version_reference?.pipeline_version_id;
5255

53-
const { isLoading: templateStrIsLoading, data: templateStrFromPipelineVersion } =
54-
usePipelineVersionTemplate(pipelineId, pipelineVersionId);
56+
const {
57+
isLoading: templateStrIsLoading,
58+
isError: templateStrIsError,
59+
error: templateStrError,
60+
data: templateStrFromPipelineVersion,
61+
} = usePipelineVersionTemplate(
62+
pipelineManifest ? undefined : pipelineId,
63+
pipelineManifest ? undefined : pipelineVersionId,
64+
);
65+
66+
useEffect(() => {
67+
if (templateStrIsError && templateStrError) {
68+
let cancelled = false;
69+
errorToMessage(templateStrError).then((msg) => {
70+
if (!cancelled) {
71+
updateBanner({
72+
message:
73+
'Error: failed to retrieve pipeline version template. Click Details for more information.',
74+
mode: 'error',
75+
additionalInfo: msg,
76+
});
77+
}
78+
});
79+
return () => {
80+
cancelled = true;
81+
};
82+
}
83+
return undefined;
84+
}, [templateStrIsError, templateStrError, updateBanner]);
5585

5686
const templateString = pipelineManifest ?? templateStrFromPipelineVersion;
5787

0 commit comments

Comments
 (0)