fix(amplify-category-api-e2e-tests): guard getS3Buckets so one dead region can't abort cleanup

[sarayev]

Description of changes

The scheduled amplify-category-api-cleanup-workflow job (cd packages/amplify-e2e-tests && yarn clean-e2e-resources) had failed 5/5 consecutive runs, leaving every e2e account uncleaned.

The per-region STS / CloudFormation / RDS listing calls in cleanup-e2e-resources.ts are already wrapped in skip-and-continue error handling, but the S3 sweep was not. When getS3Buckets reached the hard-down me-south-1 region, the per-bucket catch fell through to throw e after logging Irrecoverable error in getS3Buckets {code: ETIMEDOUT}. That rejection propagated up through cleanupAccount's Promise.all, so one unreachable region aborted the entire cleanup run for all ~13 e2e accounts. Stacks then accumulated until accounts hit the 2000-stack CloudFormation quota (L-0485CB21), which made the CreateApi canary hang (LimitExceededException swallowed by the CLI → 1800s watchdog kill) and fire alarms.

A bucket lives in exactly one region, so a region-level failure can only ever mean "this bucket is unprocessable right now" — it is never a reason to abandon the remaining accounts. This change mirrors the existing skip-and-continue pattern already used by getOrphanRdsInstances, getAmplifyApps and listStacks (same (opt-in region failure) ... Skipping. log style) and applies it to the S3 sweep:

• getS3Buckets — a per-bucket failure that is not NoSuchTagSet / NoSuchBucket / InvalidToken is now logged as a skip and the sweep continues, instead of rethrowing. The existing handling of those three cases is unchanged.
• getOrphanS3TestBuckets — bucket region resolution is guarded per bucket, so one bucket can no longer reject the surrounding Promise.all.
• listStaleTestBuckets (new, shared by both) — guards the previously unprotected top-level ListBuckets call so an account-level listing failure stays scoped to that account.
• The script's self-invocation is now behind require.main === module, so the module can be imported by unit tests without deleting real resources. yarn clean-e2e-resources behaviour is unchanged (verified below).

Deliberately not in scope: the set of swept regions is unchanged (removing me-south-1 is a separate decision), and no other cleanup behaviour was touched.

CDK / CloudFormation Parameters Changed

None.

Issue #, if available

Internal oncall ticket P492565382.

Description of how you validated changes

Added packages/amplify-e2e-tests/src/__unit_tests__/cleanup-e2e-resources.test.ts (6 tests), which mocks the @aws-sdk/client-s3 boundary and simulates me-south-1 failing with the exact ticket error (connect ETIMEDOUT, code: ETIMEDOUT, a name matching none of the specially handled S3 error names).

PASS src/__unit_tests__/cleanup-e2e-resources.test.ts
  getS3Buckets
    ✓ skips a bucket whose region is unreachable and still returns the buckets of every other region
    ✓ keeps sweeping the remaining regions when resolving a bucket region times out
    ✓ returns no buckets instead of rejecting when the account cannot be listed at all
    ✓ still records buckets that have no tag set, and still skips buckets with an InvalidToken failure
  getOrphanS3TestBuckets
    ✓ skips the bucket in the unreachable region and still returns the others
    ✓ returns no buckets instead of rejecting when the account cannot be listed at all

Tests:       6 passed, 6 total

The tests are proven non-vacuous: reverting only the getS3Buckets guard back to throw e (keeping everything else) makes exactly the two fault-isolation tests fail with connect ETIMEDOUT, reproducing the aborted run. Restoring the guard makes them pass again.

These tests needed a jest config that does not load the package's AWS-backed e2e runner/environment/global-setup, so jest.unit.config.js was added along with a test script (the package previously had none, so yarn test-ci skipped it entirely — these tests now actually run in CI). src/__unit_tests__ is excluded from the e2e jest config so yarn e2e is unaffected.

Also verified:

• prettier --check clean on all four changed files.
• eslint introduces zero new errors in cleanup-e2e-resources.ts (error set is byte-identical to origin/main apart from a line-number shift in a pre-existing no-shadow error); the new test file is eslint-clean.
• The entry point still works: ts-node ./src/cleanup-e2e-resources.ts --help exits 0 and prints the yargs help, confirming cleanup() is still invoked when run as a script.

Checklist

• PR description included
• yarn test passes
• E2E test run linked — not applicable; this changes only the standalone resource-cleanup script, no e2e test behaviour
• Tests are changed or added
• Relevant documentation is changed or added (and PR referenced)
• New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies — no new SDK calls; error handling only
• Any CDK or CloudFormation parameter changes are called out explicitly

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

[sarayev]

Heads-up for reviewers building this locally: ts-node/tsc on packages/amplify-e2e-tests/src/cleanup-e2e-resources.ts fails with a pre-existing TS2345 at the deleteS3Bucket(name, regionalizedS3Client) call, because a nested amplify-category-api-e2e-core/node_modules/@aws-sdk/client-s3 copy clashes with the root copy (S3ClientResolvedConfig missing protocol/protocolSettings).

I verified this is not introduced by this PR: pristine origin/main fails identically (same error, at the pre-shift line). That duplicate-dependency hygiene issue is why the new jest.unit.config.js sets ts-jest diagnostics: false, matching the convention the package's existing e2e jest config in package.json already uses. It looks worth a separate hygiene ticket, but it is out of scope here.

To run the new tests: cd packages/amplify-e2e-tests && yarn test

[Simone319]

**🟢 Minor
e?.name ?? e?.code resolves to name first, but a plain socket error like this ticket's ETIMEDOUT has name === 'Error' (truthy), so ?? never reaches .code and the log reads code Error instead of code ETIMEDOUT — swallowing the one detail this guard exists to catch. Flipping to e?.code ?? e?.name (or logging both, ${e?.name}/${e?.code}) fixes it. Here and in getOrphanS3TestBuckets there's no JSON.stringify(e) either, so unlike getS3Buckets the stack is lost too — worth including it so a persistent failure is diagnosable.

[sarayev]

Good catch — you're right, and this was a real defect in the guard's observability. Fixed in e22d3c15c.

I verified the claim before changing anything:

error	name	code	name ?? code (old)	code ?? name (new)
socket ETIMEDOUT	Error	ETIMEDOUT	Error ❌	ETIMEDOUT ✅
SDK InvalidClientTokenId	InvalidClientTokenId	undefined	InvalidClientTokenId	InvalidClientTokenId ✅

Exactly as you described: ?? only falls through on null/undefined, and name is the truthy string 'Error', so the ticket's ETIMEDOUT — the one detail this guard exists to surface — never made it into the log. I went with e?.code ?? e?.name rather than logging both, since the sentence already reads "…with code X" and, per the table, AWS SDK service errors carry no code, so the existing named paths (InvalidClientTokenId, NoSuchTagSet, InvalidToken) log character-for-character as they did before. Applied at all three sites: listStaleTestBuckets, the per-bucket catch in getOrphanS3TestBuckets, and the per-bucket catch in getS3Buckets.

On the serialized error — agreed the stack should be there, but I deliberately did not use JSON.stringify(e), because it would not actually have given you the stack:

JSON.stringify(socketError)
// {"errno":-110,"code":"ETIMEDOUT","syscall":"connect","address":"52.95.128.1","port":443}

message and stack are non-enumerable own properties of Error, so JSON.stringify silently drops both — which is the same class of "the useful detail is quietly gone" problem you flagged in the first place. Instead I pass the caught error as the second console.log argument, so Node's formatter prints the full stack and the enumerable system-error fields. That also means getOrphanS3TestBuckets, which previously logged no error at all, now does. Happy to switch to literal JSON.stringify(e) for consistency with the neighbouring CFN/RDS guards if you'd rather keep the style uniform — just say so and I'll push it.

The tests now pin the ordering so this can't silently regress. Each timeout test asserts both the rendered code and identity of the logged error:

expect(skipLog.message).toContain('failed with error with code ETIMEDOUT');
expect(skipLog.loggedError).toBe(thrown); // the real Error, stack intact — not a stringified husk

skipLogFor() throws a descriptive failure if no matching skip log was emitted, so these can't pass vacuously. Confirmed non-vacuous by reverting only the operand order: 5 of the 6 tests fail, each with failed with error with code Error. The sixth (NoSuchTagSet/InvalidToken) keeps passing, which is the evidence that the flip doesn't regress the SDK-named paths.

6 passed, 6 total with the fix in place; prettier clean; eslint unchanged from the pre-existing baseline (7 errors, all outside the edited regions).