Repository navigation
fix(lambda): recognise AWS SDK errors that lost their name - #5578
Open
Brend-Smits wants to merge 1 commit into
Open
Brend-Smits wants to merge 1 commit into
Brend-Smits wants to merge 1 commit into
Conversation
When the SDK error classes are not resolved (as with the duplicate @smithy/core and @aws-sdk/core copies in v8.0.0), a service failure reaches the lambdas as a plain Error: the name is "Error" and the code is only in `Code`, `__type` or the message. Every check keyed on `error.name` then missed it: - the runner group cache store treated a missing cache parameter as a failure, so scale-up terminated the launched instance and retried forever instead of resolving the group from GitHub; - request-level EC2 failures such as UnauthorizedOperation never matched scale_errors, so the message was dropped instead of retried; - the SSM runner config consumer stopped polling on a missing parameter and did not retry throttled deletes. Add awsErrorCodes() in a new aws-error-util lib, so storage-providers and compute-providers share one way to read the code from the name, `Code`, `__type` or a bare-code message along the cause chain. It is used for these checks, and requestFailureCodes() now also records those codes; existing codes such as aws-name:Error are unchanged. Add a test that fails when more than one copy of @smithy/core or @aws-sdk/core is reachable from the AWS clients, so a partial dependency bump cannot reintroduce the duplicates. Refs #5565, #5571 Signed-off-by: Brend Smits <brend.smits@philips.com>
Contributor
Dependency ReviewThe following issues were found:
License Issueslambdas/libs/compute-providers/package.json
lambdas/libs/storage-providers/package.json
OpenSSF Scorecard
Scanned Files
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the changes addressing unnamed AWS SDK errors.
No open issues 🎉
What changed in this PR
Makes Lambda error handling resilient when AWS SDK errors lose their names, addressing runner-group cache failures in #5571 and missed EC2 retry classifications in #5565.
Changes:
- Adds a shared helper that extracts AWS error codes from error fields and cause chains.
- Uses derived codes for SSM cache misses, configuration polling and retries, and EC2 failure classification.
- Adds regression tests and a dependency guard against duplicate AWS SDK core installations.
| File | Description |
|---|---|
lambdas/yarn.lock |
Registers the helper workspace and consumer dependencies. |
lambdas/libs/storage-providers/package.json |
Adds the shared error helper dependency. |
lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.ts |
Recognizes missing cache parameters despite lost error names. |
lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.test.ts |
Tests unnamed cache misses and propagation of other failures. |
lambdas/libs/storage-providers/aws/ssm/runner-config-consumer.ts |
Recognizes unnamed missing-parameter errors while polling. |
lambdas/libs/storage-providers/aws/ssm/runner-config-consumer.test.ts |
Tests unnamed polling, throttling, and access-error handling. |
lambdas/libs/storage-providers/aws/ssm/runner-config-consumer-common.ts |
Uses derived codes to identify retryable errors. |
lambdas/libs/compute-providers/package.json |
Adds the shared error helper dependency. |
lambdas/libs/compute-providers/aws/ec2/src/runners.ts |
Adds derived AWS codes to request failure classifications. |
lambdas/libs/compute-providers/aws/ec2/src/runners.test.ts |
Covers unnamed EC2 and AMI lookup failures, including retry classification. |
lambdas/libs/aws-ssm-util/src/index.test.ts |
Verifies preservation of SDK error causes and messages. |
lambdas/libs/aws-powertools-util/src/aws-sdk-single-instance.test.ts |
Guards against multiple reachable SDK core installations. |
lambdas/libs/aws-error-util/vitest.config.ts |
Sets full coverage thresholds for the helper. |
lambdas/libs/aws-error-util/tsconfig.json |
Configures the helper’s TypeScript source scope. |
lambdas/libs/aws-error-util/src/index.ts |
Extracts error codes with bounded, cycle-safe cause traversal. |
lambdas/libs/aws-error-util/src/index.test.ts |
Tests code extraction, input filtering, and traversal limits. |
lambdas/libs/aws-error-util/package.json |
Defines the helper workspace and development scripts. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In v8.0.0 the lambda bundles contained several copies of
@smithy/coreand@aws-sdk/core. The SDK registers service exceptions in aTypeRegistryowned by@smithy/coreand looks them up from@aws-sdk/core, so with duplicate copies the lookup missed and service failures reached the lambdas as a plainError: the name isErrorand the code is only inCode,__typeor the message. Every check keyed onerror.namethen missed the error.mainand v8.1.0 no longer have the duplicates, but nothing in the code or CI protects against them coming back, and the checks only trustedname.This PR makes those checks tolerant of that shape and adds a guard for the root cause:
@aws-github-runner/aws-error-utillib withawsErrorCodes(), which reads the code fromname(ignoring the genericError),Code,__type(with or without a namespace) or a message that is a bare code, along thecausechain. It lives in its own lib rather than inaws-powertools-util/aws-ssm-utilbecause many suites replace those modules with partial mocks, and sostorage-providersandcompute-providerscan share it.requestFailureCodes()also records the derived codes asaws-name:<code>, so request-level EC2 failures such asUnauthorizedOperationmatchscale_errorsinstead of being dropped as non-retryable (v8.0.0: EC2 request errors lose their name in the lambda bundles, so scale_errors never match request-level failures (UnauthorizedOperation) #5565). Existing codes, includingaws-name:Error, are unchanged.name-only check).@smithy/coreor@aws-sdk/coreis reachable from the AWS clients used by the workspaces, which is the check requested in v8.0.0: EC2 request errors lose their name in the lambda bundles, so scale_errors never match request-level failures (UnauthorizedOperation) #5565.A message only counts as a code when it is a single identifier, so ordinary messages such as
Request failed with a socket hang upare not turned into failure codes, and unnamedAccessDeniedException,ThrottlingExceptionorParameterVersionNotFounderrors are still treated as failures where they should be.Test Plan
create(CreateFleet, RunInstances dedicated host, AMI lookup, and the resulting retryable classification throughscale_errors), plus unit tests forawsErrorCodes()at 100% coverage. 11 of the new tests failed before the fix and pass now.aws-ssm-utiltests pin the error chain thatgetParameterreally produces (GetParameterErrorwith the SDK error ascause) so the fixtures match what the cache store sees.mainand fails on the v8.0.0 lockfile (13 copies of@smithy/coreand 10 of@aws-sdk/corereachable from the lambdas).yarn format-check,yarn lint,yarn test(12 projects, uncached) andyarn buildall pass inlambdas/.Related Issues
Fixes #5571. Related to #5565: this adds the guard against duplicate SDK cores that it asks for, and makes the affected checks robust.