From ee9c37306b15bf2769b0e1a7d75f983495c99447 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Fri, 17 Apr 2026 17:15:30 +0100 Subject: [PATCH 01/13] ALB/webhook uses https for mTLS and non mTLS - remove http endpoint --- .../terraform/components/callbacks/locals.tf | 2 +- .../callbacks/module_mock_webhook_alb_mtls.tf | 27 +------------------ 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/infrastructure/terraform/components/callbacks/locals.tf b/infrastructure/terraform/components/callbacks/locals.tf index 64bd622..d80b5b7 100644 --- a/infrastructure/terraform/components/callbacks/locals.tf +++ b/infrastructure/terraform/components/callbacks/locals.tf @@ -21,7 +21,7 @@ locals { targets = [ for target in try(client.targets, []) : merge(target, { - invocationEndpoint = try(target.delivery.mtls.enabled, false) ? "https://${aws_lb.mock_webhook_mtls[0].dns_name}/${target.targetId}" : "http://${aws_lb.mock_webhook_mtls[0].dns_name}/${target.targetId}" + invocationEndpoint = "https://${aws_lb.mock_webhook_mtls[0].dns_name}/${target.targetId}" apiKey = merge(target.apiKey, { headerValue = random_password.mock_webhook_api_key[0].result }) }) ] diff --git a/infrastructure/terraform/components/callbacks/module_mock_webhook_alb_mtls.tf b/infrastructure/terraform/components/callbacks/module_mock_webhook_alb_mtls.tf index 7e7badf..eb8b677 100644 --- a/infrastructure/terraform/components/callbacks/module_mock_webhook_alb_mtls.tf +++ b/infrastructure/terraform/components/callbacks/module_mock_webhook_alb_mtls.tf @@ -19,18 +19,7 @@ resource "aws_vpc_security_group_ingress_rule" "mock_webhook_alb_https" { from_port = 443 to_port = 443 ip_protocol = "tcp" - description = "Allow HTTPS Client Lambda to reach mock webhook via mTLS" - tags = local.default_tags -} - -resource "aws_vpc_security_group_ingress_rule" "mock_webhook_alb_http" { - count = var.deploy_mock_clients ? 1 : 0 - security_group_id = aws_security_group.mock_webhook_alb[0].id - referenced_security_group_id = aws_security_group.https_client_lambda.id - from_port = 80 - to_port = 80 - ip_protocol = "tcp" - description = "Allow HTTPS Client Lambda to reach mock webhook without mTLS" + description = "Allow HTTPS Client Lambda to reach mock webhook (mTLS and non-mTLS)" tags = local.default_tags } @@ -102,17 +91,3 @@ resource "aws_lb_listener" "mock_webhook_mtls" { tags = local.default_tags } - -resource "aws_lb_listener" "mock_webhook_http" { - count = var.deploy_mock_clients ? 1 : 0 - load_balancer_arn = aws_lb.mock_webhook_mtls[0].arn - port = 80 - protocol = "HTTP" - - default_action { - type = "forward" - target_group_arn = aws_lb_target_group.mock_webhook_mtls[0].arn - } - - tags = local.default_tags -} From 1fbb3f3c3e32e5a44941b30faf584f9280fb11b2 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 10:16:57 +0100 Subject: [PATCH 02/13] Log thrown errors in http-client-lambda --- lambdas/https-client-lambda/src/handler.ts | 6 ++++- scripts/tests/integration-debug.sh | 30 ++++++++++++++++++---- scripts/tests/test.mk | 2 +- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lambdas/https-client-lambda/src/handler.ts b/lambdas/https-client-lambda/src/handler.ts index e30d550..e83f635 100644 --- a/lambdas/https-client-lambda/src/handler.ts +++ b/lambdas/https-client-lambda/src/handler.ts @@ -211,7 +211,11 @@ export async function processRecords( try { await processRecord(record, redis); return null; - } catch { + } catch (error) { + logger.error("Failed to process record", { + messageId: record.messageId, + err: error, + }); return { itemIdentifier: record.messageId }; } }, diff --git a/scripts/tests/integration-debug.sh b/scripts/tests/integration-debug.sh index ac9fb90..840e480 100755 --- a/scripts/tests/integration-debug.sh +++ b/scripts/tests/integration-debug.sh @@ -12,10 +12,11 @@ set -euo pipefail # Actions: # queue-status Show SQS queue message counts # queue-peek Peek one message from each SQS queue -# tail-transform Tail client-transform-filter lambda logs -# tail-webhook Tail mock-webhook lambda logs -# tail-pipe Tail EventBridge pipe log group -# pipe-state Show EventBridge pipe state and recent metrics +# tail-transform Tail client-transform-filter lambda logs +# tail-https-client Tail https-client lambda logs (requires CLIENT_ID) +# tail-webhook Tail mock-webhook lambda logs +# tail-pipe Tail EventBridge pipe log group +# pipe-state Show EventBridge pipe state and recent metrics # # Required: # ENVIRONMENT @@ -160,6 +161,22 @@ action_tail_transform() { "${filter_args[@]}" } +action_tail_https_client() { + require_client_id + local -a filter_args=() + mapfile -t filter_args < <(log_filter_args) + + print_section "HTTPS Client Lambda Logs" + aws logs tail \ + "/aws/lambda/${PREFIX}-https-client-${CLIENT_ID}" \ + --region "$REGION" \ + --profile "$AWS_PROFILE" \ + --since 30m \ + --follow \ + --format short \ + "${filter_args[@]}" +} + action_tail_webhook() { local -a filter_args=() mapfile -t filter_args < <(log_filter_args) @@ -266,6 +283,9 @@ case "$ACTION" in tail-transform) action_tail_transform ;; + tail-https-client) + action_tail_https_client + ;; tail-webhook) action_tail_webhook ;; @@ -277,7 +297,7 @@ case "$ACTION" in ;; *) echo "Unknown action: $ACTION" >&2 - echo "Actions: queue-status, queue-peek, tail-transform, tail-webhook, tail-pipe, pipe-state" >&2 + echo "Actions: queue-status, queue-peek, tail-transform, tail-https-client, tail-webhook, tail-pipe, pipe-state" >&2 exit 1 ;; esac diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index a94a5af..2bb7074 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -38,7 +38,7 @@ test-integration-local: # Run integration tests locally against a remoptely depl test-integration-debug: # Debug a live environment - inspect queues, tail logs, check pipe state (requires ENVIRONMENT, AWS_PROFILE, ACTION) @Testing make _test name="integration-debug" ACTION="$(or $(ACTION),$(word 2,$(MAKECMDGOALS)))" -queue-status queue-peek tail-transform tail-webhook tail-pipe pipe-state: +queue-status queue-peek tail-transform tail-https-client tail-webhook tail-pipe pipe-state: @: test-load: # Run all your load tests @Testing From 675b6e591ae75d43605b1c142d1115d83ab5317c Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 10:17:34 +0100 Subject: [PATCH 03/13] Update int test debug script --- scripts/tests/integration-debug.sh | 42 +++++++++++++----------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/scripts/tests/integration-debug.sh b/scripts/tests/integration-debug.sh index 840e480..a4ebbd6 100755 --- a/scripts/tests/integration-debug.sh +++ b/scripts/tests/integration-debug.sh @@ -23,6 +23,9 @@ set -euo pipefail # AWS_PROFILE # ACTION # +# Required for queue-status, queue-peek: +# CLIENT_ID Client ID (e.g. mock-client-1) +# # Optional: # LOG_FILTER CloudWatch Logs filter pattern / text # AWS_REGION (default: eu-west-2) @@ -46,7 +49,7 @@ fi REGION="${AWS_REGION:-eu-west-2}" LOG_FILTER="${LOG_FILTER:-}" -SUBSCRIPTION_FIXTURE_PATH="${SUBSCRIPTION_FIXTURE_PATH:-tests/integration/fixtures/subscriptions/mock-client-1.json}" +CLIENT_ID="${CLIENT_ID:-}" if ! aws sts get-caller-identity --profile "$AWS_PROFILE" >/dev/null 2>&1; then echo "No active AWS SSO session for profile '$AWS_PROFILE'. Running aws sso login..." @@ -70,21 +73,12 @@ queue_url() { echo "https://sqs.${REGION}.amazonaws.com/${ACCOUNT_ID}/${queue_name}" } -target_dlq_queue_name() { - local target_id - - if [ ! -f "$SUBSCRIPTION_FIXTURE_PATH" ]; then - echo "Error: subscription fixture not found: $SUBSCRIPTION_FIXTURE_PATH" >&2 - exit 1 - fi - - target_id="$(jq -r '.targets[0].targetId // empty' "$SUBSCRIPTION_FIXTURE_PATH")" - if [ -z "$target_id" ]; then - echo "Error: unable to read targets[0].targetId from $SUBSCRIPTION_FIXTURE_PATH" >&2 +require_client_id() { + if [ -z "$CLIENT_ID" ]; then + echo "Error: CLIENT_ID must be set for this action." >&2 + echo "Example: CLIENT_ID=mock-client-1 ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=queue-status" >&2 exit 1 fi - - echo "${PREFIX}-${target_id}-dlq-queue" } show_queue_counts() { @@ -102,9 +96,11 @@ show_queue_counts() { } action_queue_status() { - show_queue_counts "Mock Target DLQ - Queue Message Counts" "$(target_dlq_queue_name)" - show_queue_counts "Inbound Event Queue - Queue Message Counts" "${PREFIX}-inbound-event-queue" - show_queue_counts "Inbound Event DLQ - Queue Message Counts" "${PREFIX}-inbound-event-dlq" + require_client_id + show_queue_counts "Client Delivery Queue - Message Counts" "${PREFIX}-${CLIENT_ID}-delivery-queue" + show_queue_counts "Client Delivery DLQ - Message Counts" "${PREFIX}-${CLIENT_ID}-delivery-dlq-queue" + show_queue_counts "Inbound Event Queue - Message Counts" "${PREFIX}-inbound-event-queue" + show_queue_counts "Inbound Event DLQ - Message Counts" "${PREFIX}-inbound-event-dlq" } peek_queue_message() { @@ -129,21 +125,19 @@ peek_queue_message() { } action_queue_peek() { - peek_queue_message "Mock Target DLQ - Message Peek" "$(target_dlq_queue_name)" + require_client_id + peek_queue_message "Client Delivery Queue - Message Peek" "${PREFIX}-${CLIENT_ID}-delivery-queue" + peek_queue_message "Client Delivery DLQ - Message Peek" "${PREFIX}-${CLIENT_ID}-delivery-dlq-queue" peek_queue_message "Inbound Event Queue - Message Peek" "${PREFIX}-inbound-event-queue" peek_queue_message "Inbound Event DLQ - Message Peek" "${PREFIX}-inbound-event-dlq" } log_filter_args() { - local -a args=() - local escaped_log_filter if [[ -n "$LOG_FILTER" ]]; then - escaped_log_filter="${LOG_FILTER//\"/\\\"}" + local escaped_log_filter="${LOG_FILTER//\"/\\\"}" # CloudWatch filter patterns treat quoted strings as exact phrases. - args+=(--filter-pattern "\"$escaped_log_filter\"") + printf '%s\n' --filter-pattern "\"$escaped_log_filter\"" fi - - printf '%s\n' "${args[@]}" } action_tail_transform() { From 43115c9e58fdb93ffe753f15a65746505f62f41c Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 10:18:12 +0100 Subject: [PATCH 04/13] Update SQS to webhook int tests to use correct queues --- tests/integration/helpers/sqs.ts | 15 +++++++++------ tests/integration/inbound-sqs-to-webhook.test.ts | 14 +++++++++++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/integration/helpers/sqs.ts b/tests/integration/helpers/sqs.ts index 857fd3a..747f746 100644 --- a/tests/integration/helpers/sqs.ts +++ b/tests/integration/helpers/sqs.ts @@ -46,13 +46,16 @@ function buildQueueUrl( export function buildMockClientDlqQueueUrl( deploymentDetails: DeploymentDetails, - targets: { targetId: string }[], + clientId: string, ): string { - const [firstTarget] = targets; - if (!firstTarget) { - throw new Error("At least one target is required to build DLQ URL"); - } - return buildQueueUrl(deploymentDetails, `${firstTarget.targetId}-dlq`); + return buildQueueUrl(deploymentDetails, `${clientId}-delivery-dlq`); +} + +export function buildMockClientDeliveryQueueUrl( + deploymentDetails: DeploymentDetails, + clientId: string, +): string { + return buildQueueUrl(deploymentDetails, `${clientId}-delivery`); } export async function sendSqsEvent( diff --git a/tests/integration/inbound-sqs-to-webhook.test.ts b/tests/integration/inbound-sqs-to-webhook.test.ts index 4305f05..3a73f5c 100644 --- a/tests/integration/inbound-sqs-to-webhook.test.ts +++ b/tests/integration/inbound-sqs-to-webhook.test.ts @@ -28,6 +28,7 @@ import { assertCallbackHeaders } from "./helpers/signature"; import { awaitQueueMessage, awaitQueueMessageByMessageId, + buildMockClientDeliveryQueueUrl, buildMockClientDlqQueueUrl, ensureInboundQueueIsEmpty, purgeQueues, @@ -49,6 +50,7 @@ describe("SQS to Webhook Integration", () => { let cloudWatchClient: CloudWatchLogsClient; let callbackEventQueueUrl: string; let clientDlqQueueUrl: string; + let clientDeliveryQueueUrl: string; let inboundEventDlqQueueUrl: string; let webhookLogGroupName: string; let webhookTargetPath: string; @@ -56,12 +58,16 @@ describe("SQS to Webhook Integration", () => { beforeAll(async () => { const deploymentDetails = getDeploymentDetails(); - const { targets } = getMockItClientConfig(); + const { clientId } = getMockItClientConfig(); sqsClient = createSqsClient(deploymentDetails); cloudWatchClient = createCloudWatchLogsClient(deploymentDetails); callbackEventQueueUrl = buildInboundEventQueueUrl(deploymentDetails); - clientDlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, targets); + clientDlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, clientId); + clientDeliveryQueueUrl = buildMockClientDeliveryQueueUrl( + deploymentDetails, + clientId, + ); inboundEventDlqQueueUrl = buildInboundEventDlqQueueUrl(deploymentDetails); webhookLogGroupName = buildLambdaLogGroupName( deploymentDetails, @@ -72,6 +78,7 @@ describe("SQS to Webhook Integration", () => { await purgeQueues(sqsClient, [ inboundEventDlqQueueUrl, clientDlqQueueUrl, + clientDeliveryQueueUrl, callbackEventQueueUrl, ]); }); @@ -80,6 +87,7 @@ describe("SQS to Webhook Integration", () => { await purgeQueues(sqsClient, [ inboundEventDlqQueueUrl, clientDlqQueueUrl, + clientDeliveryQueueUrl, callbackEventQueueUrl, ]); @@ -195,7 +203,7 @@ describe("SQS to Webhook Integration", () => { }); describe("Client Webhook DLQ", () => { - it("should route a non-retriable (4xx) webhook response to the per-target DLQ", async () => { + it("should route a non-retriable (4xx) webhook response to the per-client DLQ", async () => { const event: StatusPublishEvent = createMessageStatusPublishEvent({ data: { From e23208cb2b576477275ed4373bb6ceff6e11fd6c Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 10:36:28 +0100 Subject: [PATCH 05/13] Update debug int script README --- tests/integration/README.md | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/integration/README.md b/tests/integration/README.md index a58531b..27117c0 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -13,6 +13,7 @@ In normal delivery flow, integration tests are triggered via the CI workflow. - `ENVIRONMENT` (required) - `AWS_PROFILE` (required) - `AWS_REGION` (optional, defaults to `eu-west-2`) + - `CLIENT_ID` (required for queue and https-client actions, e.g. `mock-client-1`) ## Run Integration Tests Locally @@ -50,30 +51,31 @@ All are run via `make test-integration-debug ACTION=`. - [`queue-status`](#queue-status) – SQS queue message counts - [`queue-peek`](#queue-peek) – Peek at one message from each SQS queue - [`tail-transform`](#tail-transform) – Tail the transform/filter Lambda logs +- [`tail-https-client`](#tail-https-client) – Tail the https-client Lambda logs - [`tail-webhook`](#tail-webhook) – Tail the mock-webhook Lambda logs - [`tail-pipe`](#tail-pipe) – Tail the EventBridge pipe logs - [`pipe-state`](#pipe-state) – Show EventBridge pipe state and recent metrics -All log-tailing actions (`tail-transform`, `tail-webhook`, `tail-pipe`) accept an optional `LOG_FILTER` to narrow output to a specific message ID or pattern. +All log-tailing actions (`tail-transform`, `tail-https-client`, `tail-webhook`, `tail-pipe`) accept an optional `LOG_FILTER` to narrow output to a specific message ID or pattern. --- ### `queue-status` -Shows approximate message counts for the inbound event queue, inbound event DLQ, and mock target DLQ. +Shows approximate message counts for the inbound event queue, inbound event DLQ, client delivery queue, and client delivery DLQ. Requires `CLIENT_ID`. ```sh -ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=queue-status +CLIENT_ID= ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=queue-status ``` --- ### `queue-peek` -Reads one message (without deleting it) from each of the same three queues, printing body, attributes, and message attributes. +Reads one message (without deleting it) from each of the same four queues, printing body, attributes, and message attributes. Requires `CLIENT_ID`. ```sh -ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=queue-peek +CLIENT_ID= ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=queue-peek ``` --- @@ -94,6 +96,22 @@ ENVIRONMENT= AWS_PROFILE= LOG_FILTER=SOME-MESSAGE-ID make test-int --- +### `tail-https-client` + +Tails CloudWatch logs for the `https-client` Lambda for the given client, following from the last 30 minutes. Requires `CLIENT_ID`. + +```sh +CLIENT_ID= ENVIRONMENT= AWS_PROFILE= make test-integration-debug ACTION=tail-https-client +``` + +Filter to a specific message ID: + +```sh +CLIENT_ID= ENVIRONMENT= AWS_PROFILE= LOG_FILTER=SOME-MESSAGE-ID make test-integration-debug ACTION=tail-https-client +``` + +--- + ### `tail-webhook` Tails CloudWatch logs for the `mock-webhook` Lambda, following from the last 30 minutes. From c857768784645b3c24a74f6b1d030d703d078353 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 10:52:10 +0100 Subject: [PATCH 06/13] Fix redis script error and better logging for redis errors --- .../src/__tests__/endpoint-gate.test.ts | 60 +++++++++++++------ .../src/services/endpoint-gate.ts | 13 ++-- 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/lambdas/https-client-lambda/src/__tests__/endpoint-gate.test.ts b/lambdas/https-client-lambda/src/__tests__/endpoint-gate.test.ts index 394a89b..597727e 100644 --- a/lambdas/https-client-lambda/src/__tests__/endpoint-gate.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/endpoint-gate.test.ts @@ -126,14 +126,6 @@ describe("admit", () => { ); }); - it("propagates non-NOSCRIPT Redis errors", async () => { - mockSendCommand.mockRejectedValueOnce(new Error("Connection refused")); - - await expect( - admit(mockRedis, "target-1", 10, true, defaultConfig), - ).rejects.toThrow("Connection refused"); - }); - it("passes cbProbeIntervalMs=0 when circuit breaker is disabled", async () => { mockSendCommand.mockResolvedValueOnce([1, "allowed", 0, 10]); @@ -151,8 +143,46 @@ describe("admit", () => { await admit(mockRedis, "my-target", 5, true, defaultConfig); const args = mockSendCommand.mock.calls[0]![0] as string[]; - expect(args[3]).toBe("cb:my-target"); - expect(args[4]).toBe("rl:my-target"); + expect(args[3]).toBe("cb:{my-target}"); + expect(args[4]).toBe("rl:{my-target}"); + }); +}); + +describe("evalScript", () => { + it("throws a wrapped error including the original message when EVALSHA fails with a non-NOSCRIPT Error", async () => { + const redisError = new Error("WRONGTYPE Operation against a key"); + mockSendCommand.mockRejectedValueOnce(redisError); + + const thrown = await admit( + mockRedis, + "target-1", + 10, + true, + defaultConfig, + ).catch((error: unknown) => error); + + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toContain("Redis error in script"); + expect((thrown as Error).message).toContain( + "WRONGTYPE Operation against a key", + ); + expect((thrown as Error & { cause: unknown }).cause).toBe(redisError); + }); + + it("throws a wrapped error using String() when EVALSHA rejects with a non-Error value", async () => { + mockSendCommand.mockRejectedValueOnce("connection refused"); + + const thrown = await admit( + mockRedis, + "target-1", + 10, + true, + defaultConfig, + ).catch((error: unknown) => error); + + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toContain("Redis error in script"); + expect((thrown as Error).message).toContain("connection refused"); }); }); @@ -215,21 +245,13 @@ describe("recordResult", () => { expect(mockSendCommand).toHaveBeenCalledTimes(2); }); - it("propagates non-NOSCRIPT Redis errors", async () => { - mockSendCommand.mockRejectedValueOnce(new Error("Connection refused")); - - await expect( - recordResult(mockRedis, "target-1", false, defaultConfig), - ).rejects.toThrow("Connection refused"); - }); - it("passes correct cb key for target", async () => { mockSendCommand.mockResolvedValueOnce([1, "closed"]); await recordResult(mockRedis, "my-target", true, defaultConfig); const args = mockSendCommand.mock.calls[0]![0] as string[]; - expect(args[3]).toBe("cb:my-target"); + expect(args[3]).toBe("cb:{my-target}"); }); }); diff --git a/lambdas/https-client-lambda/src/services/endpoint-gate.ts b/lambdas/https-client-lambda/src/services/endpoint-gate.ts index c0dd1b6..6932227 100644 --- a/lambdas/https-client-lambda/src/services/endpoint-gate.ts +++ b/lambdas/https-client-lambda/src/services/endpoint-gate.ts @@ -64,7 +64,12 @@ async function evalScript( const isNoScript = error instanceof Error && error.message.includes("NOSCRIPT"); if (!isNoScript) { - throw error; + throw new Error( + `Redis error in script ${script}: ${ + error instanceof Error ? error.message : String(error) + }`, + { cause: error }, + ); } return client.sendCommand(["EVAL", script, keyCount, ...keys, ...args]); } @@ -77,8 +82,8 @@ export async function admit( cbEnabled: boolean, config: EndpointGateConfig, ): Promise { - const cbKey = `cb:${targetId}`; - const rlKey = `rl:${targetId}`; + const cbKey = `cb:{${targetId}}`; + const rlKey = `rl:{${targetId}}`; const now = Date.now().toString(); const probeIntervalMs = cbEnabled ? config.cbProbeIntervalMs.toString() : "0"; @@ -129,7 +134,7 @@ export async function recordResult( success: boolean, config: EndpointGateConfig, ): Promise { - const cbKey = `cb:${targetId}`; + const cbKey = `cb:{${targetId}}`; const now = Date.now().toString(); const args = [ From 79407daeae22d00e930f733f043d41fafa847497 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 11:23:42 +0100 Subject: [PATCH 07/13] Log status code of perm failures --- .../src/__tests__/https-client.test.ts | 14 +++++++++++--- lambdas/https-client-lambda/src/handler.ts | 7 ++++++- .../src/services/delivery-observability.ts | 4 ++++ .../src/services/delivery/https-client.ts | 13 +++++++++---- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts index e185056..8d9be8f 100644 --- a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts @@ -134,7 +134,7 @@ describe("deliverPayload", () => { createMockAgent(), ); - expect(result).toEqual({ outcome: "permanent_failure" }); + expect(result).toEqual({ outcome: "permanent_failure", statusCode: 400 }); }); it("returns permanent_failure on TLS error CERT_HAS_EXPIRED", async () => { @@ -147,7 +147,10 @@ describe("deliverPayload", () => { createMockAgent(), ); - expect(result).toEqual({ outcome: "permanent_failure" }); + expect(result).toEqual({ + outcome: "permanent_failure", + errorCode: "CERT_HAS_EXPIRED", + }); }); it("returns permanent_failure on TLS pinning error", async () => { @@ -160,7 +163,10 @@ describe("deliverPayload", () => { createMockAgent(), ); - expect(result).toEqual({ outcome: "permanent_failure" }); + expect(result).toEqual({ + outcome: "permanent_failure", + errorCode: "ERR_CERT_PINNING_FAILED", + }); }); it("returns transient_failure on 5xx", async () => { @@ -189,6 +195,7 @@ describe("deliverPayload", () => { expect(result).toEqual({ outcome: "rate_limited", retryAfterHeader: "60", + statusCode: 429, }); }); @@ -205,6 +212,7 @@ describe("deliverPayload", () => { expect(result).toEqual({ outcome: "rate_limited", retryAfterHeader: undefined, + statusCode: 429, }); }); diff --git a/lambdas/https-client-lambda/src/handler.ts b/lambdas/https-client-lambda/src/handler.ts index e83f635..513c502 100644 --- a/lambdas/https-client-lambda/src/handler.ts +++ b/lambdas/https-client-lambda/src/handler.ts @@ -100,7 +100,12 @@ async function handleDeliveryResult( } if (result.outcome === "permanent_failure") { - recordDeliveryPermanentFailure(clientId, targetId); + recordDeliveryPermanentFailure( + clientId, + targetId, + result.statusCode, + result.errorCode, + ); await sendToDlq(record.body); return; } diff --git a/lambdas/https-client-lambda/src/services/delivery-observability.ts b/lambdas/https-client-lambda/src/services/delivery-observability.ts index 8fd4cea..50dbb30 100644 --- a/lambdas/https-client-lambda/src/services/delivery-observability.ts +++ b/lambdas/https-client-lambda/src/services/delivery-observability.ts @@ -31,11 +31,15 @@ export function recordDeliverySuccess( export function recordDeliveryPermanentFailure( clientId: string, targetId: string, + statusCode?: number, + errorCode?: string, ): void { emitDeliveryPermanentFailure(targetId); logger.warn("Permanent delivery failure — sending to DLQ", { clientId, targetId, + ...(statusCode !== undefined && { statusCode }), + ...(errorCode !== undefined && { errorCode }), }); } diff --git a/lambdas/https-client-lambda/src/services/delivery/https-client.ts b/lambdas/https-client-lambda/src/services/delivery/https-client.ts index c651fe6..9d85f2a 100644 --- a/lambdas/https-client-lambda/src/services/delivery/https-client.ts +++ b/lambdas/https-client-lambda/src/services/delivery/https-client.ts @@ -5,8 +5,12 @@ import { PERMANENT_TLS_ERROR_CODES } from "services/delivery/tls-agent-factory"; export type DeliveryResult = | { outcome: "success" } - | { outcome: "permanent_failure" } - | { outcome: "rate_limited"; retryAfterHeader: string | undefined } + | { outcome: "permanent_failure"; statusCode?: number; errorCode?: string } + | { + outcome: "rate_limited"; + statusCode: 429; + retryAfterHeader: string | undefined; + } | { outcome: "transient_failure"; statusCode: number }; export function deliverPayload( @@ -46,13 +50,14 @@ export function deliverPayload( const retryAfterHeader = res.headers["retry-after"]; resolve({ outcome: "rate_limited", + statusCode, retryAfterHeader, }); return; } if (statusCode >= 400 && statusCode < 500) { - resolve({ outcome: "permanent_failure" }); + resolve({ outcome: "permanent_failure", statusCode }); return; } @@ -66,7 +71,7 @@ export function deliverPayload( req.on("error", (error: NodeJS.ErrnoException) => { if (error.code && PERMANENT_TLS_ERROR_CODES.has(error.code)) { - resolve({ outcome: "permanent_failure" }); + resolve({ outcome: "permanent_failure", errorCode: error.code }); return; } From 3065a520aa255b1a07a62ba075b9b51592595c78 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 12:06:07 +0100 Subject: [PATCH 08/13] fix: load test CA for server trust when mtls is disabled In test environments, the mock webhook ALB uses a server certificate signed by a locally-generated test CA. Previously, the CA was only loaded into the TLS agent when mtls.enabled was true (needed for client certificate auth). Targets with mtls.enabled: false used Node's default trust store, which does not include the test CA, causing every delivery attempt to fail with SELF_SIGNED_CERT_IN_CHAIN. Fix by loading the CA whenever MTLS_TEST_CA_S3_KEY is set, regardless of mtls.enabled. The client key and cert are still only applied when mtls.enabled is true. MTLS_TEST_CA_S3_KEY is not set in production, so non-mTLS targets in production are unaffected. --- .../src/__tests__/tls-agent-factory.test.ts | 28 +++++++++++++++++++ .../services/delivery/tls-agent-factory.ts | 12 ++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lambdas/https-client-lambda/src/__tests__/tls-agent-factory.test.ts b/lambdas/https-client-lambda/src/__tests__/tls-agent-factory.test.ts index b2ca787..fae8112 100644 --- a/lambdas/https-client-lambda/src/__tests__/tls-agent-factory.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/tls-agent-factory.test.ts @@ -129,6 +129,34 @@ describe("tls-agent-factory", () => { expect(mockSecretsManagerSend).not.toHaveBeenCalled(); }); + it("loads test CA for server trust when MTLS_TEST_CA_S3_KEY is set and mtls is disabled", async () => { + process.env.MTLS_TEST_CA_S3_KEY = "test-ca.pem"; + jest.resetModules(); + // @ts-expect-error -- modulePaths resolves at runtime + const mod = await import("services/delivery/tls-agent-factory"); + + const caPem = + "-----BEGIN CERTIFICATE-----\ntest-ca\n-----END CERTIFICATE-----"; + mockS3Send + .mockResolvedValueOnce({ + Body: { + transformToString: jest.fn().mockResolvedValue(COMBINED_PEM), + }, + }) + .mockResolvedValueOnce({ + Body: { transformToString: jest.fn().mockResolvedValue(caPem) }, + }); + + const agent = await mod.buildAgent( + createTarget({ delivery: { mtls: { enabled: false } } }), + ); + + expect(agent).toBeDefined(); + expect(agent.options.ca).toBe(caPem); + expect(agent.options.key).toBeUndefined(); + expect(agent.options.cert).toBeUndefined(); + }); + it("loads test CA when MTLS_TEST_CA_S3_KEY is set", async () => { process.env.MTLS_TEST_CA_S3_KEY = "test-ca.pem"; jest.resetModules(); diff --git a/lambdas/https-client-lambda/src/services/delivery/tls-agent-factory.ts b/lambdas/https-client-lambda/src/services/delivery/tls-agent-factory.ts index e6c0fcf..fb1ea13 100644 --- a/lambdas/https-client-lambda/src/services/delivery/tls-agent-factory.ts +++ b/lambdas/https-client-lambda/src/services/delivery/tls-agent-factory.ts @@ -150,14 +150,20 @@ export async function buildAgent(target: CallbackTarget): Promise { ); } - if (target.delivery?.mtls?.enabled) { + // Always load the CA in test environments (MTLS_TEST_CA_S3_KEY set) so that + // targets with mtls.enabled: false can still verify the server's cert chain. + // In production the CA comes from SecretsManager only when mTLS is in use. + if (target.delivery?.mtls?.enabled || MTLS_TEST_CA_S3_KEY) { const material = await getMaterial(); - agentOptions.key = material.key; - agentOptions.cert = material.cert; if (material.ca) { agentOptions.ca = material.ca; } + + if (target.delivery?.mtls?.enabled) { + agentOptions.key = material.key; + agentOptions.cert = material.cert; + } } if (certPinning?.enabled) { From 9a918171e3f3b34bf79e16c8957b00e027b42200 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 13:28:53 +0100 Subject: [PATCH 09/13] fix: set ERROR_CODE and ERROR_MESSAGE on DLQ messages for permanent delivery failures AWS API Destinations previously set these SQS message attributes automatically. After the migration to the https-client lambda, they were no longer being set. - Read the response body for 4xx responses (previously discarded with res.resume()) so the error message can be included in the DLQ message attributes - Set ERROR_CODE=HTTP_CLIENT_ERROR for 4xx webhook rejections, or the TLS error code (e.g. CERT_HAS_EXPIRED) for connection-level failures - Set ERROR_MESSAGE to the message field from the JSON response body, falling back to the raw body if not valid JSON - Extended sendToDlq to accept and forward MessageAttributes to SQS --- .../src/__tests__/handler.test.ts | 45 ++++++++++++++++++- .../src/__tests__/https-client.test.ts | 17 +++++-- lambdas/https-client-lambda/src/handler.ts | 31 ++++++++++++- .../src/services/delivery/https-client.ts | 19 ++++++-- .../src/services/dlq-sender.ts | 12 ++++- .../inbound-sqs-to-webhook.test.ts | 2 +- 6 files changed, 114 insertions(+), 12 deletions(-) diff --git a/lambdas/https-client-lambda/src/__tests__/handler.test.ts b/lambdas/https-client-lambda/src/__tests__/handler.test.ts index 8b24d5e..f13c18f 100644 --- a/lambdas/https-client-lambda/src/__tests__/handler.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/handler.test.ts @@ -135,7 +135,50 @@ describe("processRecords", () => { const failures = await processRecords([makeRecord()]); expect(failures).toEqual([]); - expect(mockSendToDlq).toHaveBeenCalledWith(makeRecord().body); + expect(mockSendToDlq).toHaveBeenCalledWith(makeRecord().body, {}); + }); + + it("sends ERROR_CODE and ERROR_MESSAGE attributes to DLQ for 4xx with JSON body", async () => { + mockDeliverPayload.mockResolvedValue({ + outcome: "permanent_failure", + statusCode: 400, + responseBody: JSON.stringify({ message: "Bad request" }), + }); + + await processRecords([makeRecord()]); + + expect(mockSendToDlq).toHaveBeenCalledWith(makeRecord().body, { + ERROR_CODE: { DataType: "String", StringValue: "HTTP_CLIENT_ERROR" }, + ERROR_MESSAGE: { DataType: "String", StringValue: "Bad request" }, + }); + }); + + it("uses raw response body as ERROR_MESSAGE when not valid JSON", async () => { + mockDeliverPayload.mockResolvedValue({ + outcome: "permanent_failure", + statusCode: 400, + responseBody: "Bad request", + }); + + await processRecords([makeRecord()]); + + expect(mockSendToDlq).toHaveBeenCalledWith(makeRecord().body, { + ERROR_CODE: { DataType: "String", StringValue: "HTTP_CLIENT_ERROR" }, + ERROR_MESSAGE: { DataType: "String", StringValue: "Bad request" }, + }); + }); + + it("sends ERROR_CODE from TLS errorCode with no ERROR_MESSAGE", async () => { + mockDeliverPayload.mockResolvedValue({ + outcome: "permanent_failure", + errorCode: "CERT_HAS_EXPIRED", + }); + + await processRecords([makeRecord()]); + + expect(mockSendToDlq).toHaveBeenCalledWith(makeRecord().body, { + ERROR_CODE: { DataType: "String", StringValue: "CERT_HAS_EXPIRED" }, + }); }); it("returns failure for transient 5xx errors", async () => { diff --git a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts index 8d9be8f..b78009e 100644 --- a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts @@ -36,6 +36,7 @@ type MockResponse = EventEmitter & { function mockHttpsRequest( statusCode: number, headers: Record = {}, + body = "", ) { const mockReq = new EventEmitter() as EventEmitter & { end: jest.Mock; @@ -56,7 +57,13 @@ function mockHttpsRequest( }); if (callback) { - process.nextTick(() => callback(res)); + process.nextTick(() => { + callback(res); + process.nextTick(() => { + if (body) res.emit("data", Buffer.from(body)); + res.emit("end"); + }); + }); } return mockReq as unknown as ReturnType; @@ -125,7 +132,7 @@ describe("deliverPayload", () => { }); it("returns permanent_failure on 4xx non-429", async () => { - mockHttpsRequest(400); + mockHttpsRequest(400, {}, JSON.stringify({ message: "Bad request" })); const result = await deliverPayload( createTarget(), @@ -134,7 +141,11 @@ describe("deliverPayload", () => { createMockAgent(), ); - expect(result).toEqual({ outcome: "permanent_failure", statusCode: 400 }); + expect(result).toEqual({ + outcome: "permanent_failure", + statusCode: 400, + responseBody: JSON.stringify({ message: "Bad request" }), + }); }); it("returns permanent_failure on TLS error CERT_HAS_EXPIRED", async () => { diff --git a/lambdas/https-client-lambda/src/handler.ts b/lambdas/https-client-lambda/src/handler.ts index 513c502..8dc551b 100644 --- a/lambdas/https-client-lambda/src/handler.ts +++ b/lambdas/https-client-lambda/src/handler.ts @@ -35,11 +35,40 @@ import { } from "services/delivery-observability"; import { flushMetrics } from "services/delivery-metrics"; +import type { MessageAttributeValue } from "@aws-sdk/client-sqs"; + type RedisClientType = Awaited>; const DEFAULT_MAX_RETRY_DURATION_MS = 7_200_000; const DEFAULT_CONCURRENCY_LIMIT = 5; +function buildDlqAttributes( + result: Extract, +): Record { + const attrs: Record = {}; + + if (result.errorCode) { + attrs.ERROR_CODE = { DataType: "String", StringValue: result.errorCode }; + } else if (result.statusCode !== undefined) { + attrs.ERROR_CODE = { DataType: "String", StringValue: "HTTP_CLIENT_ERROR" }; + } + + if (result.responseBody) { + let errorMessage = result.responseBody; + try { + const parsed = JSON.parse(result.responseBody) as { message?: string }; + if (parsed.message) { + errorMessage = parsed.message; + } + } catch { + // use raw body if not valid JSON + } + attrs.ERROR_MESSAGE = { DataType: "String", StringValue: errorMessage }; + } + + return attrs; +} + const gateConfig: EndpointGateConfig = { burstCapacity: Number(process.env.TOKEN_BUCKET_BURST_CAPACITY ?? "10"), cbProbeIntervalMs: Number(process.env.CB_PROBE_INTERVAL_MS ?? "60000"), @@ -106,7 +135,7 @@ async function handleDeliveryResult( result.statusCode, result.errorCode, ); - await sendToDlq(record.body); + await sendToDlq(record.body, buildDlqAttributes(result)); return; } diff --git a/lambdas/https-client-lambda/src/services/delivery/https-client.ts b/lambdas/https-client-lambda/src/services/delivery/https-client.ts index 9d85f2a..1cc3583 100644 --- a/lambdas/https-client-lambda/src/services/delivery/https-client.ts +++ b/lambdas/https-client-lambda/src/services/delivery/https-client.ts @@ -5,7 +5,12 @@ import { PERMANENT_TLS_ERROR_CODES } from "services/delivery/tls-agent-factory"; export type DeliveryResult = | { outcome: "success" } - | { outcome: "permanent_failure"; statusCode?: number; errorCode?: string } + | { + outcome: "permanent_failure"; + statusCode?: number; + errorCode?: string; + responseBody?: string; + } | { outcome: "rate_limited"; statusCode: 429; @@ -37,16 +42,16 @@ export function deliverPayload( }, }, (res) => { - res.resume(); - const statusCode = res.statusCode ?? 0; if (statusCode >= 200 && statusCode < 300) { + res.resume(); resolve({ outcome: "success" }); return; } if (statusCode === 429) { + res.resume(); const retryAfterHeader = res.headers["retry-after"]; resolve({ outcome: "rate_limited", @@ -57,10 +62,16 @@ export function deliverPayload( } if (statusCode >= 400 && statusCode < 500) { - resolve({ outcome: "permanent_failure", statusCode }); + const chunks: Buffer[] = []; + res.on("data", (chunk: Buffer) => chunks.push(chunk)); + res.on("end", () => { + const responseBody = Buffer.concat(chunks).toString("utf8"); + resolve({ outcome: "permanent_failure", statusCode, responseBody }); + }); return; } + res.resume(); resolve({ outcome: "transient_failure", statusCode }); }, ); diff --git a/lambdas/https-client-lambda/src/services/dlq-sender.ts b/lambdas/https-client-lambda/src/services/dlq-sender.ts index af61a66..d482e79 100644 --- a/lambdas/https-client-lambda/src/services/dlq-sender.ts +++ b/lambdas/https-client-lambda/src/services/dlq-sender.ts @@ -1,8 +1,15 @@ -import { SQSClient, SendMessageCommand } from "@aws-sdk/client-sqs"; +import { + type MessageAttributeValue, + SQSClient, + SendMessageCommand, +} from "@aws-sdk/client-sqs"; const sqsClient = new SQSClient({}); -export async function sendToDlq(messageBody: string): Promise { +export async function sendToDlq( + messageBody: string, + messageAttributes?: Record, +): Promise { const { DLQ_URL } = process.env; if (!DLQ_URL) { throw new Error("DLQ_URL is required"); @@ -12,6 +19,7 @@ export async function sendToDlq(messageBody: string): Promise { new SendMessageCommand({ QueueUrl: DLQ_URL, MessageBody: messageBody, + ...(messageAttributes && { MessageAttributes: messageAttributes }), }), ); } diff --git a/tests/integration/inbound-sqs-to-webhook.test.ts b/tests/integration/inbound-sqs-to-webhook.test.ts index 3a73f5c..d75ad40 100644 --- a/tests/integration/inbound-sqs-to-webhook.test.ts +++ b/tests/integration/inbound-sqs-to-webhook.test.ts @@ -217,7 +217,7 @@ describe("SQS to Webhook Integration", () => { expect(dlqMessage.Body).toBeDefined(); expect(dlqMessage.MessageAttributes?.ERROR_CODE?.StringValue).toBe( - "INVALID_PARAMETER", + "HTTP_CLIENT_ERROR", ); expect( dlqMessage.MessageAttributes?.ERROR_MESSAGE?.StringValue, From ec68d4c42d8782b0ed9c1550b36a928e365879b4 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 13:46:00 +0100 Subject: [PATCH 10/13] Fix redrive IT dlq names --- tests/integration/dlq-redrive.test.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/integration/dlq-redrive.test.ts b/tests/integration/dlq-redrive.test.ts index e88e492..639eadc 100644 --- a/tests/integration/dlq-redrive.test.ts +++ b/tests/integration/dlq-redrive.test.ts @@ -19,8 +19,10 @@ import { sendSqsEvent, } from "./helpers/sqs"; import { + CLIENT_FIXTURES, + type ClientFixtureKey, buildMockWebhookTargetPath, - getAllSubscriptionTargetIds, + getClientConfig, getMockItClientConfig, } from "./helpers/mock-client-config"; import { awaitSignedCallbacksFromWebhookLogGroup } from "./helpers/cloudwatch"; @@ -37,20 +39,20 @@ describe("DLQ Redrive", () => { beforeAll(async () => { const deploymentDetails = getDeploymentDetails(); - const mockClient1 = getMockItClientConfig(); - - const allSubscriptionTargetIds = getAllSubscriptionTargetIds(); + const { clientId } = getMockItClientConfig(); sqsClient = createSqsClient(deploymentDetails); cloudWatchClient = createCloudWatchLogsClient(deploymentDetails); inboundQueueUrl = buildInboundEventQueueUrl(deploymentDetails); - dlqQueueUrl = buildMockClientDlqQueueUrl( - deploymentDetails, - mockClient1.targets, - ); - allTargetDlqQueueUrls = allSubscriptionTargetIds.map((targetId) => - buildMockClientDlqQueueUrl(deploymentDetails, [{ targetId }]), + dlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, clientId); + allTargetDlqQueueUrls = ( + Object.keys(CLIENT_FIXTURES) as ClientFixtureKey[] + ).map((key) => + buildMockClientDlqQueueUrl( + deploymentDetails, + getClientConfig(key).clientId, + ), ); webhookLogGroupName = buildLambdaLogGroupName( deploymentDetails, @@ -67,7 +69,7 @@ describe("DLQ Redrive", () => { }); describe("Infrastructure validation", () => { - it("should confirm a target DLQ is accessible for all configured subscription targets", async () => { + it("should confirm a DLQ is accessible for all configured clients", async () => { const responses = await Promise.all( allTargetDlqQueueUrls.map((queueUrl) => sqsClient.send( From 9822d774f16ef49fca02c917bd3c6f6e0a3271de Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 13:46:31 +0100 Subject: [PATCH 11/13] Fix metric IT dlq names --- tests/integration/metrics.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/metrics.test.ts b/tests/integration/metrics.test.ts index 2f314f8..f40eba6 100644 --- a/tests/integration/metrics.test.ts +++ b/tests/integration/metrics.test.ts @@ -40,12 +40,12 @@ describe("Metrics", () => { beforeAll(async () => { const deploymentDetails = getDeploymentDetails(); - const { targets } = getMockItClientConfig(); + const { clientId } = getMockItClientConfig(); sqsClient = createSqsClient(deploymentDetails); cloudWatchClient = createCloudWatchLogsClient(deploymentDetails); callbackEventQueueUrl = buildInboundEventQueueUrl(deploymentDetails); - clientDlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, targets); + clientDlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, clientId); inboundEventDlqQueueUrl = buildInboundEventDlqQueueUrl(deploymentDetails); logGroupName = buildLambdaLogGroupName( deploymentDetails, From b8d5b8a67daef132944bb505ad6a6a8395cfd596 Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 14:01:38 +0100 Subject: [PATCH 12/13] Fix alarm test --- tests/integration/dlq-alarms.test.ts | 35 +++++++++++-------- .../integration/helpers/mock-client-config.ts | 23 ------------ 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/tests/integration/dlq-alarms.test.ts b/tests/integration/dlq-alarms.test.ts index 1cf3a57..c4f69fa 100644 --- a/tests/integration/dlq-alarms.test.ts +++ b/tests/integration/dlq-alarms.test.ts @@ -5,14 +5,18 @@ import { } from "@aws-sdk/client-cloudwatch"; import type { DeploymentDetails } from "@nhs-notify-client-callbacks/test-support/helpers"; import { getDeploymentDetails } from "@nhs-notify-client-callbacks/test-support/helpers"; -import { getAllSubscriptionTargetIds } from "./helpers/mock-client-config"; +import { + CLIENT_FIXTURES, + type ClientFixtureKey, + getClientConfig, +} from "./helpers/mock-client-config"; import { buildMockClientDlqQueueUrl } from "./helpers/sqs"; function buildDlqDepthAlarmName( { component, environment, project }: DeploymentDetails, - targetId: string, + clientId: string, ): string { - return `${project}-${environment}-${component}-${targetId}-dlq-depth`; + return `${project}-${environment}-${component}-${clientId}-dlq-depth`; } function getQueueNameFromUrl(queueUrl: string): string { @@ -27,7 +31,7 @@ function getQueueNameFromUrl(queueUrl: string): string { describe("DLQ alarms", () => { let cloudWatchClient: CloudWatchClient; let deploymentDetails: DeploymentDetails; - let targetIds: string[]; + let clientIds: string[]; beforeAll(() => { deploymentDetails = getDeploymentDetails(); @@ -35,22 +39,25 @@ describe("DLQ alarms", () => { region: deploymentDetails.region, }); - targetIds = getAllSubscriptionTargetIds(); + clientIds = (Object.keys(CLIENT_FIXTURES) as ClientFixtureKey[]).map( + (key) => getClientConfig(key).clientId, + ); }); afterAll(() => { cloudWatchClient.destroy(); }); - it("should create a DLQ depth alarm for every target DLQ", async () => { - expect(targetIds.length).toBeGreaterThan(0); + it("should create a DLQ depth alarm for every client DLQ", async () => { + expect(clientIds.length).toBeGreaterThan(0); - for (const targetId of targetIds) { - const alarmName = buildDlqDepthAlarmName(deploymentDetails, targetId); - const targetDlqQueueUrl = buildMockClientDlqQueueUrl(deploymentDetails, [ - { targetId }, - ]); - const targetDlqQueueName = getQueueNameFromUrl(targetDlqQueueUrl); + for (const clientId of clientIds) { + const alarmName = buildDlqDepthAlarmName(deploymentDetails, clientId); + const clientDlqQueueUrl = buildMockClientDlqQueueUrl( + deploymentDetails, + clientId, + ); + const clientDlqQueueName = getQueueNameFromUrl(clientDlqQueueUrl); const response = await cloudWatchClient.send( new DescribeAlarmsCommand({ AlarmNames: [alarmName], @@ -67,7 +74,7 @@ describe("DLQ alarms", () => { expect.arrayContaining([ expect.objectContaining({ Name: "QueueName", - Value: targetDlqQueueName, + Value: clientDlqQueueName, }), ]), ); diff --git a/tests/integration/helpers/mock-client-config.ts b/tests/integration/helpers/mock-client-config.ts index a004b4b..950e699 100644 --- a/tests/integration/helpers/mock-client-config.ts +++ b/tests/integration/helpers/mock-client-config.ts @@ -29,14 +29,6 @@ export const CLIENT_FIXTURES = { export type ClientFixtureKey = keyof typeof CLIENT_FIXTURES; -const ALL_CLIENT_FIXTURE_KEYS = Object.keys( - CLIENT_FIXTURES, -) as ClientFixtureKey[]; - -function dedupe(values: string[]): string[] { - return [...new Set(values)]; -} - export function getClientConfig(key: ClientFixtureKey): MockItClientConfig { // eslint-disable-next-line security/detect-object-injection -- key is constrained to ClientFixtureKey, a keyof the hardcoded as-const CLIENT_FIXTURES object const { apiKeyVar, applicationIdVar, fixture } = CLIENT_FIXTURES[key]; @@ -82,18 +74,3 @@ export function buildMockWebhookTargetPaths( ): string[] { return buildWebhookTargetPaths(key); } - -export function getSubscriptionTargetIds( - key: ClientFixtureKey = "client1", -): string[] { - const config = getClientConfig(key); - return dedupe( - config.subscriptions.flatMap((subscription) => subscription.targetIds), - ); -} - -export function getAllSubscriptionTargetIds( - keys: ClientFixtureKey[] = ALL_CLIENT_FIXTURE_KEYS, -): string[] { - return dedupe(keys.flatMap((key) => getSubscriptionTargetIds(key))); -} From a30e7f97590bcbd5f7e9ca537eeff364487fe01f Mon Sep 17 00:00:00 2001 From: Mike Wild Date: Mon, 20 Apr 2026 15:18:23 +0100 Subject: [PATCH 13/13] Bump test coverage --- .../src/__tests__/dlq-sender.test.ts | 13 +++++++ .../src/__tests__/handler.test.ts | 13 +++++++ .../src/__tests__/https-client.test.ts | 39 +++++++++++++++++++ lambdas/https-client-lambda/src/handler.ts | 2 +- 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/lambdas/https-client-lambda/src/__tests__/dlq-sender.test.ts b/lambdas/https-client-lambda/src/__tests__/dlq-sender.test.ts index 21ae370..9570700 100644 --- a/lambdas/https-client-lambda/src/__tests__/dlq-sender.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/dlq-sender.test.ts @@ -54,4 +54,17 @@ describe("sendToDlq", () => { process.env.DLQ_URL = saved; }); + + it("includes MessageAttributes in the command when provided", async () => { + mockSend.mockResolvedValue({}); + const attributes = { + ERROR_CODE: { DataType: "String", StringValue: "HTTP_CLIENT_ERROR" }, + }; + + await sendToDlq('{"test":"message"}', attributes); + + const command = mockSend.mock.calls[0][0]; + expect(command).toBeInstanceOf(SendMessageCommand); + expect(command.input.MessageAttributes).toEqual(attributes); + }); }); diff --git a/lambdas/https-client-lambda/src/__tests__/handler.test.ts b/lambdas/https-client-lambda/src/__tests__/handler.test.ts index f13c18f..d2b4fc3 100644 --- a/lambdas/https-client-lambda/src/__tests__/handler.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/handler.test.ts @@ -511,4 +511,17 @@ describe("processRecords", () => { expect(emitRateLimited).toHaveBeenCalledWith("target-1"); }); + + it("returns no failure when handleRateLimitedRecord resolves without throwing", async () => { + mockDeliverPayload.mockResolvedValue({ + outcome: "permanent_failure", + statusCode: 429, + retryAfterHeader: "60", + }); + mockHandleRateLimitedRecord.mockResolvedValueOnce(undefined); + + const failures = await processRecords([makeRecord()]); + + expect(failures).toEqual([]); + }); }); diff --git a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts index b78009e..a6229c5 100644 --- a/lambdas/https-client-lambda/src/__tests__/https-client.test.ts +++ b/lambdas/https-client-lambda/src/__tests__/https-client.test.ts @@ -306,4 +306,43 @@ describe("deliverPayload", () => { expect(result).toEqual({ outcome: "transient_failure", statusCode: 0 }); }); + + it("treats undefined statusCode as 0", async () => { + const mockReq = new EventEmitter() as EventEmitter & { + end: jest.Mock; + destroy: jest.Mock; + }; + mockReq.end = jest.fn(); + mockReq.destroy = jest.fn(); + + jest.spyOn(https, "request").mockImplementation((...args: unknown[]) => { + const callback = args.find((a) => typeof a === "function") as + | ((res: MockResponse) => void) + | undefined; + + const res = Object.assign(new EventEmitter(), { + statusCode: undefined as unknown as number, + headers: {}, + resume: jest.fn(), + }) as MockResponse; + + if (callback) { + process.nextTick(() => { + callback(res); + process.nextTick(() => (res as EventEmitter).emit("end")); + }); + } + + return mockReq as unknown as ReturnType; + }); + + const result = await deliverPayload( + createTarget(), + '{"test":true}', + "sig-abc", + createMockAgent(), + ); + + expect(result).toEqual({ outcome: "transient_failure", statusCode: 0 }); + }); }); diff --git a/lambdas/https-client-lambda/src/handler.ts b/lambdas/https-client-lambda/src/handler.ts index 8dc551b..75fbfbd 100644 --- a/lambdas/https-client-lambda/src/handler.ts +++ b/lambdas/https-client-lambda/src/handler.ts @@ -43,7 +43,7 @@ const DEFAULT_MAX_RETRY_DURATION_MS = 7_200_000; const DEFAULT_CONCURRENCY_LIMIT = 5; function buildDlqAttributes( - result: Extract, + result: Extract, ): Record { const attrs: Record = {};