Skip to content

fix(custom-resources): retry CloudFormation response PUT in AwsCustomResource handler - #38429

Draft
aemada-aws wants to merge 4 commits into
aws:mainfrom
aemada-aws:fix/awscr-response-retry-shared
Draft

fix(custom-resources): retry CloudFormation response PUT in AwsCustomResource handler#38429
aemada-aws wants to merge 4 commits into
aws:mainfrom
aemada-aws:fix/awscr-response-retry-shared

Conversation

@aemada-aws

@aemada-aws aemada-aws commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Issue

Internal ref: CFN-118294 (no public issue linked yet — draft).

Reason for this change

The AwsCustomResource handler's respond() sends the CloudFormation response with a single, un-retried https PUT to the pre-signed S3 response URL, and passes resolve directly as the response callback so the HTTP status code is never inspected. A transient PUT failure or a non-2xx response is silently swallowed: CloudFormation never receives the response and waits out its ~1 hour timeout, even though the function already logged a SUCCESS payload and exited cleanly. The log-retention handler has the identical un-retried respond() pattern and the same latent failure mode.

Description of changes

Add a small shared module, lib/shared/http-response.ts, mirroring the custom resource provider framework runtime:

  • withRetries (exponential backoff with jitter) + DEFAULT_RESPONSE_RETRY_OPTIONS (5 attempts, 1s base).
  • httpRequest consumes the response stream, rejects on a >= 400 status code (and on socket errors), and resolves only on a successful response.

Both bundled handlers that PUT their own CloudFormation response now use it:

  • custom-resources/aws-custom-resource-handler (respond()).
  • aws-logs/log-retention-handler (respond()).

respond() now returns withRetries(DEFAULT_RESPONSE_RETRY_OPTIONS, httpRequest)(requestOptions, responseBody) in both.

Why not the nodejs-entrypoint handler too? It is packaged with minifyAndBundle: false — the framework copies its single index.js verbatim (it dynamically require()s the user handler, which esbuild can't bundle), so it cannot import a relative shared module and keeps its own equivalent copy. The two handlers changed here are esbuild-bundled, so the shared module is inlined at build time.

Considered but not included: a per-attempt socket timeout. A PUT that hangs with no response is only recovered by a socket timeout (retry alone won't help), but the provider-framework logic being mirrored here does not set one. Happy to add it if reviewers prefer.

Describe any new or updated permissions being added

None.

Description of how you validated changes

  • New test/shared/http-response.test.ts: withRetries (success / retry-then-succeed / exhaustion) and httpRequest (2xx resolves, >= 400 rejects, network error rejects).
  • Extended utils.test.ts with respond tests (mocking https): single PUT on success, retry-then-succeed, rejection after exhausting retries.
  • Updated the existing aws-sdk-v3-handler.test.ts https mock to supply a response object (statusCode: 200) now that the status code is inspected. The log-retention tests use nock (reply 200), so they pass unchanged.
  • Full package build (tsc -b + framework esbuild bundle + cdk-lint) and the complete package jest suite (333 tests) pass locally.

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

…Resource handler

The AwsCustomResource handler's respond() sent the CloudFormation response
with a single, un-retried https PUT to the pre-signed S3 response URL and
passed `resolve` directly as the response callback, so the HTTP status code
was never inspected. A transient PUT failure or a non-2xx response was
silently swallowed, so CloudFormation never received the response and waited
out its ~1 hour timeout even though the function logged SUCCESS and exited
cleanly.

Extract the retry + exponential-backoff and status-code-checking HTTP logic
(matching the provider framework runtime) into a shared module,
lib/shared/http-response.ts, and use it from respond(): retry on network
errors and >= 400 responses, treating only a successful response as success.

sim: CFN-118294
@github-actions github-actions Bot added the p2 label Jul 28, 2026
@aws-cdk-automation
aws-cdk-automation requested a review from a team July 28, 2026 11:54
@mergify mergify Bot added the contribution/core This is a PR that came from AWS. label Jul 28, 2026
@mergify
mergify Bot temporarily deployed to automation July 28, 2026 11:54 Inactive
@mergify
mergify Bot temporarily deployed to automation July 28, 2026 11:55 Inactive
@github-actions

Copy link
Copy Markdown
Contributor

👋 It looks like your PR description follows the template but is missing a valid issue number in the first section.

PRs without a linked issue will receive lower priority for review and merging. Please update the description to include a reference like Closes #123. If no existing issue matches your change, create one first.

@aws-cdk-automation aws-cdk-automation left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

…-retention handler too

The log-retention handler's respond() had the same single, un-retried https
PUT to the CloudFormation pre-signed S3 response URL as the AwsCustomResource
handler, with no HTTP status-code check. Point it at the shared
lib/shared/http-response.ts (withRetries + httpRequest) so both bundled
handlers retry the response PUT consistently, and hoist the shared default
retry options (DEFAULT_RESPONSE_RETRY_OPTIONS) into that module.

The nodejs-entrypoint handler intentionally keeps its own copy: it is packaged
with minifyAndBundle: false (copied verbatim, because it dynamically requires
the user handler), so it cannot import a relative shared module.

sim: CFN-118294
…s.ts

Move lib/shared/http-response.ts to lib/utils.ts (and the test to
test/utils.test.ts), and update the aws-custom-resource and log-retention
handlers to import from '../../utils'. No behavior change.

sim: CFN-118294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants