feat(site launch): Allow for ops to remove timed out domain associations #720
Conversation
This file contains 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
technically we mock both create and get calls, changing the var name to reflect this
redundant associations
kishore03109
changed the title
kishore03109
commented
Apr 20, 2023
| ) { | ||
| if (!this.mockDomainAssociations.has(input.domainName)) { | ||
| throw new AmplifyError( | ||
| `NotFoundException: Domain association ${input.domainName} not found.` |
There was a problem hiding this comment.
This mocks the behaviour of the real amplify client and throws an exception if domain hasnt been created yet
kishore03109
commented
Apr 20, 2023
| getDomainAssociationCommandInput | ||
| ) | ||
| const hasDomainAssociationFailed = | ||
| getDomainAssociationResult?.domainAssociation?.domainStatus === "FAILED" |
There was a problem hiding this comment.
Refer here for types of domain association result
| @@ -159,6 +161,41 @@ export class LaunchesService { | |||
| throw siteIdResult.error | |||
| } | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
Wrapped in a try catch since AmplifySDK will throw an (expected) not found error, which is still the happy path
alexanderleegs
approved these changes
Apr 20, 2023
mergify
bot
deleted the
IS-93-Ensure-site-launch-form-can-handle-failures-or-it-requires-manual-deletes-from-amplify-db
branch
Merged
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.
Problem
If we try to associate the same domain twice, Amplify will generally throw a
BadRequestException: One or more domains requested are already associated with another Amplify app, which can inform the user to delete + retry.However, this did not hold true if the domain association process timed out, in which case Amplify did not throw an error, and neither did it create a new Domain Association during the
CreateDomainAssociationcall.To replicate the bug, this sequence of errors would need to happen:
One would think that the email received in step 5 is the most recent values, but it is in fact a lingering DomainAssociation that has yet to be deleted.
Closes IS93
Solution
Before calling a
CreateDomainAssociation, delete any failed domain associations for the application.Breaking Changes
Testing
While I have tested it manually on the
kishore-testapp, it is a bit hard to easily replicate this test this as it requires the existence of an App timeout over 24 hours. :/