Update: [AEA-3861] - Provide error response if data store update time…

…s out (#146)

## Summary

🎫 [AEA-3861](https://nhsd-jira.digital.nhs.uk/browse/AEA-3861) Provide
error response if data store update times out

- Routine Change

### Details

The solution must ensure that the OperationOutcome relating to timed out
request contains an issue resource constructed using the 504 error code
in the case where the DynomoDB request times out. See timeout
requirement in [NPPTS NFR
Specification](https://nhsd-confluence.digital.nhs.uk/display/APIMC/NPPT+Service+-+Non-functional+Requirements).

.pre-commit-config.yaml

@@ -41,14 +41,6 @@ repos:
         types_or: [ts, tsx, javascript, jsx, json]
         pass_filenames: false
 
-      - id: oas-build-checks
-        name: Build OAS
-        entry: make
-        args: ["build-specification"]
-        language: system
-        files: ^(examples|packages\/specification)
-        types_or: [json, yaml]
-
       - id: lint-samtemplates
         name: Lint sam templates
         entry: make

Makefile

@@ -17,9 +17,6 @@ install-node:
 install-hooks: install-python
 	poetry run pre-commit install --install-hooks --overwrite
 
-build-specification:
-	$(MAKE) --directory=packages/specification build
-
 sam-build: sam-validate compile
 	sam build --template-file SAMtemplates/main_template.yaml --region eu-west-2
 

README.md

@@ -125,10 +125,6 @@ There are `make` commands that are run as part of the CI pipeline and help alias
 - `install-hooks` Installs git pre commit hooks
 - `install` Runs all install targets
 
-#### Build targets
-
-- `build-specification` Builds the specification component
-
 #### SAM targets
 
 These are used to do common commands

packages/specification/eps-prescription-status-update-api.yaml

@@ -283,7 +283,7 @@ x-nhsd-apim:
     grants:
       app-level0: []
   target:
-    type: hosted
+    type: external
     healthcheck: /_status
     url: https://psu.dev.eps.national.nhs.uk
     security:

packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts

@@ -6,17 +6,19 @@ import middy from "@middy/core"
 import inputOutputLogger from "@middy/input-output-logger"
 import errorHandler from "@nhs/fhir-middy-error-handler"
 import {Bundle, BundleEntry, Task} from "fhir/r4"
-
+import {persistDataItems} from "./utils/databaseClient"
+import {jobWithTimeout, hasTimedOut} from "./utils/timeoutUtils"
 import {transactionBundle, validateEntry} from "./validation/content"
 import {
   accepted,
   badRequest,
   bundleWrap,
   createSuccessResponseEntries,
-  serverError
+  serverError,
+  timeoutResponse
 } from "./utils/responses"
-import {persistDataItems} from "./utils/databaseClient"
 
+const LAMBDA_TIMEOUT_MS = 9500
 const logger = new Logger({serviceName: "updatePrescriptionStatus"})
 
 export interface DataItem {
@@ -68,9 +70,16 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
   }
 
   const dataItems = buildDataItems(requestEntries, xRequestID)
+  const persistSuccess = persistDataItems(dataItems)
+  const persistResponse = await jobWithTimeout(LAMBDA_TIMEOUT_MS, persistSuccess)
+
+  if (hasTimedOut(persistResponse)) {
+    responseEntries = [timeoutResponse()]
+    logger.info("DynamoDB operation timed out.")
+    return response(408, responseEntries)
+  }
 
-  const persistSuccess = await persistDataItems(dataItems)
-  if (!persistSuccess) {
+  if (!persistResponse) {
     responseEntries = [serverError()]
     return response(500, responseEntries)
   }

packages/updatePrescriptionStatus/src/utils/databaseClient.ts

@@ -3,6 +3,7 @@ import {DynamoDBClient, TransactWriteItem, TransactWriteItemsCommand} from "@aws
 import {marshall} from "@aws-sdk/util-dynamodb"
 
 import {DataItem} from "../updatePrescriptionStatus"
+import {Timeout} from "./timeoutUtils"
 
 const logger = new Logger({serviceName: "databaseClient"})
 const client = new DynamoDBClient()
@@ -21,14 +22,14 @@ function createTransactionCommand(dataItems: Array<DataItem>): TransactWriteItem
   return new TransactWriteItemsCommand({TransactItems: transactItems})
 }
 
-export async function persistDataItems(dataItems: Array<DataItem>): Promise<boolean> {
+export async function persistDataItems(dataItems: Array<DataItem>): Promise<boolean | Timeout> {
   const transactionCommand = createTransactionCommand(dataItems)
   try {
     logger.info("Sending TransactWriteItemsCommand to DynamoDB.", {command: transactionCommand})
     await client.send(transactionCommand)
     logger.info("TransactWriteItemsCommand sent to DynamoDB successfully.", {command: transactionCommand})
     return true
-  } catch(e) {
+  } catch (e) {
     logger.error("Error sending TransactWriteItemsCommand to DynamoDB.", {error: e})
     return false
   }

packages/updatePrescriptionStatus/src/utils/responses.ts

@@ -21,16 +21,7 @@ export function badRequest(diagnostics: string, fullUrl: string | undefined = un
           {
             code: "processing",
             severity: "error",
-            diagnostics: diagnostics,
-            details: {
-              coding: [
-                {
-                  system: "https://fhir.nhs.uk/CodeSystem/Spine-ErrorOrWarningCode",
-                  code: "INVALID_VALUE",
-                  display: "Invalid value"
-                }
-              ]
-            }
+            diagnostics: diagnostics
           }
         ]
       }
@@ -42,6 +33,27 @@ export function badRequest(diagnostics: string, fullUrl: string | undefined = un
   return bundleEntry
 }
 
+export function timeoutResponse(): BundleEntry {
+  return {
+    response: {
+      status: "504 The request timed out",
+      outcome: {
+        resourceType: "OperationOutcome",
+        meta: {
+          lastUpdated: new Date().toISOString()
+        },
+        issue: [
+          {
+            code: "timeout",
+            severity: "fatal",
+            diagnostics: "The Server has timed out while processing the request sent by the client."
+          }
+        ]
+      }
+    }
+  }
+}
+
 export function accepted(fullUrl: string): BundleEntry {
   return {
     fullUrl: fullUrl,
@@ -99,16 +111,7 @@ export function serverError(): BundleEntry {
           {
             code: "exception",
             severity: "fatal",
-            diagnostics: "The Server has encountered an error processing the request.",
-            details: {
-              coding: [
-                {
-                  system: "https://fhir.nhs.uk/CodeSystem/http-error-codes",
-                  code: "SERVER_ERROR",
-                  display: "Server error"
-                }
-              ]
-            }
+            diagnostics: "The Server has encountered an error processing the request."
           }
         ]
       }
@@ -117,5 +120,5 @@ export function serverError(): BundleEntry {
 }
 
 export function createSuccessResponseEntries(entries: Array<BundleEntry>) {
-  return entries.map(e => created(e.fullUrl!))
+  return entries.map((e) => created(e.fullUrl!))
 }

packages/updatePrescriptionStatus/src/utils/timeoutUtils.ts

@@ -0,0 +1,16 @@
+export interface Timeout {
+  isTimeout: true
+}
+
+export async function jobWithTimeout<T>(timeoutMS: number, job: Promise<T>): Promise<T | Timeout> {
+  const timeoutPromise: Promise<Timeout> = new Promise((resolve) => {
+    setTimeout(() => {
+      resolve({isTimeout: true})
+    }, timeoutMS)
+  })
+  return Promise.race([job, timeoutPromise])
+}
+
+export function hasTimedOut<T>(response: T | Timeout): response is Timeout {
+  return !!(response as Timeout)?.isTimeout
+}

packages/updatePrescriptionStatus/tests/testHandler.test.ts

@@ -24,10 +24,16 @@ import requestMultipleMissingFields from "../../specification/examples/request-m
 import requestNoItems from "../../specification/examples/request-no-items.json"
 import responseSingleItem from "../../specification/examples/response-single-item.json"
 import responseMultipleItems from "../../specification/examples/response-multiple-items.json"
-import {badRequest, bundleWrap, serverError} from "../src/utils/responses"
+import {
+  badRequest,
+  bundleWrap,
+  serverError,
+  timeoutResponse
+} from "../src/utils/responses"
 
 const {mockSend, mockTransact} = mockDynamoDBClient()
 const {handler} = await import("../src/updatePrescriptionStatus")
+const LAMBDA_TIMEOUT_MS = 9500 // 9.5 sec
 
 describe("Integration tests for updatePrescriptionStatus handler", () => {
   beforeEach(() => {
@@ -94,29 +100,26 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
       httpResponseCode: 200,
       scenarioDescription: "200 status code if there are no entries to process"
     }
-  ])(
-    "should return $scenarioDescription",
-    async ({example, httpResponseCode}) => {
-      const event: APIGatewayProxyEvent = generateMockEvent(example)
+  ])("should return $scenarioDescription", async ({example, httpResponseCode}) => {
+    const event: APIGatewayProxyEvent = generateMockEvent(example)
 
-      const response: APIGatewayProxyResult = await handler(event, {})
+    const response: APIGatewayProxyResult = await handler(event, {})
 
-      const responseBody = JSON.parse(response.body)
-      expect(response.statusCode).toBe(httpResponseCode)
-      expect(responseBody).toHaveProperty("resourceType", "Bundle")
-      expect(responseBody).toHaveProperty("type", "transaction-response")
-    }
-  )
+    const responseBody = JSON.parse(response.body)
+    expect(response.statusCode).toBe(httpResponseCode)
+    expect(responseBody).toHaveProperty("resourceType", "Bundle")
+    expect(responseBody).toHaveProperty("type", "transaction-response")
+  })
 
   it("when missing fields, expect 400 status code and message indicating missing fields", async () => {
     const event: APIGatewayProxyEvent = generateMockEvent(requestMissingFields)
 
     const response: APIGatewayProxyResult = await handler(event, {})
 
     expect(response.statusCode).toBe(400)
-    expect(JSON.parse(response.body)).toEqual(bundleWrap(
-      [badRequest("Missing required field(s) - PharmacyODSCode, TaskID.", FULL_URL_0)]
-    ))
+    expect(JSON.parse(response.body)).toEqual(
+      bundleWrap([badRequest("Missing required field(s) - PharmacyODSCode, TaskID.", FULL_URL_0)])
+    )
   })
 
   it("when dynamo call fails, expect 500 status code and internal server error message", async () => {
@@ -129,17 +132,32 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
     expect(JSON.parse(response.body)).toEqual(bundleWrap([serverError()]))
   })
 
+  it("when data store update times out, expect 408 status code and relevant error message", async () => {
+    mockSend.mockImplementation(() => new Promise(() => {}))
+
+    const event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
+    const eventHandler: Promise<APIGatewayProxyResult> = handler(event, {})
+
+    await jest.advanceTimersByTimeAsync(LAMBDA_TIMEOUT_MS)
+
+    const response = await eventHandler
+    expect(response.statusCode).toBe(408)
+    expect(JSON.parse(response.body)).toEqual(bundleWrap([timeoutResponse()]))
+  })
+
   it("when multiple tasks have missing fields, expect 400 status code and messages indicating missing fields", async () => {
     const body: any = {...requestMultipleMissingFields}
     const event: APIGatewayProxyEvent = generateMockEvent(body)
 
     const response: APIGatewayProxyResult = await handler(event, {})
 
     expect(response.statusCode).toEqual(400)
-    expect(JSON.parse(response.body)).toEqual(bundleWrap([
-      badRequest("Missing required field(s) - PharmacyODSCode, TaskID.", FULL_URL_0),
-      badRequest("Missing required field(s) - PharmacyODSCode.", FULL_URL_1)
-    ]))
+    expect(JSON.parse(response.body)).toEqual(
+      bundleWrap([
+        badRequest("Missing required field(s) - PharmacyODSCode, TaskID.", FULL_URL_0),
+        badRequest("Missing required field(s) - PharmacyODSCode.", FULL_URL_1)
+      ])
+    )
   })
 
   it("when x-request-id header is present but empty, expect 400 status code and relevant error message", async () => {