Skip to content

Commit 2761b90

Browse files
francisco-videira-nhsfrancisco-videira-nhs
committed
Add correlationId as error.id
1 parent 5fa01fa commit 2761b90

File tree

11 files changed

+167
-92
lines changed

11 files changed

+167
-92
lines changed

‎infrastructure/terraform/components/api/locals.tf‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@ locals {
1818
LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
1919
LETTER_TTL_HOURS = 24,
2020
SUPPLIER_ID_HEADER = "nhsd-supplier-id"
21+
SUPPLIER_ID_HEADER = "nhsd-correlation-id"
2122
}
2223
}

‎lambdas/api-handler/src/config/lambda-config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
interface LambdaConfig {
22
SUPPLIER_ID_HEADER: string;
3+
APIM_CORRELATION_HEADER: string;
34
}
45

56
export const lambdaConfig: LambdaConfig = {
67
SUPPLIER_ID_HEADER: getEnv("SUPPLIER_ID_HEADER")!,
8+
APIM_CORRELATION_HEADER: getEnv("APIM_CORRELATION_HEADER")!
79
};
810

911
function getEnv(name: string, required = true): string | undefined {

‎lambdas/api-handler/src/contracts/errors.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { randomUUID } from 'crypto';
2-
31
export interface ApiError {
42
id: string;
53
code: ApiErrorCode;
@@ -36,17 +34,19 @@ export enum ApiErrorDetail {
3634
InvalidRequestBody = 'The request body is invalid',
3735
InvalidRequestLimitNotANumber = 'The limit parameter is not a number',
3836
InvalidRequestLimitNotInRange = 'The limit parameter must be a positive number not greater than %s',
39-
InvalidRequestLimitOnly = "Only 'limit' query parameter is supported"
37+
InvalidRequestLimitOnly = "Only 'limit' query parameter is supported",
38+
InvalidRequestNoRequestId = 'The request does not contain a request id'
4039
}
4140

4241
export function buildApiError(params: {
42+
id: string
4343
code: ApiErrorCode;
4444
status: ApiErrorStatus;
4545
title: ApiErrorTitle;
4646
detail: ApiErrorDetail | string;
4747
}): ApiError {
4848
return {
49-
id: randomUUID(), // TODO CCM-11188: correlation ID?
49+
id: params.id,
5050
code: params.code,
5151
links: { about: 'https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier' },
5252
status: params.status,

‎lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ jest.mock('../../mappers/error-mapper');
1212
const mockedMapErrorToResponse = jest.mocked(mapErrorToResponse);
1313
const expectedErrorResponse: APIGatewayProxyResult = {
1414
statusCode: 400,
15-
body: "Error"
15+
body: 'Error'
1616
};
1717
mockedMapErrorToResponse.mockReturnValue(expectedErrorResponse);
1818

1919
jest.mock('../../services/letter-operations');
2020

21-
jest.mock("../../config/lambda-config", () => ({
21+
jest.mock('../../config/lambda-config', () => ({
2222
lambdaConfig: {
23-
SUPPLIER_ID_HEADER: "nhsd-supplier-id"
23+
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
24+
APIM_CORRELATION_HEADER: 'nhsd-correlation-id'
2425
}
2526
}));
2627

@@ -69,7 +70,8 @@ describe('API Lambda handler', () => {
6970
},
7071
]);
7172

72-
const event = makeApiGwEvent({path: '/letters', headers: {'nhsd-supplier-id': 'supplier1'}});
73+
const event = makeApiGwEvent({path: '/letters',
74+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}});
7375
const context = mockDeep<Context>();
7476
const callback = jest.fn();
7577
const result = await getLetters(event, context, callback);
@@ -104,69 +106,74 @@ describe('API Lambda handler', () => {
104106
const event = makeApiGwEvent({
105107
path: "/letters",
106108
queryStringParameters: { limit: "1%" },
107-
headers: {'nhsd-supplier-id': 'supplier1'}});
109+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
110+
});
108111

109112
const context = mockDeep<Context>();
110113
const callback = jest.fn();
111114
const result = await getLetters(event, context, callback);
112115

113-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotANumber));
116+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotANumber), 'correlationId');
114117
expect(result).toEqual(expectedErrorResponse);
115118
});
116119

117120
it("returns 400 if the limit parameter is negative", async () => {
118121
const event = makeApiGwEvent({
119122
path: "/letters",
120123
queryStringParameters: { limit: "-1" },
121-
headers: {'nhsd-supplier-id': 'supplier1'}});
124+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
125+
});
122126

123127
const context = mockDeep<Context>();
124128
const callback = jest.fn();
125129
const result = await getLetters(event, context, callback);
126130

127-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }));
131+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(
132+
new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId');
128133
expect(result).toEqual(expectedErrorResponse);
129134
});
130135

131136
it("returns 400 if the limit parameter is zero", async () => {
132137
const event = makeApiGwEvent({
133138
path: "/letters",
134139
queryStringParameters: { limit: "0" },
135-
headers: {'nhsd-supplier-id': 'supplier1'}
140+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
136141
});
137142
const context = mockDeep<Context>();
138143
const callback = jest.fn();
139144
const result = await getLetters(event, context, callback);
140145

141-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }));
146+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(
147+
new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId');
142148
expect(result).toEqual(expectedErrorResponse);
143149
});
144150

145151
it("returns 400 if the limit parameter is higher than max limit", async () => {
146152
const event = makeApiGwEvent({
147153
path: "/letters",
148154
queryStringParameters: { limit: "2501" },
149-
headers: {'nhsd-supplier-id': 'supplier1'}
155+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
150156
});
151157
const context = mockDeep<Context>();
152158
const callback = jest.fn();
153159
const result = await getLetters(event, context, callback);
154160

155-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }));
161+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(
162+
new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId');
156163
expect(result).toEqual(expectedErrorResponse);
157164
});
158165

159166
it("returns 400 if unknown parameters are present", async () => {
160167
const event = makeApiGwEvent({
161168
path: "/letters",
162169
queryStringParameters: { max: "2000" },
163-
headers: {'nhsd-supplier-id': 'supplier1'}
170+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
164171
});
165172
const context = mockDeep<Context>();
166173
const callback = jest.fn();
167174
const result = await getLetters(event, context, callback);
168175

169-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitOnly));
176+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitOnly), 'correlationId');
170177
expect(result).toEqual(expectedErrorResponse);
171178
});
172179

@@ -176,17 +183,21 @@ describe('API Lambda handler', () => {
176183
const callback = jest.fn();
177184
const result = await getLetters(event, context, callback);
178185

179-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingSupplierId));
186+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error('The request headers are empty'), undefined);
180187
expect(result).toEqual(expectedErrorResponse);
181188
});
182189

183-
it('returns 400 for missing supplier ID (undefined headers)', async () => {
184-
const event = makeApiGwEvent({ path: "/letters", headers: undefined });
190+
it("returns 500 if correlation id not provided in request", async () => {
191+
const event = makeApiGwEvent({
192+
path: "/letters",
193+
queryStringParameters: { limit: "2000" },
194+
headers: {'nhsd-supplier-id': 'supplier1'}
195+
});
185196
const context = mockDeep<Context>();
186197
const callback = jest.fn();
187198
const result = await getLetters(event, context, callback);
188199

189-
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingSupplierId));
200+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error("The request headers don't contain the APIM correlation id"), undefined);
190201
expect(result).toEqual(expectedErrorResponse);
191202
});
192203
});

‎lambdas/api-handler/src/handlers/__tests__/patch-letters.test.ts‎

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@ import { makeApiGwEvent } from './utils/test-utils';
55
import * as letterService from '../../services/letter-operations';
66
import { LetterApiDocument, LetterApiStatus } from '../../contracts/letter-api';
77
import { mapErrorToResponse } from '../../mappers/error-mapper';
8+
import { ValidationError } from '../../errors';
9+
import * as errors from '../../contracts/errors';
810

911
jest.mock('../../services/letter-operations');
1012
jest.mock('../../mappers/error-mapper');
1113

1214
jest.mock("../../config/lambda-config", () => ({
1315
lambdaConfig: {
14-
SUPPLIER_ID_HEADER: "nhsd-supplier-id"
16+
SUPPLIER_ID_HEADER: "nhsd-supplier-id",
17+
APIM_CORRELATION_HEADER: 'nhsd-correlation-id'
1518
}
1619
}));
1720

@@ -53,7 +56,8 @@ describe('patchLetters API Handler', () => {
5356
path: '/letters/id1',
5457
body: requestBody,
5558
pathParameters: {id: "id1"},
56-
headers: {'nhsd-supplier-id': 'supplier1'}});
59+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
60+
});
5761
const context = mockDeep<Context>();
5862
const callback = jest.fn();
5963

@@ -71,54 +75,63 @@ describe('patchLetters API Handler', () => {
7175
const event = makeApiGwEvent({
7276
path: '/letters/id1',
7377
pathParameters: {id: "id1"},
74-
headers: {'nhsd-supplier-id': 'supplier1'}});
78+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
79+
});
7580
const context = mockDeep<Context>();
7681
const callback = jest.fn();
7782

7883
const result = await patchLetters(event, context, callback);
7984

85+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingBody), 'correlationId');
8086
expect(result).toEqual(expectedErrorResponse);
8187
});
8288

8389
it('returns error response when path parameter letterId is not found', async () => {
8490
const event = makeApiGwEvent({
8591
path: '/letters/',
8692
body: requestBody,
87-
headers: {'nhsd-supplier-id': 'supplier1'}});
93+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
94+
});
8895
const context = mockDeep<Context>();
8996
const callback = jest.fn();
9097
const result = await patchLetters(event, context, callback);
9198

99+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingLetterIdPathParameter), 'correlationId');
92100
expect(result).toEqual(expectedErrorResponse);
93101
});
94102

95103
it('returns error response when error is thrown by service', async () => {
96-
mockedPatchLetterStatus.mockRejectedValue(new Error('Service error'));
104+
const error = new Error('Service error');
105+
mockedPatchLetterStatus.mockRejectedValue(error);
97106

98107
const event = makeApiGwEvent({
99108
path: '/letters/id1',
100109
body: requestBody,
101110
pathParameters: {id: "id1"},
102-
headers: {'nhsd-supplier-id': 'supplier1'}
111+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
103112
});
104113
const context = mockDeep<Context>();
105114
const callback = jest.fn();
106115

107116
const result = await patchLetters(event, context, callback);
108117

118+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(error, 'correlationId');
109119
expect(result).toEqual(expectedErrorResponse);
110120
});
111121

112-
it('returns error when nhsd-supplier-id is missing', async () => {
122+
it('returns error when supplier id is missing', async () => {
113123
const event = makeApiGwEvent({
114124
path: '/letters/id1',
115125
body: requestBody,
116-
pathParameters: {id: "id1"}});
126+
pathParameters: {id: "id1"},
127+
headers: {'nhsd-correlation-id': 'correlationId'}
128+
});
117129
const context = mockDeep<Context>();
118130
const callback = jest.fn();
119131

120132
const result = await patchLetters(event, context, callback);
121133

134+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingSupplierId), 'correlationId');
122135
expect(result).toEqual(expectedErrorResponse);
123136
});
124137

@@ -127,12 +140,14 @@ describe('patchLetters API Handler', () => {
127140
path: '/letters/id1',
128141
body: '{test: "test"}',
129142
pathParameters: {id: "id1"},
130-
headers: {'nhsd-supplier-id': 'supplier1'}});
143+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
144+
});
131145
const context = mockDeep<Context>();
132146
const callback = jest.fn();
133147

134148
const result = await patchLetters(event, context, callback);
135149

150+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId');
136151
expect(result).toEqual(expectedErrorResponse);
137152
});
138153

@@ -141,32 +156,69 @@ describe('patchLetters API Handler', () => {
141156
path: '/letters/id1',
142157
body: '{#invalidJSON',
143158
pathParameters: {id: "id1"},
144-
headers: {'nhsd-supplier-id': 'supplier1'}});
159+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
160+
});
145161
const context = mockDeep<Context>();
146162
const callback = jest.fn();
147163

148164
const result = await patchLetters(event, context, callback);
149165

166+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId');
150167
expect(result).toEqual(expectedErrorResponse);
151168
});
152169

153170
it('returns error if unexpected error is thrown', async () => {
154171
const event = makeApiGwEvent({
155172
path: '/letters/id1',
156-
body: '{#invalidJSON',
173+
body: 'somebody',
157174
pathParameters: {id: "id1"},
158-
headers: {'nhsd-supplier-id': 'supplier1'}});
175+
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
176+
});
159177
const context = mockDeep<Context>();
160178
const callback = jest.fn();
161179

180+
const error = "Unexpected error";
162181
const spy = jest.spyOn(JSON, "parse").mockImplementation(() => {
163-
throw "Unexpected error";
182+
throw error;
164183
});
165184

166185
const result = await patchLetters(event, context, callback);
167186

187+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(error, 'correlationId');
168188
expect(result).toEqual(expectedErrorResponse);
169189

170190
spy.mockRestore();
171191
});
192+
193+
it("returns error if correlation id not provided in request", async () => {
194+
const event = makeApiGwEvent({
195+
path: '/letters/id1',
196+
body: requestBody,
197+
pathParameters: {id: "id1"},
198+
headers: {'nhsd-supplier-id': 'supplier1'}
199+
});
200+
const context = mockDeep<Context>();
201+
const callback = jest.fn();
202+
203+
const result = await patchLetters(event, context, callback);
204+
205+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error("The request headers don't contain the APIM correlation id"), undefined);
206+
expect(result).toEqual(expectedErrorResponse);
207+
});
208+
209+
it('returns 400 for missing supplier ID (empty headers)', async () => {
210+
const event = makeApiGwEvent({
211+
path: '/letters/id1',
212+
body: requestBody,
213+
pathParameters: {id: "id1"},
214+
headers: {}
215+
});
216+
const context = mockDeep<Context>();
217+
const callback = jest.fn();
218+
219+
const result = await patchLetters(event, context, callback);
220+
221+
expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error('The request headers are empty'), undefined);
222+
expect(result).toEqual(expectedErrorResponse);
223+
});
172224
});

0 commit comments

Comments
 (0)