-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error mapping for Amplify app not found in specified region #2313
base: main
Are you sure you want to change the base?
Changes from 4 commits
16e96d7
f0e0bbe
645e1e0
28a37d6
afb801e
a2c4031
a5692a0
6e73f46
33b2ccd
536dad0
eab450f
b400709
37b62bb
3236c1e
31b406a
357babb
5c9b41f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@aws-amplify/backend-cli': patch | ||
--- | ||
|
||
Added error mapping for app name not available in the region |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ import { | |||||||||||||
GenerateModelsOptions, | ||||||||||||||
} from '@aws-amplify/model-generator'; | ||||||||||||||
import { ArgumentsKebabCase } from '../../../kebab_case.js'; | ||||||||||||||
import { AmplifyUserError } from '@aws-amplify/platform-core'; | ||||||||||||||
|
||||||||||||||
type GenerateOptions = | ||||||||||||||
| GenerateGraphqlCodegenOptions | ||||||||||||||
|
@@ -89,20 +90,36 @@ export class GenerateGraphqlClientCodeCommand | |||||||||||||
handler = async ( | ||||||||||||||
args: ArgumentsCamelCase<GenerateGraphqlClientCodeCommandOptions> | ||||||||||||||
): Promise<void> => { | ||||||||||||||
const backendIdentifier = | ||||||||||||||
await this.backendIdentifierResolver.resolveDeployedBackendIdentifier( | ||||||||||||||
args | ||||||||||||||
); | ||||||||||||||
const out = this.getOutDir(args); | ||||||||||||||
const format = args.format ?? GenerateApiCodeFormat.GRAPHQL_CODEGEN; | ||||||||||||||
const formatParams = this.formatParamBuilders[format](args); | ||||||||||||||
try { | ||||||||||||||
const backendIdentifier = | ||||||||||||||
await this.backendIdentifierResolver.resolveDeployedBackendIdentifier( | ||||||||||||||
args | ||||||||||||||
); | ||||||||||||||
const out = this.getOutDir(args); | ||||||||||||||
const format = args.format ?? GenerateApiCodeFormat.GRAPHQL_CODEGEN; | ||||||||||||||
const formatParams = this.formatParamBuilders[format](args); | ||||||||||||||
|
||||||||||||||
const result = await this.generateApiCodeAdapter.invokeGenerateApiCode({ | ||||||||||||||
...backendIdentifier, | ||||||||||||||
...formatParams, | ||||||||||||||
} as unknown as InvokeGenerateApiCodeProps); | ||||||||||||||
|
||||||||||||||
const result = await this.generateApiCodeAdapter.invokeGenerateApiCode({ | ||||||||||||||
...backendIdentifier, | ||||||||||||||
...formatParams, | ||||||||||||||
} as unknown as InvokeGenerateApiCodeProps); | ||||||||||||||
await result.writeToDirectory(out); | ||||||||||||||
} catch (error) { | ||||||||||||||
const appNotFoundMatch = (error as Error).message.match( | ||||||||||||||
/No apps found with name (?<appName>.*) in region (?<region>.*)/ | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
await result.writeToDirectory(out); | ||||||||||||||
if (appNotFoundMatch?.groups) { | ||||||||||||||
const { appName, region } = appNotFoundMatch.groups; | ||||||||||||||
throw new AmplifyUserError('AmplifyAppNotFoundError', { | ||||||||||||||
message: `No Amplify app found with name "${appName}" in region "${region}".`, | ||||||||||||||
resolution: `Ensure that an Amplify app named "${appName}" exists in the "${region}" region.`, | ||||||||||||||
}); | ||||||||||||||
} | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My question is still not answered. How are you able to reproduce this and validate it? I don't see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have moved the try-catch wrapper to a common resolver where all the generate command finally lands.I have updated the repro steps in the description. Seems to be happening only if the --branch option is set without the AppID There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is mostly happening in the console (based on the isCI flag in the telemetry) where customers might not be explicitly providing these values and the region should match if customers are using the Amplify Console to download the output file.
Yes, when only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its a valid concern, since appName is internally derived. When I tested out in the right region it works every time. When I'm working in the wrong region it fails. So we can safely call it as UserError. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tested this in the Console? I understand you can reproduce this in the CLI, but the fact that this issue happens in CI points to the issue could be something else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||
// Re-throw other errors | ||||||||||||||
throw error; | ||||||||||||||
} | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of regex matching, we should update the error thrown to have contextual information. E.g. see this example here in the
deployed-backend-client
packageamplify-backend/packages/deployed-backend-client/src/backend_output_client_factory.ts
Lines 9 to 21 in 3838849
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated