diff --git a/packages/distanceSelling/src/distanceSelling.ts b/packages/distanceSelling/src/distanceSelling.ts index 2af385794..ac1c39058 100644 --- a/packages/distanceSelling/src/distanceSelling.ts +++ b/packages/distanceSelling/src/distanceSelling.ts @@ -23,13 +23,13 @@ export class DistanceSelling { this.servicesCache = servicesCache } - async search(searchsetBundle: Bundle) { + async search(searchsetBundle: Bundle, correlationId: string) { const prescriptions: Array = this.isolatePrescriptions(searchsetBundle) const performerReferences: Set = this.getPerformerReferences(prescriptions) const performerOrganisations: Array = this.getPerformerOrganisations( performerReferences, prescriptions ) - await this.processOdsCodes(performerOrganisations) + await this.processOdsCodes(performerOrganisations, correlationId) } isolatePrescriptions(searchsetBundle: Bundle): Array { @@ -60,7 +60,7 @@ export class DistanceSelling { ) } - async processOdsCodes(organisations: Array) { + async processOdsCodes(organisations: Array, correlationId: string) { for (const organisation of organisations) { const odsCode = organisation.identifier![0].value?.toLowerCase() if (odsCode) { @@ -79,16 +79,16 @@ export class DistanceSelling { this.logger.info( `ods code ${odsCode} not found in cache. calling service search.`, {odsCode: odsCode, cacheHit: false} ) - await this.searchOdsCode(odsCode, organisation) + await this.searchOdsCode(odsCode, organisation, correlationId) } } } } - async searchOdsCode(odsCode: string, organisation: Organization) { + async searchOdsCode(odsCode: string, organisation: Organization, correlationId: string) { let url: URL | undefined try { - url = await this.client.searchService(odsCode, this.logger) + url = await this.client.searchService(odsCode, correlationId, this.logger) } catch { this.logger.warn(`call to service search unsuccessful for odsCode ${odsCode}`, {odsCode: odsCode}) return diff --git a/packages/distanceSelling/tests/distanceSelling.test.ts b/packages/distanceSelling/tests/distanceSelling.test.ts index 658958472..253b6802a 100644 --- a/packages/distanceSelling/tests/distanceSelling.test.ts +++ b/packages/distanceSelling/tests/distanceSelling.test.ts @@ -13,6 +13,7 @@ import {Logger} from "@aws-lambda-powertools/logger" const mock = new MockAdapter(axios) const mockBundleString = JSON.stringify(mockInteractionResponseBody) +const dummyCorrelationId = "corr-id-123" describe("ServiceSearch tests", function () { const logger = new Logger({serviceName: "distanceSelling"}) @@ -213,7 +214,7 @@ describe("ServiceSearch tests", function () { const expectedTelecom: ContactPoint = {use: "work", system: "url", value: "www.pharmacy2u.co.uk"} - await distanceSelling.processOdsCodes(organisations) + await distanceSelling.processOdsCodes(organisations, dummyCorrelationId) const organisation: Organization = organisations[0] // The address is removed, since we're a distance selling pharmacy @@ -226,7 +227,7 @@ describe("ServiceSearch tests", function () { const distanceSellingWithCache = new DistanceSelling(cache, logger) const searchsetBundle = JSON.parse(mockBundleString) as Bundle - await distanceSellingWithCache.search(searchsetBundle) + await distanceSellingWithCache.search(searchsetBundle, dummyCorrelationId) expect(mock.history.get.length).toEqual(0) }) @@ -259,7 +260,7 @@ describe("ServiceSearch tests", function () { const organisation = distanceSelling.getPerformerOrganisations(performerReferences, prescriptions)[0] const odsCode = "flm49" - await distanceSelling.searchOdsCode(odsCode, organisation) + await distanceSelling.searchOdsCode(odsCode, organisation, dummyCorrelationId) expect(odsCode in servicesCache).toBeTruthy() expect(servicesCache[odsCode]).toEqual("www.pharmacy2u.co.uk") @@ -276,7 +277,7 @@ describe("ServiceSearch tests", function () { const organisation = distanceSelling.getPerformerOrganisations(performerReferences, prescriptions)[0] const odsCode = "flm49" - await distanceSelling.searchOdsCode(odsCode, organisation) + await distanceSelling.searchOdsCode(odsCode, organisation, dummyCorrelationId) expect(odsCode in servicesCache).toBeTruthy() expect(servicesCache[odsCode]).toEqual(undefined) @@ -293,7 +294,7 @@ describe("ServiceSearch tests", function () { const organisation = distanceSelling.getPerformerOrganisations(performerReferences, prescriptions)[0] const odsCode = "flm49" - await distanceSelling.searchOdsCode(odsCode, organisation) + await distanceSelling.searchOdsCode(odsCode, organisation, dummyCorrelationId) expect(odsCode in servicesCache).toBeFalsy() }) @@ -323,7 +324,7 @@ describe("ServiceSearch tests", function () { const organisations = distanceSelling.getPerformerOrganisations(performerReferences, prescriptions) // Run the cache‐hit branch - await distanceSelling.processOdsCodes(organisations) + await distanceSelling.processOdsCodes(organisations, dummyCorrelationId) organisations.forEach((org) => { expect(org.address).toBeUndefined() diff --git a/packages/getMyPrescriptions/src/getMyPrescriptions.ts b/packages/getMyPrescriptions/src/getMyPrescriptions.ts index 4162284a8..be5e7a422 100644 --- a/packages/getMyPrescriptions/src/getMyPrescriptions.ts +++ b/packages/getMyPrescriptions/src/getMyPrescriptions.ts @@ -84,6 +84,7 @@ async function eventHandler( const traceIDs: TraceIDs = logTraceIds(headers) const spineClient = params.spineClient const applicationName = headers["nhsd-application-name"] ?? "unknown" + const correlationId = headers["nhsd-correlation-id"] ?? crypto.randomUUID() checkSpineCertificateConfiguration(spineClient) await handleTestCaseIfApplicable(params, headers) @@ -98,7 +99,7 @@ async function eventHandler( const distanceSelling = new DistanceSelling(servicesCache, logger) const distanceSellingBundle = structuredClone(searchsetBundle) - const distanceSellingCallout = distanceSelling.search(distanceSellingBundle) + const distanceSellingCallout = distanceSelling.search(distanceSellingBundle, correlationId) const distanceSellingResponse = await jobWithTimeout(params.serviceSearchTimeoutMs, distanceSellingCallout) if (hasTimedOut(distanceSellingResponse)) { diff --git a/packages/serviceSearchClient/src/live-serviceSearch-client.ts b/packages/serviceSearchClient/src/live-serviceSearch-client.ts index 439403548..3c138d6ae 100644 --- a/packages/serviceSearchClient/src/live-serviceSearch-client.ts +++ b/packages/serviceSearchClient/src/live-serviceSearch-client.ts @@ -7,7 +7,8 @@ import {handleUrl} from "./handleUrl" import {ServiceSearchClient} from "./serviceSearch-client" // timeout in ms to wait for response from serviceSearch to avoid lambda timeout -const SERVICE_SEARCH_TIMEOUT = 45000 +// each call is retried up to 3 times so total wait time could be up to 4x this value +const SERVICE_SEARCH_TIMEOUT = 1000 // 1 second const DISTANCE_SELLING = "DistanceSelling" type Service = { @@ -66,7 +67,12 @@ export function getServiceSearchEndpoint(logger: Logger | null = null): string { export class LiveServiceSearchClient implements ServiceSearchClient { private readonly axiosInstance: AxiosInstance private readonly logger: Logger - private readonly outboundHeaders: {"apikey"?: string, "Subscription-Key"?: string} + private readonly outboundHeaders: { + "apikey"?: string, + "Subscription-Key"?: string, + "x-request-id"?: string, + "x-correlation-id"?: string + } constructor(logger: Logger) { this.logger = logger @@ -157,7 +163,7 @@ export class LiveServiceSearchClient implements ServiceSearchClient { } } - async searchService(odsCode: string): Promise { + async searchService(odsCode: string, correlationId: string): Promise { try { // Load API key if not set in environment (secrets layer is failing to load v3 key) const apiVsn = getServiceSearchVersion(this.logger) @@ -165,11 +171,20 @@ export class LiveServiceSearchClient implements ServiceSearchClient { this.logger.info("API key not in environment, attempting to load from Secrets Manager") this.outboundHeaders.apikey = await this.loadApiKeyFromSecretsManager() } + this.outboundHeaders["x-correlation-id"] = correlationId + const xRequestId = crypto.randomUUID() + this.outboundHeaders["x-request-id"] = xRequestId const address = getServiceSearchEndpoint(this.logger) const queryParams = {...SERVICE_SEARCH_BASE_QUERY_PARAMS, search: odsCode} - this.logger.info(`making request to ${address} with ods code ${odsCode}`, {odsCode: odsCode}) + this.logger.info(`making request to ${address} with ods code ${odsCode}`, { + odsCode: odsCode, + requestHeaders: { + "x-request-id": xRequestId, + "x-correlation-id": correlationId + } + }) const response = await this.axiosInstance.get(address, { headers: this.outboundHeaders, params: queryParams, diff --git a/packages/serviceSearchClient/src/serviceSearch-client.ts b/packages/serviceSearchClient/src/serviceSearch-client.ts index c67cdd145..9a7723283 100644 --- a/packages/serviceSearchClient/src/serviceSearch-client.ts +++ b/packages/serviceSearchClient/src/serviceSearch-client.ts @@ -2,7 +2,7 @@ import {Logger} from "@aws-lambda-powertools/logger" import {LiveServiceSearchClient} from "./live-serviceSearch-client" export interface ServiceSearchClient { - searchService(odsCode: string, logger: Logger): Promise + searchService(odsCode: string, correlationId: string, logger: Logger): Promise } export class SandboxServiceSearchClient implements ServiceSearchClient { diff --git a/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts b/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts index cc17efdc2..980dcf881 100644 --- a/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts +++ b/packages/serviceSearchClient/tests/live-serviceSearch-client.test.ts @@ -10,6 +10,7 @@ const mock = new MockAdapter(axios) process.env.TargetServiceSearchServer = "live" process.env.ServiceSearchApiKey = "test-key" const serviceSearchUrl = "https://live/service-search" +const dummyCorrelationId = "corr-id-123" interface ServiceSearchTestData { scenarioDescription: string @@ -140,7 +141,7 @@ describe("live serviceSearch client", () => { }) const errSpy = jest.spyOn(Logger.prototype, "error") - await expect(client.searchService("code123")).rejects.toThrow("generic fail") + await expect(client.searchService("code123", dummyCorrelationId)).rejects.toThrow("generic fail") expect(errSpy).toHaveBeenCalledWith( "general error", {error: expect.any(Error)} ) @@ -166,7 +167,7 @@ describe("live serviceSearch client", () => { jest.spyOn(client["axiosInstance"], "get").mockRejectedValue(axiosErr) const errSpy = jest.spyOn(Logger.prototype, "error") - await expect(client.searchService("x")).rejects.toBe(axiosErr) + await expect(client.searchService("x", dummyCorrelationId)).rejects.toBe(axiosErr) expect(errSpy).toHaveBeenCalledWith( "error in response from serviceSearch", expect.objectContaining({ @@ -189,7 +190,7 @@ describe("live serviceSearch client", () => { jest.spyOn(client["axiosInstance"], "get").mockRejectedValue(axiosErr) const errSpy = jest.spyOn(Logger.prototype, "error") - await expect(client.searchService("y")).rejects.toBe(axiosErr) + await expect(client.searchService("y", dummyCorrelationId)).rejects.toBe(axiosErr) expect(errSpy).toHaveBeenCalledWith( "error in request to serviceSearch", {error: axiosErr} ) @@ -208,12 +209,34 @@ describe("live serviceSearch client", () => { jest.spyOn(client["axiosInstance"], "get").mockRejectedValue(axiosErr) const errSpy = jest.spyOn(Logger.prototype, "error") - await expect(client.searchService("y")).rejects.toBe(axiosErr) + await expect(client.searchService("y", dummyCorrelationId)).rejects.toBe(axiosErr) expect(errSpy).toHaveBeenCalledWith( "general error calling serviceSearch", {error: axiosErr} ) }) + test("passes correlation ID and x-request-id header in request", async () => { + const getSpy = jest.spyOn(client["axiosInstance"], "get").mockResolvedValue({ + data: {value: []}, + status: 200, + statusText: "OK", + headers: {}, + config: { + headers: new axios.AxiosHeaders()} + } satisfies AxiosResponse) + + await client.searchService("corr-test", dummyCorrelationId) + + expect(getSpy).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + headers: expect.objectContaining({ + "x-correlation-id": dummyCorrelationId, + "x-request-id": expect.any(String) + }) + }) + ) + }) describe("integration scenarios", () => { const validUrlData: ServiceSearchData = { value: [ @@ -246,13 +269,13 @@ describe("live serviceSearch client", () => { test.each(scenarios)("$scenarioDescription", async ({serviceSearchData, expected}) => { mock.onGet(serviceSearchUrl).reply(200, serviceSearchData) - const result = await client.searchService("z") + const result = await client.searchService("z", dummyCorrelationId) expect(result).toEqual(expected) }) test("gzip header handled correctly", async () => { mock.onGet(serviceSearchUrl).reply(200, validUrlData, {"Content-Encoding": "gzip"}) - const result = await client.searchService("z") + const result = await client.searchService("z", dummyCorrelationId) expect(result).toEqual(new URL(validUrlData.value[0].URL)) }) @@ -261,19 +284,19 @@ describe("live serviceSearch client", () => { .onGet(serviceSearchUrl).replyOnce(500) .onGet(serviceSearchUrl).replyOnce(500) .onGet(serviceSearchUrl).reply(200, validUrlData) - const result = await client.searchService("z") + const result = await client.searchService("z", dummyCorrelationId) expect(result).toEqual(new URL(validUrlData.value[0].URL)) }) test("fails after exceeding retries", async () => { mock.onGet(serviceSearchUrl).reply(500) - await expect(client.searchService("z")).rejects.toThrow("Request failed with status code 500") + await expect(client.searchService("z", dummyCorrelationId)).rejects.toThrow("Request failed with status code 500") }) test("logs duration in info on success and failure", async () => { const infoSpy = jest.spyOn(Logger.prototype, "info") mock.onGet(serviceSearchUrl).networkError() - await expect(client.searchService("z")).rejects.toThrow("Network Error") + await expect(client.searchService("z", dummyCorrelationId)).rejects.toThrow("Network Error") expect(infoSpy).toHaveBeenCalledWith( "serviceSearch request duration", {serviceSearch_duration: expect.any(Number)} @@ -284,7 +307,7 @@ describe("live serviceSearch client", () => { mock.onGet(serviceSearchUrl).reply(200, {value: [{URL: null, OrganisationSubType: "DistanceSelling"}]}) const warnSpy = jest.spyOn(Logger.prototype, "warn") const errorSpy = jest.spyOn(Logger.prototype, "error") - const result = await client.searchService("none") + const result = await client.searchService("none", dummyCorrelationId) expect(result).toBeUndefined() expect(warnSpy).toHaveBeenCalledWith( "ods code none has no URL but is of type DistanceSelling", {odsCode: "none"} @@ -322,7 +345,8 @@ describe("live serviceSearch client", () => { const errSpy = jest.spyOn(Logger.prototype, "error") // invoking searchService should end up throwing the generic interceptor Error - await expect(client.searchService("Z123")).rejects.toThrow("Axios error in serviceSearch request") + await expect(client.searchService("Z123", dummyCorrelationId)) + .rejects.toThrow("Axios error in serviceSearch request") expect(errSpy).toHaveBeenCalledWith( "Axios error in serviceSearch request",