Merge pull request #146 from DEFRA/DSFAAP-747_timeout_for_notify_calls

Dsfaap 747 timeout for notify calls

src/config/config.js

@@ -233,6 +233,12 @@ export const config = convict({
       default: null,
       nullable: true,
       env: 'NOTIFY_CASE_DELIVERY_EMAIL_ADDRESS'
+    },
+    timeout: {
+      doc: 'Timeout for notify requests in milliseconds',
+      format: Number,
+      default: 10_000,
+      env: 'NOTIFY_TIMEOUT'
     }
   }
 })

src/server/common/connectors/notify/notify.integration.test.js

@@ -0,0 +1,34 @@
+import { sendNotification } from './notify.js'
+import { config } from '~/src/config/config.js'
+
+jest.mock(
+  '~/src/server/common/connectors/notify/notify-token-utils.js',
+  () => ({
+    createToken: jest.fn().mockReturnValue('mocked-jwt-token')
+  })
+)
+
+describe('sendNotification (integration)', () => {
+  it('should abort if the configured timeout is hit', async () => {
+    const configGet = config.get.bind(config)
+    const notifyConfig = {
+      ...config.get('notify'),
+      timeout: 0
+    }
+    jest.spyOn(config, 'get').mockImplementation((name) => {
+      if (name === 'notify') {
+        return notifyConfig
+      } else {
+        return configGet(name)
+      }
+    })
+
+    const testData = { content: 'test' }
+
+    const result = sendNotification(testData)
+
+    await expect(result).rejects.toThrow(
+      'Request to GOV.uk notify timed out after 0ms'
+    )
+  })
+})

src/server/common/connectors/notify/notify.js

@@ -5,28 +5,42 @@ import { createToken } from '~/src/server/common/connectors/notify/notify-token-
 /**
  * @typedef {{ content: string}} NotifyContent
  */
-const notifyConfig = config.get('notify')
+
+export const NOTIFY_URL =
+  'https://api.notifications.service.gov.uk/v2/notifications/email'
 
 /**
  * @param {NotifyContent} data
  */
 export async function sendNotification(data) {
+  const { timeout, ...notifyConfig } = config.get('notify')
+
   const body = JSON.stringify({
     template_id: notifyConfig.templateId,
     email_address: notifyConfig.caseDeliveryEmailAddress,
     personalisation: data
   })
 
-  const response = await proxyFetch(
-    'https://api.notifications.service.gov.uk/v2/notifications/email',
-    {
+  let response
+
+  try {
+    response = await proxyFetch(NOTIFY_URL, {
       method: 'POST',
       body,
       headers: {
         Authorization: 'Bearer ' + createToken(notifyConfig.apiKey)
-      }
+      },
+      signal: AbortSignal.timeout(timeout)
+    })
+  } catch (err) {
+    if (err.code && err.code === err.TIMEOUT_ERR) {
+      throw new Error(`Request to GOV.uk notify timed out after ${timeout}ms`)
+    } else {
+      throw new Error(
+        `Request to GOV.uk notify failed with error: ${err.message}`
+      )
     }
-  )
+  }
 
   if (!response.ok) {
     const responseBody = await response.json()
@@ -35,5 +49,6 @@ export async function sendNotification(data) {
       `HTTP failure from GOV.uk notify: status ${response.status} with the following errors: ${errors.join(', ')}`
     )
   }
+
   return response
 }

src/server/common/connectors/notify/notify.test.js

@@ -1,11 +1,8 @@
-import { sendNotification } from './notify.js'
-import { proxyFetch } from '~/src/server/common/helpers/proxy.js'
+import { NOTIFY_URL, sendNotification } from './notify.js'
+import * as proxyFetchObject from '~/src/server/common/helpers/proxy.js'
 import { config } from '~/src/config/config.js'
 
-jest.mock('~/src/server/common/helpers/proxy.js', () => ({
-  proxyFetch: jest.fn()
-}))
-const mockProxyFetch = /** @type {jest.Mock} */ (proxyFetch)
+const testData = { content: 'test' }
 
 jest.mock(
   '~/src/server/common/connectors/notify/notify-token-utils.js',
@@ -14,22 +11,30 @@ jest.mock(
   })
 )
 
-const testData = { content: 'test' }
-
 describe('sendNotification', () => {
+  afterEach(() => {
+    jest.restoreAllMocks()
+  })
+
   it('should send a notification successfully', async () => {
     const mockResponse = { ok: true }
-    mockProxyFetch.mockImplementation(() => Promise.resolve(mockResponse))
+    const mockProxyFetch = jest
+      .spyOn(proxyFetchObject, 'proxyFetch')
+      .mockImplementation(() => Promise.resolve(mockResponse))
+
     const response = await sendNotification(testData)
 
     const [url, options] = mockProxyFetch.mock.calls[0]
+    /**
+     * @type {string | undefined}
+     */
+    // @ts-expect-error: options.body might note be a string
+    const body = options.body
 
-    expect(url).toBe(
-      'https://api.notifications.service.gov.uk/v2/notifications/email'
-    )
+    expect(url).toBe(NOTIFY_URL)
 
     expect(options.method).toBe('POST')
-    expect(JSON.parse(options.body)).toEqual({
+    expect(JSON.parse(body ?? '')).toEqual({
       personalisation: testData,
       template_id: config.get('notify').templateId,
       email_address: config.get('notify').caseDeliveryEmailAddress
@@ -62,10 +67,43 @@ describe('sendNotification', () => {
         ]
       })
     }
-    mockProxyFetch.mockImplementation(() => Promise.resolve(mockResponse))
+    jest
+      .spyOn(proxyFetchObject, 'proxyFetch')
+      .mockImplementation(() => Promise.resolve(mockResponse))
 
     await expect(sendNotification(testData)).rejects.toThrow(
       "HTTP failure from GOV.uk notify: status 400 with the following errors: Can't send to this recipient using a team-only API key, Can't send to this recipient when service is in trial mode"
     )
   })
+
+  it('should throw an error on reject', async () => {
+    const errorMessage = 'test error'
+    jest
+      .spyOn(proxyFetchObject, 'proxyFetch')
+      .mockImplementation(() => Promise.reject(new Error(errorMessage)))
+
+    await expect(sendNotification(testData)).rejects.toThrow(
+      `Request to GOV.uk notify failed with error: ${errorMessage}`
+    )
+  })
+
+  it('should call proxyFetch passing the correct timeout', async () => {
+    const expectedTimeout = 10000
+    const mockResponse = { ok: true }
+    const fetchSpy = jest
+      .spyOn(proxyFetchObject, 'proxyFetch')
+      .mockImplementation(() => Promise.resolve(mockResponse))
+    const abortSignalTimeoutSpy = jest.spyOn(global.AbortSignal, 'timeout')
+
+    await sendNotification(testData)
+    expect(abortSignalTimeoutSpy).toHaveBeenCalledWith(expectedTimeout)
+
+    const mockSignal = abortSignalTimeoutSpy.mock.results[0].value
+    expect(fetchSpy).toHaveBeenCalledWith(
+      NOTIFY_URL,
+      expect.objectContaining({
+        signal: mockSignal
+      })
+    )
+  })
 })