Skip to content

Commit 529c90e

Browse files
adhityamamallanadhityamamallan
authored
Add error page config for Workflow Diagnostics (#970)
- Handle Workflow Diagnostics errors from workflow page tabs config - Create new helper getWorkflowDiagnosticsErrorConfig that tries to match the errors for specific Diagnostics errors - Check workflow close status in workflow-diagnostics.tsx before running diagnostics, to ensure that we don't run it for a running workflow
1 parent 8a3af8d commit 529c90e

File tree

7 files changed

+188
-50
lines changed

7 files changed

+188
-50
lines changed

‎src/views/workflow-diagnostics/__tests__/workflow-diagnostics.test.tsx

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,12 @@ import { HttpResponse } from 'msw';
55
import { render, screen } from '@/test-utils/rtl';
66

77
import ErrorBoundary from '@/components/error-boundary/error-boundary';
8+
import { type DescribeWorkflowResponse } from '@/route-handlers/describe-workflow/describe-workflow.types';
89
import { mockWorkflowDiagnosticsResult } from '@/route-handlers/diagnose-workflow/__fixtures__/mock-workflow-diagnostics-result';
10+
import { mockDescribeWorkflowResponse } from '@/views/workflow-page/__fixtures__/describe-workflow-response';
911

1012
import WorkflowDiagnostics from '../workflow-diagnostics';
1113

12-
jest.mock('@/components/error-panel/error-panel', () =>
13-
jest.fn(({ message }) => <div data-testid="error-panel">{message}</div>)
14-
);
15-
16-
jest.mock('@/components/panel-section/panel-section', () =>
17-
jest.fn(({ children }) => <div data-testid="panel-section">{children}</div>)
18-
);
19-
2014
jest.mock(
2115
'@/components/section-loading-indicator/section-loading-indicator',
2216
() => jest.fn(() => <div data-testid="loading-indicator">Loading...</div>)
@@ -46,10 +40,6 @@ jest.mock(
4640
))
4741
);
4842

49-
jest.mock('../config/workflow-diagnostics-disabled-error-panel.config', () => ({
50-
message: 'Workflow Diagnostics is currently disabled',
51-
}));
52-
5343
describe(WorkflowDiagnostics.name, () => {
5444
beforeEach(() => {
5545
jest.clearAllMocks();
@@ -117,14 +107,13 @@ describe(WorkflowDiagnostics.name, () => {
117107
).toBeInTheDocument();
118108
});
119109

120-
it('should render error panel when diagnostics is disabled', async () => {
110+
it('should throw error when diagnostics is disabled', async () => {
121111
await setup({ isDiagnosticsEnabled: false });
122112

123-
await screen.findByTestId('error-panel');
124-
125-
expect(screen.getByTestId('panel-section')).toBeInTheDocument();
126113
expect(
127-
screen.getByText('Workflow Diagnostics is currently disabled')
114+
await screen.findByText(
115+
'Error: Workflow diagnostics is currently disabled'
116+
)
128117
).toBeInTheDocument();
129118
});
130119

@@ -135,16 +124,31 @@ describe(WorkflowDiagnostics.name, () => {
135124
await screen.findByText('Error: Failed to fetch config')
136125
).toBeInTheDocument();
137126
});
127+
128+
it('should throw error when workflow is not closed', async () => {
129+
await setup({
130+
isDiagnosticsEnabled: true,
131+
isWorkflowClosed: false,
132+
});
133+
134+
expect(
135+
await screen.findByText(
136+
'Error: Cannot load diagnostics for a running workflow'
137+
)
138+
).toBeInTheDocument();
139+
});
138140
});
139141

140142
async function setup({
141143
isDiagnosticsEnabled,
142144
error,
143145
diagnosticsResponse,
146+
isWorkflowClosed = true,
144147
}: {
145148
isDiagnosticsEnabled: boolean;
146149
error?: boolean;
147150
diagnosticsResponse?: any;
151+
isWorkflowClosed?: boolean;
148152
}) {
149153
render(
150154
<ErrorBoundary
@@ -179,6 +183,25 @@ async function setup({
179183
}
180184
},
181185
},
186+
{
187+
path: '/api/domains/:domain/:cluster/workflows/:workflowId/:runId',
188+
httpMethod: 'GET',
189+
mockOnce: false,
190+
httpResolver: async () => {
191+
return HttpResponse.json({
192+
...mockDescribeWorkflowResponse,
193+
workflowExecutionInfo: {
194+
...mockDescribeWorkflowResponse.workflowExecutionInfo,
195+
...(isWorkflowClosed
196+
? { closeStatus: 'WORKFLOW_EXECUTION_CLOSE_STATUS_FAILED' }
197+
: {
198+
closeStatus: 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID',
199+
closeEvent: null,
200+
}),
201+
},
202+
} satisfies DescribeWorkflowResponse);
203+
},
204+
},
182205
...(isDiagnosticsEnabled
183206
? [
184207
{

‎src/views/workflow-diagnostics/config/workflow-diagnostics-disabled-error-panel.config.ts

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎src/views/workflow-diagnostics/helpers/__tests__/get-workflow-diagnostics-error-config.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import {
2+
DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG,
3+
DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG,
4+
} from '../../workflow-diagnostics.constants';
5+
import getWorkflowDiagnosticsErrorConfig from '../get-workflow-diagnostics-error-config';
6+
7+
jest.mock('@/views/workflow-page/helpers/get-workflow-page-error-config', () =>
8+
jest.fn(() => ({
9+
message: 'Failed to load workflow diagnostics',
10+
}))
11+
);
12+
13+
describe(getWorkflowDiagnosticsErrorConfig.name, () => {
14+
beforeEach(() => {
15+
jest.clearAllMocks();
16+
});
17+
18+
it('returns diagnostics disabled error config when error message matches DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG', () => {
19+
const error = new Error(DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG);
20+
21+
const result = getWorkflowDiagnosticsErrorConfig(error);
22+
23+
expect(result).toEqual({
24+
message: 'Workflow Diagnostics is currently disabled',
25+
omitLogging: true,
26+
actions: [
27+
{
28+
kind: 'link-internal',
29+
link: './summary',
30+
label: 'Go to workflow summary',
31+
},
32+
],
33+
});
34+
});
35+
36+
it('returns running workflow error config when error message matches DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG', () => {
37+
const error = new Error(DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG);
38+
39+
const result = getWorkflowDiagnosticsErrorConfig(error);
40+
41+
expect(result).toEqual({
42+
message: 'Cannot load diagnostics for a running workflow',
43+
omitLogging: true,
44+
actions: [
45+
{
46+
kind: 'retry',
47+
label: 'Retry',
48+
},
49+
{
50+
kind: 'link-internal',
51+
link: './summary',
52+
label: 'Go to workflow summary',
53+
},
54+
],
55+
});
56+
});
57+
58+
it('returns result from getWorkflowPageErrorConfig for other errors', () => {
59+
const error = new Error('Some other error');
60+
61+
const result = getWorkflowDiagnosticsErrorConfig(error);
62+
63+
expect(result).toEqual({ message: 'Failed to load workflow diagnostics' });
64+
});
65+
});

‎src/views/workflow-diagnostics/helpers/get-workflow-diagnostics-error-config.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import getWorkflowPageErrorConfig from '@/views/workflow-page/helpers/get-workflow-page-error-config';
2+
import { type WorkflowPageTabErrorConfig } from '@/views/workflow-page/workflow-page-tabs-error/workflow-page-tabs-error.types';
3+
4+
import {
5+
DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG,
6+
DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG,
7+
} from '../workflow-diagnostics.constants';
8+
9+
export default function getWorkflowDiagnosticsErrorConfig(
10+
error: Error
11+
): WorkflowPageTabErrorConfig {
12+
if (error.message === DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG) {
13+
return {
14+
message: 'Workflow Diagnostics is currently disabled',
15+
omitLogging: true,
16+
actions: [
17+
{
18+
kind: 'link-internal',
19+
link: './summary',
20+
label: 'Go to workflow summary',
21+
},
22+
],
23+
};
24+
}
25+
26+
if (error.message === DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG) {
27+
return {
28+
message: 'Cannot load diagnostics for a running workflow',
29+
omitLogging: true,
30+
actions: [
31+
{
32+
kind: 'retry',
33+
label: 'Retry',
34+
},
35+
{
36+
kind: 'link-internal',
37+
link: './summary',
38+
label: 'Go to workflow summary',
39+
},
40+
],
41+
};
42+
}
43+
44+
return getWorkflowPageErrorConfig(
45+
error,
46+
'Failed to load workflow diagnostics',
47+
'diagnostics'
48+
);
49+
}

‎src/views/workflow-diagnostics/workflow-diagnostics.constants.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG =
2+
'Workflow diagnostics is currently disabled';
3+
4+
export const DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG =
5+
'Cannot load diagnostics for a running workflow';

‎src/views/workflow-diagnostics/workflow-diagnostics.tsx

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,42 +2,57 @@
22

33
import React from 'react';
44

5-
import ErrorPanel from '@/components/error-panel/error-panel';
6-
import PanelSection from '@/components/panel-section/panel-section';
75
import SectionLoadingIndicator from '@/components/section-loading-indicator/section-loading-indicator';
86
import { type WorkflowPageTabContentProps } from '@/views/workflow-page/workflow-page-tab-content/workflow-page-tab-content.types';
97

8+
import { useSuspenseDescribeWorkflow } from '../workflow-page/hooks/use-describe-workflow';
109
import useSuspenseIsWorkflowDiagnosticsEnabled from '../workflow-page/hooks/use-is-workflow-diagnostics-enabled/use-suspense-is-workflow-diagnostics-enabled';
1110

12-
import workflowDiagnosticsDisabledErrorPanelConfig from './config/workflow-diagnostics-disabled-error-panel.config';
1311
import useDiagnoseWorkflow from './hooks/use-diagnose-workflow/use-diagnose-workflow';
1412
import WorkflowDiagnosticsContent from './workflow-diagnostics-content/workflow-diagnostics-content';
1513
import WorkflowDiagnosticsFallback from './workflow-diagnostics-fallback/workflow-diagnostics-fallback';
14+
import {
15+
DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG,
16+
DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG,
17+
} from './workflow-diagnostics.constants';
1618

1719
export default function WorkflowDiagnostics({
1820
params,
1921
}: WorkflowPageTabContentProps) {
2022
const { data: isWorkflowDiagnosticsEnabled } =
2123
useSuspenseIsWorkflowDiagnosticsEnabled();
2224

23-
const { data, error, status } = useDiagnoseWorkflow(params, {
24-
enabled: isWorkflowDiagnosticsEnabled,
25+
const {
26+
data: { workflowExecutionInfo },
27+
} = useSuspenseDescribeWorkflow(params);
28+
29+
const isWorkflowClosed = Boolean(
30+
workflowExecutionInfo?.closeStatus &&
31+
workflowExecutionInfo.closeStatus !==
32+
'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID'
33+
);
34+
35+
const { data, status } = useDiagnoseWorkflow(params, {
36+
enabled: isWorkflowDiagnosticsEnabled && isWorkflowClosed,
37+
throwOnError: true,
2538
});
2639

2740
if (!isWorkflowDiagnosticsEnabled) {
28-
return (
29-
<PanelSection>
30-
<ErrorPanel {...workflowDiagnosticsDisabledErrorPanelConfig} />
31-
</PanelSection>
32-
);
41+
throw new Error(DIAGNOSTICS_CONFIG_DISABLED_ERROR_MSG);
42+
}
43+
44+
if (!isWorkflowClosed) {
45+
throw new Error(DIAGNOSTICS_RUNNING_WORKFLOW_ERROR_MSG);
3346
}
3447

3548
if (status === 'pending') {
3649
return <SectionLoadingIndicator />;
3750
}
3851

39-
if (status === 'error') {
40-
throw error;
52+
if (!data) {
53+
throw new Error(
54+
'Unreachable case, react-query should have thrown error above'
55+
);
4156
}
4257

4358
return data.parsingError ? (

‎src/views/workflow-page/config/workflow-page-tabs.config.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
} from 'react-icons/md';
77
import { RiStethoscopeLine } from 'react-icons/ri';
88

9+
import getWorkflowDiagnosticsErrorConfig from '@/views/workflow-diagnostics/helpers/get-workflow-diagnostics-error-config';
910
import WorkflowDiagnostics from '@/views/workflow-diagnostics/workflow-diagnostics';
1011
import WorkflowHistory from '@/views/workflow-history/workflow-history';
1112
import WorkflowQueries from '@/views/workflow-queries/workflow-queries';
@@ -46,12 +47,7 @@ const workflowPageTabsConfig: WorkflowPageTabsConfig<
4647
title: 'Diagnostics',
4748
artwork: RiStethoscopeLine,
4849
content: WorkflowDiagnostics,
49-
getErrorConfig: (err) =>
50-
getWorkflowPageErrorConfig(
51-
err,
52-
'Failed to load workflow diagnostics',
53-
'diagnostics'
54-
),
50+
getErrorConfig: getWorkflowDiagnosticsErrorConfig,
5551
},
5652
queries: {
5753
title: 'Queries',

0 commit comments

Comments
 (0)