Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/helpers/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"dependencies": {
"pino": "^9.7.0",
"zod": "^4.1.11"
},
"description": "Common helper utilities for NHS Notify Supplier API",
Expand Down
15 changes: 15 additions & 0 deletions internal/helpers/src/__tests__/logger.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { createLogger } from "../logger";

describe("createLogger", () => {
it("should create a logger with default log level", () => {
const logger = createLogger();

expect(logger.level).toBe("info");
});

it("should create a logger with custom log level", () => {
const logger = createLogger({ logLevel: "debug" });

expect(logger.level).toBe("debug");
});
});
1 change: 1 addition & 0 deletions internal/helpers/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
export * from "./version";
export { default as $Environment } from "./environment";
export * from "./id-ref";
export * from "./logger";
26 changes: 26 additions & 0 deletions internal/helpers/src/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import pino, { Logger } from "pino";

export type LoggerOptions = {
logLevel?: string;
};

/**
* Creates a configured pino logger instance for use across lambdas.
*
* @param options - Optional configuration for the logger
* @param options.logLevel - The log level (defaults to "info")
* @returns A configured pino Logger instance
*/
export function createLogger(options: LoggerOptions = {}): Logger {
const { logLevel = "info" } = options;

return pino({
level: logLevel,
formatters: {
level: (label) => {
return { level: label.toUpperCase() };
},
},
timestamp: () => `,"timestamp":"${new Date(Date.now()).toISOString()}"`,
});
}
11 changes: 5 additions & 6 deletions lambdas/api-handler/src/config/__tests__/deps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ describe("createDependenciesContainer", () => {
jest.clearAllMocks();
jest.resetModules();

// pino
jest.mock("pino", () => ({
__esModule: true,
default: jest.fn(() => ({
jest.mock("@internal/helpers", () => ({
createLogger: jest.fn(() => ({
info: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
debug: jest.fn(),
level: "info",
})),
}));

Expand All @@ -49,7 +48,7 @@ describe("createDependenciesContainer", () => {
// get current mock instances
const { S3Client } = jest.requireMock("@aws-sdk/client-s3");
const { SQSClient } = jest.requireMock("@aws-sdk/client-sqs");
const pinoMock = jest.requireMock("pino");
const { createLogger } = jest.requireMock("@internal/helpers");
const { LetterRepository, MIRepository } = jest.requireMock(
"@internal/datastore",
);
Expand All @@ -63,7 +62,7 @@ describe("createDependenciesContainer", () => {

expect(SQSClient).toHaveBeenCalledTimes(1);

expect(pinoMock.default).toHaveBeenCalledTimes(1);
expect(createLogger).toHaveBeenCalledTimes(1);

expect(LetterRepository).toHaveBeenCalledTimes(1);
const letterRepoCtorArgs = LetterRepository.mock.calls[0];
Expand Down
14 changes: 6 additions & 8 deletions lambdas/api-handler/src/config/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import { S3Client } from "@aws-sdk/client-s3";
import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb";
import { SQSClient } from "@aws-sdk/client-sqs";
import pino from "pino";
import { Logger } from "pino";
import {
DBHealthcheck,
LetterRepository,
MIRepository,
} from "@internal/datastore";
import { createLogger } from "@internal/helpers";
import { EnvVars, envVars } from "./env";

export type Deps = {
Expand All @@ -16,7 +17,7 @@ export type Deps = {
letterRepo: LetterRepository;
miRepo: MIRepository;
dbHealthcheck: DBHealthcheck;
logger: pino.Logger;
logger: Logger;
env: EnvVars;
};

Expand All @@ -26,7 +27,7 @@ function createDocumentClient(): DynamoDBDocumentClient {
}

function createLetterRepository(
log: pino.Logger,
log: Logger,
environment: EnvVars,
): LetterRepository {
const config = {
Expand All @@ -46,10 +47,7 @@ function createDBHealthcheck(environment: EnvVars): DBHealthcheck {
return new DBHealthcheck(createDocumentClient(), config);
}

function createMIRepository(
log: pino.Logger,
environment: EnvVars,
): MIRepository {
function createMIRepository(log: Logger, environment: EnvVars): MIRepository {
const config = {
miTableName: environment.MI_TABLE_NAME,
miTtlHours: environment.MI_TTL_HOURS,
Expand All @@ -59,7 +57,7 @@ function createMIRepository(
}

export function createDependenciesContainer(): Deps {
const log = pino();
const log = createLogger({ logLevel: envVars.PINO_LOG_LEVEL });

return {
s3Client: new S3Client(),
Expand Down
1 change: 1 addition & 0 deletions lambdas/api-handler/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const EnvVarsSchema = z.object({
DOWNLOAD_URL_TTL_SECONDS: z.coerce.number().int(),
MAX_LIMIT: z.coerce.number().int().optional(),
QUEUE_URL: z.coerce.string().optional(),
PINO_LOG_LEVEL: z.coerce.string().optional(),
});

export type EnvVars = z.infer<typeof EnvVarsSchema>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,12 @@ describe("createLetterStatusUpdateHandler", () => {
expect(mockedDeps.letterRepo.updateLetterStatus).toHaveBeenCalledWith(
updateLetterCommands[0],
);
expect(mockedDeps.logger.error).toHaveBeenCalledWith(
{
err: mockError,
messageId: "mid-id1",
correlationId: "correlationId-id1",
messageBody: '{"id":"id1","status":"ACCEPTED","supplierId":"s1"}',
},
"Error processing letter status update",
);
expect(mockedDeps.logger.error).toHaveBeenCalledWith({
description: "Error processing letter status update",
err: mockError,
messageId: "mid-id1",
correlationId: "correlationId-id1",
messageBody: '{"id":"id1","status":"ACCEPTED","supplierId":"s1"}',
});
});
});
19 changes: 14 additions & 5 deletions lambdas/api-handler/src/handlers/get-letter-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,23 @@ export default function createGetLetterDataHandler(
),
);

const presignedUrl = await getLetterDataUrl(
commonIds.value.supplierId,
letterId,
deps,
);

deps.logger.info({
description: "Generated presigned URL",
supplierId: commonIds.value.supplierId,
letterId,
correlationId: commonIds.value.correlationId,
});

return {
statusCode: 303,
headers: {
Location: await getLetterDataUrl(
commonIds.value.supplierId,
letterId,
deps,
),
Location: presignedUrl,
},
body: "",
};
Expand Down
1 change: 1 addition & 0 deletions lambdas/api-handler/src/handlers/get-letter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export default function createGetLetterHandler(
description: "Letter successfully fetched by id",
supplierId: commonIds.value.supplierId,
letterId,
correlationId: commonIds.value.correlationId,
});

return {
Expand Down
1 change: 1 addition & 0 deletions lambdas/api-handler/src/handlers/get-letters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default function createGetLettersHandler(
limitNumber,
status,
lettersCount: letters.length,
correlationId: commonIds.value.correlationId,
});

return {
Expand Down
8 changes: 4 additions & 4 deletions lambdas/api-handler/src/handlers/get-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ export default function createGetStatusHandler(
body: JSON.stringify({ code: 200 }, null, 2),
};
} catch (error) {
deps.logger.error(
{ err: error },
"Status endpoint error, services not available",
);
deps.logger.error({
err: error,
description: "Status endpoint error, services not available",
});
return {
statusCode: 500,
body: JSON.stringify({ code: 500 }, null, 2),
Expand Down
22 changes: 13 additions & 9 deletions lambdas/api-handler/src/handlers/letter-status-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@ export default function createLetterStatusUpdateHandler(
await deps.letterRepo.updateLetterStatus(
mapToUpdateLetter(letterToUpdate),
);
deps.logger.info({
description: "Updated letter status",
letterId: letterToUpdate.id,
messageId: message.messageId,
correlationId: message.messageAttributes.CorrelationId.stringValue,
});
} catch (error) {
deps.logger.error(
{
err: error,
messageId: message.messageId,
correlationId: message.messageAttributes.CorrelationId.stringValue,
messageBody: message.body,
},
"Error processing letter status update",
);
deps.logger.error({
description: "Error processing letter status update",
err: error,
messageId: message.messageId,
correlationId: message.messageAttributes.CorrelationId.stringValue,
messageBody: message.body,
});
}
});

Expand Down
8 changes: 8 additions & 0 deletions lambdas/api-handler/src/handlers/patch-letter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ export default function createPatchLetterHandler(
throw typedError;
}

deps.logger.info({
description: "Received patch letter request",
supplierId: commonIds.value.supplierId,
letterId,
newStatus: patchLetterRequest.data.attributes.status,
correlationId: commonIds.value.correlationId,
});

const updateLetterCommand: UpdateLetterCommand = mapToUpdateCommand(
patchLetterRequest,
commonIds.value.supplierId,
Expand Down
7 changes: 7 additions & 0 deletions lambdas/api-handler/src/handlers/post-letters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ export default function createPostLettersHandler(
throw typedError;
}

deps.logger.info({
description: "Received post letters request",
supplierId: commonIds.value.supplierId,
letterIds: postLettersRequest.data.map((letter) => letter.id),
correlationId: commonIds.value.correlationId,
});

if (postLettersRequest.data.length > maxUpdateItems) {
throw new ValidationError(
ApiErrorDetail.InvalidRequestLettersToUpdate,
Expand Down
6 changes: 6 additions & 0 deletions lambdas/api-handler/src/handlers/post-mi.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { APIGatewayProxyHandler } from "aws-lambda";
import postMIOperation from "../services/mi-operations";

Check warning on line 2 in lambdas/api-handler/src/handlers/post-mi.ts

View workflow job for this annotation

GitHub Actions / Test stage / Linting

Caution: `mi-operations.ts` has a default export `postMI`. This imports `postMI` as `postMIOperation`. Check if you meant to write `import postMI from '../services/mi-operations'` instead
import { ApiErrorDetail } from "../contracts/errors";
import ValidationError from "../errors/validation-error";
import { processError } from "../mappers/error-mapper";
Expand Down Expand Up @@ -53,6 +53,12 @@
deps.miRepo,
);

deps.logger.info({
description: "Posted management information",
supplierId: commonIds.value.supplierId,
correlationId: commonIds.value.correlationId,
});

return {
statusCode: 201,
body: JSON.stringify(result, null, 2),
Expand Down
27 changes: 11 additions & 16 deletions lambdas/api-handler/src/mappers/error-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,38 +72,33 @@ export function logAndMapToApiError(
logger: Logger,
): ApiError {
if (error instanceof ValidationError) {
logger.info(
{ err: error },
`Validation error correlationId=${correlationId}`,
);
logger.info({ description: "Validation error", err: error, correlationId });
return mapToApiError(
ApiErrorCode.InvalidRequest,
error.detail,
correlationId,
);
}
if (error instanceof NotFoundError) {
logger.info(
{ err: error },
`Not found error correlationId=${correlationId}`,
);
logger.info({ description: "Not found error", err: error, correlationId });
return mapToApiError(ApiErrorCode.NotFound, error.detail, correlationId);
}
if (error instanceof Error) {
logger.error(
{ err: error },
`Internal server error correlationId=${correlationId}`,
);
logger.error({
description: "Internal server error",
err: error,
correlationId,
});
return mapToApiError(
ApiErrorCode.InternalServerError,
"Unexpected error",
correlationId,
);
}
logger.error(
{ err: error },
`Internal server error (non-Error thrown) correlationId=${correlationId}`,
);
logger.error({
description: "Internal server error (non-Error thrown)",
correlationId,
});
return mapToApiError(
ApiErrorCode.InternalServerError,
"Unexpected error",
Expand Down
Loading
Loading