Skip to content

Commit b1a9d55

Browse files
authored
[TSPS-645] Improve error handling when submitting Teaspoons jobs (#5439)
1 parent 0c19082 commit b1a9d55

File tree

2 files changed

+156
-25
lines changed

2 files changed

+156
-25
lines changed

src/pages/scientificServices/pipelines/views/RunJob.test.tsx

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@ import userEvent from '@testing-library/user-event';
33
import React from 'react';
44
import { Teaspoons, TeaspoonsContract } from 'src/libs/ajax/teaspoons/Teaspoons';
55
import { Pipeline, PipelineInput, PipelineList, PipelineWithDetails } from 'src/libs/ajax/teaspoons/teaspoons-models';
6+
import { notify } from 'src/libs/notifications';
67
import { mockUserPipelineQuotaDetails } from 'src/pages/scientificServices/pipelines/utils/mock-utils';
7-
import { preparePipelineRun } from 'src/pages/scientificServices/pipelines/utils/submission-utils';
8+
import {
9+
preparePipelineRun,
10+
startPipelineRun,
11+
uploadPipelineFiles,
12+
} from 'src/pages/scientificServices/pipelines/utils/submission-utils';
813
import { asMockedFn, partial, renderWithAppContexts as render } from 'src/testing/test-utils';
914

1015
import { RunJob } from './RunJob';
@@ -19,6 +24,11 @@ jest.mock('src/libs/nav', () => ({
1924
getLink: jest.fn(() => '/'),
2025
}));
2126

27+
jest.mock('src/libs/notifications', () => ({
28+
...jest.requireActual('src/libs/notifications'),
29+
notify: jest.fn(),
30+
}));
31+
2232
jest.mock('src/pages/scientificServices/pipelines/utils/submission-utils', () => ({
2333
...jest.requireActual('src/pages/scientificServices/pipelines/utils/submission-utils'),
2434
preparePipelineRun: jest.fn(),
@@ -340,6 +350,112 @@ describe('RunJob Component', () => {
340350
expect.any(String)
341351
);
342352
});
353+
354+
it('handles error when preparePipelineRun fails', async () => {
355+
const mockError = new Error('Failed to prepare pipeline');
356+
asMockedFn(preparePipelineRun).mockRejectedValue(mockError);
357+
358+
const user = userEvent.setup();
359+
render(<RunJob />);
360+
361+
await waitFor(() => {
362+
expect(screen.getByText('Submit')).toBeInTheDocument();
363+
});
364+
365+
// Fill in required fields
366+
const outputPrefixInput = screen.getByLabelText('outputBasename text input');
367+
await user.type(outputPrefixInput, 'test_output');
368+
369+
const fileInput = document.querySelector('input[type="file"]') as HTMLInputElement;
370+
const file = new File(['test'], 'test.vcf.gz', { type: 'text/plain' });
371+
await waitFor(() => userEvent.upload(fileInput, file));
372+
373+
const submitButton = screen.getByText('Submit');
374+
await waitFor(() => user.click(submitButton));
375+
376+
await waitFor(() => {
377+
expect(notify).toHaveBeenCalledWith('error', 'Error: Failed to prepare pipeline');
378+
});
379+
380+
// Verify submit button is re-enabled after error
381+
expect(submitButton).not.toHaveAttribute('aria-disabled', 'true');
382+
});
383+
384+
it('handles error when uploadPipelineFiles fails', async () => {
385+
asMockedFn(preparePipelineRun).mockResolvedValue({
386+
jobId: 'mock-job-id',
387+
fileInputUploadUrls: {
388+
multiSampleVcf: { signedUrl: 'https://mock-signed-url.com/upload' },
389+
},
390+
});
391+
392+
const mockError = new Error('Upload failed :(');
393+
asMockedFn(uploadPipelineFiles).mockRejectedValue(mockError);
394+
395+
const user = userEvent.setup();
396+
render(<RunJob />);
397+
398+
await waitFor(() => {
399+
expect(screen.getByText('Submit')).toBeInTheDocument();
400+
});
401+
402+
// Fill in required fields
403+
const outputPrefixInput = screen.getByLabelText('outputBasename text input');
404+
await user.type(outputPrefixInput, 'test_output');
405+
406+
const fileInput = document.querySelector('input[type="file"]') as HTMLInputElement;
407+
const file = new File(['test'], 'test.vcf.gz', { type: 'text/plain' });
408+
await waitFor(() => userEvent.upload(fileInput, file));
409+
410+
const submitButton = screen.getByText('Submit');
411+
await waitFor(() => user.click(submitButton));
412+
413+
await waitFor(() => {
414+
expect(notify).toHaveBeenCalledWith('error', 'Error: Upload failed :(');
415+
});
416+
417+
// Verify submit button is re-enabled after error
418+
expect(submitButton).not.toHaveAttribute('aria-disabled', 'true');
419+
});
420+
421+
it('handles error when startPipelineRun fails', async () => {
422+
asMockedFn(preparePipelineRun).mockResolvedValue({
423+
jobId: 'mock-job-id',
424+
fileInputUploadUrls: {
425+
multiSampleVcf: { signedUrl: 'https://mock-signed-url.com/upload' },
426+
},
427+
});
428+
429+
asMockedFn(uploadPipelineFiles).mockResolvedValue(undefined);
430+
431+
const mockError = new Error('Failed to start run');
432+
asMockedFn(startPipelineRun).mockRejectedValue(mockError);
433+
434+
const user = userEvent.setup();
435+
render(<RunJob />);
436+
437+
await waitFor(() => {
438+
expect(screen.getByText('Submit')).toBeInTheDocument();
439+
});
440+
441+
// Fill in required fields
442+
const outputPrefixInput = screen.getByLabelText('outputBasename text input');
443+
await user.type(outputPrefixInput, 'test_output');
444+
445+
const fileInput = document.querySelector('input[type="file"]') as HTMLInputElement;
446+
const file = new File(['test'], 'test.vcf.gz', { type: 'text/plain' });
447+
await waitFor(() => userEvent.upload(fileInput, file));
448+
449+
const submitButton = screen.getByText('Submit');
450+
await waitFor(() => user.click(submitButton));
451+
452+
await waitFor(() => {
453+
expect(notify).toHaveBeenCalledWith('error', 'Error: Failed to start run');
454+
});
455+
456+
// Verify submit button is re-enabled after error
457+
expect(submitButton).not.toHaveAttribute('aria-disabled', 'true');
458+
});
343459
});
344460

345461
describe('uploadPipelineFiles function', () => {

src/pages/scientificServices/pipelines/views/RunJob.tsx

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ export const RunJob = () => {
9999
setIsSubmitting(false);
100100
}
101101

102+
const handlePipelineSubmissionError = (error: unknown, fallbackMessage: string) => {
103+
const errorMessage = error instanceof Error ? error.message : fallbackMessage;
104+
notify('error', `Error: ${errorMessage}`);
105+
setIsSubmitting(false);
106+
};
107+
102108
useEffect(() => {
103109
// Clear selected user inputs when pipeline inputs change
104110
resetSelectedUserInputs();
@@ -136,18 +142,14 @@ export const RunJob = () => {
136142

137143
const handleSubmit = async () => {
138144
if (!selectedPipeline) {
139-
notify('error', 'Missing required fields');
145+
// This should not happen as the submit button is disabled when no pipeline is selected,
146+
// but it's helpful as a type-guard here since Typescript obviously can't infer that
147+
notify('error', 'Please select a pipeline before submitting.');
140148
return;
141149
}
142150

143-
const pipeline = pipelinesList?.find((pipeline) => pipeline?.pipelineVersion === selectedPipeline?.pipelineVersion);
144-
const pipelineName = pipeline?.pipelineName;
145-
146-
// Only proceed if we have a valid pipeline name
147-
if (!pipelineName) {
148-
notify('error', 'No pipeline selected or pipeline name not found');
149-
return;
150-
}
151+
const pipelineName = selectedPipeline.pipelineName;
152+
const pipelineVersion = selectedPipeline.pipelineVersion;
151153

152154
// Filter out empty string inputs to avoid sending them to the backend.
153155
// At this point we've already validated required inputs are filled, so this
@@ -158,35 +160,48 @@ export const RunJob = () => {
158160

159161
setIsSubmitting(true);
160162

161-
const { jobId: preparedJobId, fileInputUploadUrls } = await preparePipelineRun(
162-
pipelineName,
163-
selectedPipeline.pipelineVersion,
164-
filteredUserInputs,
165-
runDescription
166-
);
163+
let preparedJobId: string;
164+
let fileInputUploadUrls: Record<string, { signedUrl: string }>;
167165

168-
setPreparedJobId(preparedJobId);
166+
// Prepare pipeline run
167+
try {
168+
const result = await preparePipelineRun(pipelineName, pipelineVersion, filteredUserInputs, runDescription);
169+
preparedJobId = result.jobId;
170+
fileInputUploadUrls = result.fileInputUploadUrls;
171+
setPreparedJobId(preparedJobId);
172+
} catch (error) {
173+
handlePipelineSubmissionError(error, 'Failed to prepare pipeline run');
174+
return;
175+
}
169176

177+
// Upload pipeline input files
170178
try {
171179
await uploadPipelineFiles(
172180
pipelineName,
173-
selectedPipeline.pipelineVersion,
181+
pipelineVersion,
174182
pipelineInputs,
175183
filteredUserInputs,
176184
fileInputUploadUrls,
177185
setUploadState
178186
);
187+
} catch (error) {
188+
handlePipelineSubmissionError(error, 'File upload failed');
189+
return;
190+
}
179191

192+
// Submit the pipeline run
193+
try {
180194
await onUploadComplete(preparedJobId);
181195
} catch (error) {
182-
const errorMessage = error instanceof Error ? error.message : '';
183-
console.error(errorMessage);
184-
} finally {
185-
Metrics().captureEvent(Events.teaspoons.submitJob, {
186-
pipelineName,
187-
pipelineVersion: selectedPipeline.pipelineVersion,
188-
});
196+
handlePipelineSubmissionError(error, 'Failed to start pipeline run');
197+
return;
189198
}
199+
200+
setIsSubmitting(false);
201+
Metrics().captureEvent(Events.teaspoons.submitJob, {
202+
pipelineName,
203+
pipelineVersion,
204+
});
190205
};
191206

192207
return (

0 commit comments

Comments
 (0)