From 92ab73665bf079de96cf585cb9ac240571d8ab6b Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Tue, 10 Feb 2026 15:23:36 +0000 Subject: [PATCH 1/2] Handle cases where CDP's file upload service has expired the upload ID --- .../FileUploadPageController.test.ts | 84 +++++++++++++++++++ .../FileUploadPageController.ts | 17 +++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts index 32315eb91..393ba30a1 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/dot-notation */ import { ComponentType, type ComponentDef } from '@defra/forms-model' +import Boom from '@hapi/boom' import { type ValidationErrorItem, type ValidationResult } from 'joi' import { @@ -195,6 +196,89 @@ describe('FileUploadPageController', () => { ) }) + it('initiates new upload when getUploadStatus throws a 404 error', async () => { + const state = { + upload: { + [controller.path]: { + upload: { + uploadId: 'some-id', + uploadUrl: 'some-url', + statusUrl: 'some-status-url' + }, + files: [] + } + } + } as unknown as FormSubmissionState + + const notFoundError = Boom.notFound('Upload not found') + + jest + .spyOn(uploadService, 'getUploadStatus') + .mockRejectedValue(notFoundError) + + const testController = controller as TestableFileUploadPageController + const initiateSpy = jest.spyOn( + testController, + 'initiateAndStoreNewUpload' + ) + initiateSpy.mockResolvedValue(state as never) + + const result = await controller['checkUploadStatus'](request, state, 1) + + expect(initiateSpy).toHaveBeenCalledWith(request, state) + expect(result).toBe(state) + }) + + it('re-throws non-404 Boom errors from getUploadStatus', async () => { + const state = { + upload: { + [controller.path]: { + upload: { + uploadId: 'some-id', + uploadUrl: 'some-url', + statusUrl: 'some-status-url' + }, + files: [] + } + } + } as unknown as FormSubmissionState + + const serverError = Boom.internal('Server error') + + jest + .spyOn(uploadService, 'getUploadStatus') + .mockRejectedValue(serverError) + + await expect( + controller['checkUploadStatus'](request, state, 1) + ).rejects.toThrow('Server error') + }) + + it('re-throws non-Boom errors from getUploadStatus', async () => { + const state = { + upload: { + [controller.path]: { + upload: { + uploadId: 'some-id', + uploadUrl: 'some-url', + statusUrl: 'some-status-url' + }, + files: [] + } + } + } as unknown as FormSubmissionState + + const networkError = new Error('Network failure') + + jest + .spyOn(uploadService, 'getUploadStatus') + .mockRejectedValue(networkError) + + await expect( + controller['checkUploadStatus'](request, state, 1) + ).rejects.toThrow('Network failure') + }) + it('handles pending upload with backoff and retries', async () => { const state = { upload: { diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts index 79e36ca6e..603681974 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts @@ -327,7 +327,22 @@ export class FileUploadPageController extends QuestionPageController { } const uploadId = upload.uploadId - const statusResponse = await getUploadStatus(uploadId) + + let statusResponse + + try { + statusResponse = await getUploadStatus(uploadId) + } catch (err) { + // if the user loads a file upload page and queries the cached upload, after the upload has + // expired in CDP, we will get a 404 from the getUploadStatus endpoint. + // In this case we want to initiate a new upload and return that state, so the form + // doesn't blow up for the end user. + if (Boom.isBoom(err) && err.output.statusCode === 404) { + return this.initiateAndStoreNewUpload(request, state) + } + throw err + } + if (!statusResponse) { throw Boom.badRequest( `Unexpected empty response from getUploadStatus for ${uploadId}` From 9fbb78a8a7476b75f86d544350aa123a1e23fddf Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Tue, 10 Feb 2026 15:39:58 +0000 Subject: [PATCH 2/2] use enum --- .../engine/pageControllers/FileUploadPageController.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts index 603681974..1a59a3e01 100644 --- a/src/server/plugins/engine/pageControllers/FileUploadPageController.ts +++ b/src/server/plugins/engine/pageControllers/FileUploadPageController.ts @@ -1,6 +1,7 @@ import { ComponentType, type PageFileUpload } from '@defra/forms-model' import Boom from '@hapi/boom' import { wait } from '@hapi/hoek' +import { StatusCodes } from 'http-status-codes' import { type ValidationErrorItem } from 'joi' import { @@ -337,7 +338,10 @@ export class FileUploadPageController extends QuestionPageController { // expired in CDP, we will get a 404 from the getUploadStatus endpoint. // In this case we want to initiate a new upload and return that state, so the form // doesn't blow up for the end user. - if (Boom.isBoom(err) && err.output.statusCode === 404) { + if ( + Boom.isBoom(err) && + err.output.statusCode === StatusCodes.NOT_FOUND.valueOf() + ) { return this.initiateAndStoreNewUpload(request, state) } throw err