Merge pull request #310 from DEFRA/bugfix/DF-834-file-upload-expired

Handle cases where CDP's file upload service can't find the upload ID

Author: alexluckett

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: {

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 {
@@ -327,7 +328,25 @@ 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 === StatusCodes.NOT_FOUND.valueOf()
+      ) {
+        return this.initiateAndStoreNewUpload(request, state)
+      }
+      throw err
+    }
+
     if (!statusResponse) {
       throw Boom.badRequest(
         `Unexpected empty response from getUploadStatus for ${uploadId}`