Skip to content

Commit 8375c3e

Browse files
authored
PRMDR-876 Virus Scanner Timeouts (#373)
* [PRMDR-876] Implement retry logic for virus scan * [PRMDR-876] remove unused import * add temporary console.log to verify delay * add logging for debug within sandbox * remove console.log * extract number of seconds as constant * [PRMDR-876] fix cypress tests * [PRMDR-876] unify duplicated imports * minor refactoring
1 parent 5ac8995 commit 8375c3e

File tree

7 files changed

+129
-8
lines changed

7 files changed

+129
-8
lines changed

app/cypress/e2e/0-ndr-core-tests/gp_user_workflows/upload_lloyd_george_is_bsol_gp_admin.cy.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ describe('GP Workflow: Upload Lloyd George record when user is GP admin BSOL and
172172
'contain',
173173
uploadedFileNames.LG[singleFileUsecaseIndex],
174174
);
175-
cy.wait(20);
175+
cy.wait('@upload_confirm');
176176

177177
cy.getByTestId('upload-complete-page')
178178
.should('include.text', 'Record uploaded for')
@@ -218,6 +218,8 @@ describe('GP Workflow: Upload Lloyd George record when user is GP admin BSOL and
218218
.should('contain', uploadedFileNames.LG[multiFileUsecaseIndex][0])
219219
.should('contain', uploadedFileNames.LG[multiFileUsecaseIndex][1]);
220220

221+
cy.wait('@upload_confirm');
222+
221223
cy.getByTestId('upload-complete-page')
222224
.should('include.text', 'Record uploaded for')
223225
.should('include.text', 'You have successfully uploaded 2 files')
@@ -260,6 +262,8 @@ describe('GP Workflow: Upload Lloyd George record when user is GP admin BSOL and
260262
.should('contain', uploadedFileNames.LG[multiFileUsecaseIndex][0])
261263
.should('contain', uploadedFileNames.LG[multiFileUsecaseIndex][1]);
262264

265+
cy.wait('@upload_confirm');
266+
263267
cy.getByTestId('upload-complete-page')
264268
.should('include.text', 'Record uploaded for')
265269
.should('include.text', 'You have successfully uploaded 2 files')
@@ -406,6 +410,7 @@ describe('GP Workflow: Upload Lloyd George record when user is GP admin BSOL and
406410
cy.getByTestId('error-box-link').should('exist');
407411
cy.getByTestId('error-box-link').click();
408412
cy.wait('@s3_retry_upload');
413+
cy.wait('@upload_confirm');
409414

410415
cy.getByTestId('upload-complete-page')
411416
.should('include.text', 'Record uploaded for')

app/src/helpers/requests/uploadDocument.test.ts

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import axios, { AxiosError } from 'axios';
22
import { buildDocument, buildTextFile } from '../test/testBuilders';
33
import { DOCUMENT_UPLOAD_STATE as documentUploadStates } from '../../types/pages/UploadDocumentsPage/types';
4-
import { UpdateStateArgs, updateDocumentState } from './uploadDocuments';
4+
import { UpdateStateArgs, updateDocumentState, virusScanResult } from './uploadDocuments';
5+
import waitForSeconds from '../utils/waitForSeconds';
56

67
// Mock out all top level functions, such as get, put, delete and post:
78
jest.mock('axios');
9+
jest.mock('../utils/waitForSeconds');
10+
811
const mockedAxios = axios as jest.Mocked<typeof axios>;
12+
const mockedWaitForSeconds = waitForSeconds as jest.Mocked<typeof waitForSeconds>;
913

1014
describe('[POST] updateDocumentState', () => {
1115
test('updateDocumentState handles a 2XX response', async () => {
@@ -23,3 +27,60 @@ describe('[POST] updateDocumentState', () => {
2327
expect(() => updateDocumentState(args)).not.toThrow(error);
2428
});
2529
});
30+
31+
describe('virusScanResult', () => {
32+
const virusScanArgs = {
33+
documentReference: 'mock_doc_id',
34+
baseUrl: '/test',
35+
baseHeaders: { 'Content-Type': 'application/json', test: 'test' },
36+
};
37+
const cleanResponse = { status: 200 };
38+
const uncleanResponse = { response: { status: 400 } };
39+
const gatewayTimeoutResponse = { response: { status: 504 } };
40+
41+
it('return CLEAN if virus scan api call result was clean', async () => {
42+
mockedAxios.post.mockResolvedValueOnce(cleanResponse);
43+
44+
const result = await virusScanResult(virusScanArgs);
45+
46+
expect(result).toEqual(documentUploadStates.CLEAN);
47+
expect(mockedWaitForSeconds).not.toBeCalled();
48+
});
49+
50+
it('return INFECTED if virus scan api call result was unclean', async () => {
51+
mockedAxios.post.mockRejectedValueOnce(uncleanResponse);
52+
53+
const result = await virusScanResult(virusScanArgs);
54+
55+
expect(result).toEqual(documentUploadStates.INFECTED);
56+
expect(mockedWaitForSeconds).not.toBeCalled();
57+
});
58+
59+
it('retry up to 3 times if virus scan api call timed out', async () => {
60+
mockedAxios.post
61+
.mockRejectedValueOnce(gatewayTimeoutResponse)
62+
.mockRejectedValueOnce(gatewayTimeoutResponse)
63+
.mockResolvedValueOnce(cleanResponse);
64+
65+
const delay_between_retry_in_seconds = 5;
66+
67+
const result = await virusScanResult(virusScanArgs);
68+
69+
expect(result).toEqual(documentUploadStates.CLEAN);
70+
71+
expect(mockedAxios.post).toBeCalledTimes(3);
72+
expect(mockedWaitForSeconds).toBeCalledTimes(2);
73+
expect(mockedWaitForSeconds).toHaveBeenCalledWith(delay_between_retry_in_seconds);
74+
});
75+
76+
it('throw an error if timed out for 3 times', async () => {
77+
mockedAxios.post.mockRejectedValue(gatewayTimeoutResponse);
78+
79+
await expect(virusScanResult(virusScanArgs)).rejects.toThrowError(
80+
'Virus scan api calls timed-out for 3 attempts.',
81+
);
82+
83+
expect(mockedAxios.post).toBeCalledTimes(3);
84+
expect(mockedWaitForSeconds).toBeCalledTimes(3);
85+
});
86+
});

app/src/helpers/requests/uploadDocuments.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ import axios, { AxiosError } from 'axios';
1010
import { S3Upload, S3UploadFields, UploadSession } from '../../types/generic/uploadResult';
1111
import { Dispatch, SetStateAction } from 'react';
1212
import { setDocument } from '../../pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage';
13+
import waitForSeconds from '../utils/waitForSeconds';
14+
15+
const VIRUS_SCAN_RETRY_LIMIT = 3;
16+
const DELAY_BETWEEN_VIRUS_SCAN_RETRY_IN_SECONDS = 5;
17+
const TIMEOUT_ERROR_STATUS_CODE = 504;
18+
const TIMEOUT_ERROR = 'TIMEOUT_ERROR';
1319

1420
type FileKeyBuilder = {
1521
[key in DOCUMENT_TYPE]: string[];
@@ -52,11 +58,20 @@ type UploadConfirmationArgs = {
5258
uploadSession: UploadSession;
5359
};
5460

55-
export const virusScanResult = async ({
56-
documentReference,
57-
baseUrl,
58-
baseHeaders,
59-
}: VirusScanArgs) => {
61+
export const virusScanResult = async (virusScanArgs: VirusScanArgs) => {
62+
for (let i = 0; i < VIRUS_SCAN_RETRY_LIMIT; i++) {
63+
const scanResult = await requestVirusScan(virusScanArgs);
64+
if (scanResult === TIMEOUT_ERROR) {
65+
await waitForSeconds(DELAY_BETWEEN_VIRUS_SCAN_RETRY_IN_SECONDS);
66+
continue;
67+
}
68+
return scanResult;
69+
}
70+
71+
throw new Error(`Virus scan api calls timed-out for ${VIRUS_SCAN_RETRY_LIMIT} attempts.`);
72+
};
73+
74+
const requestVirusScan = async ({ documentReference, baseUrl, baseHeaders }: VirusScanArgs) => {
6075
const virusScanGatewayUrl = baseUrl + endpoints.VIRUS_SCAN;
6176
const body = { documentReference };
6277
try {
@@ -67,6 +82,10 @@ export const virusScanResult = async ({
6782
});
6883
return DOCUMENT_UPLOAD_STATE.CLEAN;
6984
} catch (e) {
85+
const error = e as AxiosError;
86+
if (error.response?.status === TIMEOUT_ERROR_STATUS_CODE) {
87+
return TIMEOUT_ERROR;
88+
}
7089
return DOCUMENT_UPLOAD_STATE.INFECTED;
7190
}
7291
};
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import waitForSeconds from './waitForSeconds';
2+
3+
describe('waitForSeconds', () => {
4+
it('postpone code execution by given number of seconds', async () => {
5+
const mockFunction = jest.fn();
6+
const secondsToWait = 2;
7+
const testCode = async () => {
8+
await waitForSeconds(secondsToWait);
9+
mockFunction(Date.now());
10+
};
11+
12+
const startTime = Date.now();
13+
await testCode();
14+
15+
expect(mockFunction).toHaveBeenCalledTimes(1);
16+
17+
const timestampWhenMockFunctionCalled = mockFunction.mock.calls[0][0];
18+
expect(timestampWhenMockFunctionCalled).toBeGreaterThanOrEqual(
19+
startTime + 1000 * secondsToWait,
20+
);
21+
expect(timestampWhenMockFunctionCalled).toBeLessThanOrEqual(
22+
startTime + 1000 * (secondsToWait + 1),
23+
);
24+
});
25+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const waitForSeconds = (seconds: number): Promise<void> => {
2+
return new Promise((resolve) => {
3+
setTimeout(resolve, seconds * 1000);
4+
});
5+
};
6+
7+
export default waitForSeconds;

app/src/pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ jest.mock('moment', () => {
3434
return jest.requireActual('moment')(arg);
3535
};
3636
});
37+
jest.mock('../../helpers/utils/waitForSeconds');
3738

3839
const mockedUsePatient = usePatient as jest.Mock;
3940
const mockUploadDocuments = uploadDocuments as jest.Mock;

app/src/pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { Outlet, Route, Routes, useNavigate } from 'react-router';
2323
import { errorToParams } from '../../helpers/utils/errorToParams';
2424
import LloydGeorgeRetryUploadStage from '../../components/blocks/_lloydGeorge/lloydGeorgeRetryUploadStage/LloydGeorgeRetryUploadStage';
2525
import { getLastURLPath } from '../../helpers/utils/urlManipulations';
26+
import waitForSeconds from '../../helpers/utils/waitForSeconds';
2627
export enum LG_UPLOAD_STAGE {
2728
SELECT = 0,
2829
UPLOAD = 1,
@@ -40,6 +41,8 @@ type UpdateDocumentArgs = {
4041
ref?: string;
4142
};
4243

44+
const DELAY_BEFORE_VIRUS_SCAN_IN_SECONDS = 3;
45+
4346
export const setDocument = (
4447
setDocuments: Dispatch<SetStateAction<UploadDocument[]>>,
4548
{ id, state, progress, attempts, ref }: UpdateDocumentArgs,
@@ -68,7 +71,6 @@ function LloydGeorgeUploadPage() {
6871
const nhsNumber: string = patientDetails?.nhsNumber ?? '';
6972
const baseUrl = useBaseAPIUrl();
7073
const baseHeaders = useBaseAPIHeaders();
71-
// const [stage, setStage] = useState<LG_UPLOAD_STAGE>(LG_UPLOAD_STAGE.SELECT);
7274
const [documents, setDocuments] = useState<Array<UploadDocument>>([]);
7375
const [uploadSession, setUploadSession] = useState<UploadSession | null>(null);
7476
const confirmedReference = useRef(false);
@@ -164,6 +166,7 @@ function LloydGeorgeUploadPage() {
164166
state: DOCUMENT_UPLOAD_STATE.SCANNING,
165167
progress: 'scan',
166168
});
169+
await waitForSeconds(DELAY_BEFORE_VIRUS_SCAN_IN_SECONDS);
167170
const virusDocumentState = await virusScanResult({
168171
documentReference: document.key ?? '',
169172
baseUrl,

0 commit comments

Comments
 (0)