Repository navigation
fix(bb-data): retry migrations while a scale-to-zero cluster resumes - #451
Merged
sharonyajain merged 11 commits intoSep 6, 2026
Conversation
…ws-devtools-labs#450) A Database with minCapacity: 0 auto-pauses after ~5 minutes idle. The first Data API call of a deploy wakes the cluster and fails with DatabaseResumingException while it resumes, so the migration custom resource failed and the stack rolled back. Deterministic, not a race: any deploy made more than ~5 minutes after the last database activity hit it. The migration Lambda already backed off (1s -> 30s, 8 attempts) while Aurora was coming up, but two layers kept the resume error out of that path: - DataApiEngine classified it as QueryFailed. Errors with no SQLState are classified by SDK exception name, and DatabaseResumingException was not in that list. It now maps to ConnectionFailed, alongside ServiceUnavailableException and InternalServerErrorException, so application code querying a paused cluster also sees a transient, retryable name. - The Lambda's predicate only matched raw SDK error names, which the engine has already rewritten by the time the error arrives. It now retries on ConnectionFailed; the raw SDK names stay for errors raised outside the engine, and the retry log line names the error (its absence was the reporter's diagnostic signal). Backoff limits are unchanged. The tests drive the error through the real engine rather than hand-building an SDK error, because a raw-error test passes without the engine-side fix while the bug survives.
🦋 Changeset detectedLatest commit: 2a8af10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
pranavosu
previously approved these changes
Aug 28, 2026
sarayev
reviewed
Sep 1, 2026
sarayev
reviewed
Sep 1, 2026
Contributor
|
Overall LGTM, two minor comments that are not blockers. Approving. |
sarayev
previously approved these changes
Sep 1, 2026
…ls-labs#451) Adds the three cases review asked for, all against the error as the engine actually throws it: - withRetry rethrows a non-retryable error on the first attempt, unwrapped, so CloudFormation surfaces the real SQL error rather than a timeout. - An 08xxx connection failure is retryable. This is deliberate: the trade is that a genuinely unreachable database takes ~2 minutes to fail the deploy. - The raw DatabaseResumingException / BadRequestException names are retryable without the engine. Nothing routes around DataApiEngine today, so these branches are defensive; the test documents that intent rather than leaving them untested. Also drops an unused `mock` import that predates this branch. No production code changes, so no changeset.
Contributor
Author
|
@sarayev @pranavosu Sorry for the extra round — your approval got dismissed by a follow-up push. Could you re-approve when you get a chance? |
sarayev
previously approved these changes
Sep 3, 2026
soberm
reviewed
Sep 3, 2026
soberm
reviewed
Sep 3, 2026
soberm
reviewed
Sep 3, 2026
…ames (aws-devtools-labs#451) `DatabaseResumingException` was spelled out in both the engine's classifier and the migration Lambda's fallback predicate, so a change to one copy would silently drop the other back to `QueryFailed` and regress aws-devtools-labs#450 at deploy time. Both sites now read `TRANSIENT_DATA_API_ERROR_NAMES` from `errors.ts`, which keeps error classification knowledge in one file. The constant is internal: the entry points re-export `DatabaseErrors` by name, so the public API report is unchanged. One behavior delta: the Lambda's fallback now also accepts raw `ServiceUnavailableException` / `InternalServerErrorException`, since it shares the engine's set. Nothing routes around the engine today, and treating those as retryable matches what the engine already does with them. The raw-name test covers all three. `BadRequestException` deliberately stays a separate check — the engine maps it to `QueryFailed`, so it does not belong in a transient set. Note this does not protect against AWS renaming an exception; that would still fall through to `QueryFailed`. It removes the drift-between-copies failure mode.
…ws-devtools-labs#451) `Communications link failure` is a JDBC message on a PostgreSQL-only BB, so it reads like dead code, but it is the only check that catches a not-yet-ready writer: the engine rewrites that error's name to `QueryFailed` and leaves the message intact. The comment records that so it does not get removed as PostgreSQL-irrelevant.
…evtools-labs#451) `withRetry` re-throws the last error once the retryable attempts run out — the trailing `throw new Error('unreachable')` only satisfies the return type — so say so in the JSDoc. The error is re-thrown unwrapped, which is what puts the real failure in the CloudFormation message rather than a generic timeout.
soberm
approved these changes
Sep 4, 2026
sarayev
approved these changes
Sep 4, 2026
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
5 participants
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.
Problem
A
Databaseconfigured withminCapacity: 0(Aurora Serverless v2 scale-to-zero) auto-pauses after ~5 minutes idle. The first Data API call of a deploy wakes the cluster and fails withDatabaseResumingExceptionwhile it resumes, so the migration custom resource failed and the stack rolled back. Deterministic rather than a race: any deploy made more than ~5 minutes after the last database activity hit it.Issue #, if available: #450
Changes
The migration Lambda already backed off (1s → 30s, 8 attempts) while Aurora was coming up, but two layers kept the resume error out of that path, so a name-only fix in the Lambda would not have worked:
DataApiEngineclassified it asQueryFailed. Errors carrying no SQLState are classified by SDK exception name, andDatabaseResumingExceptionwas not in that list. It now maps toConnectionFailed, alongsideServiceUnavailableExceptionandInternalServerErrorException. Application code that queries a paused cluster sees the same, more accurate name — transient and safe to retry, unlikeQueryFailed.ConnectionFailed. The raw SDK names are kept for errors raised outside the engine, and the retry log line now names the error, since its absence was the reporter's diagnostic signal.Worth calling out for review:
ConnectionFailedalso covers SQLState08xxxconnection failures, which were previously not retried. In a migration Lambda that seems right — a transient connection failure is exactly what the backoff is for — but the cost is that a genuinely unreachable database now takes ~2 minutes to fail the deploy instead of failing immediately.DatabaseResumingExceptionis a behavior change for application code: a query against a paused cluster now throwsConnectionFailedExceptioninstead ofQueryFailedException. No types change, and the README documents the new meaning.withRetryand its predicate are now exported for testing.migration-lambda.tsis a Lambda entry point, not part of the package'sexportsmap, so this does not widen the public API surface (npm run check:apiconfirms the reports are unchanged).Validation
Unit tests, written to fail first:
data-api-engine.test.ts— the resume error maps toConnectionFailed.migration-lambda.test.ts— three predicate cases plus awithRetryloop test.The Lambda-side tests drive the error through a stub Data API client into the real
DataApiEngine.execute, rather than asserting on a hand-built SDK error. That matters here: a raw-error test passes without the engine-side change while the bug survives.No e2e test covers this combination. Local e2e runs on PGlite, so it cannot reach the Data API path at all, and a sandbox repro requires a
minCapacity: 0cluster idled past its auto-pause window. Instead it was verified manually against a real deployed stack (ap-northeast-1, Aurora PostgreSQL 16.13,minCapacity: 0) whose cluster had been paused for 30+ minutes (ServerlessDatabaseCapacityat 0), by adding a no-op migration to changemigrationsHash:The stack reached
UPDATE_COMPLETEinstead of rolling back. Note the error surfaces asConnectionFailedException, confirming the engine-side classification is the part that carries the signal.Local gates:
npm run build,npm run lint,npm run lint:deps,npm run check:api,npm test,npm run test:e2e:localall pass.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.