fix: add cdk error mapping for error 'no valid tables found' #2319
Conversation
🦋 Changeset detectedLatest commit: 88fc41d 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 |
| { | ||
| errorRegex: | ||
| /No valid tables found\. Make sure at least one table has a primary key\./, | ||
| humanReadableErrorMessage: | ||
| 'No valid tables found. Make sure at least one table has a primary key.', | ||
| resolutionMessage: | ||
| "If you are using the 'generate schema-from-database' command, make sure the database has at least one table with a primary key.", | ||
| errorName: 'GenerateSchemaFromDatabaseError', | ||
| classification: 'ERROR', | ||
| }, |
There was a problem hiding this comment.
is backend-deployer involved in generate schema-from-database command ?
There was a problem hiding this comment.
do we have error classification for non-deploy errors?
There was a problem hiding this comment.
There was a problem hiding this comment.
But we should ask data team to throw typed error for this instead of attempting to match strings.
There was a problem hiding this comment.
In cases like this I think we would want to catch and throw user error in SchemaGenerator or wherever makes sense. Unless we want to introduce error mapping for errors that can happen for CLI commands.
There was a problem hiding this comment.
In cases like this I think we would want to catch and throw user error in SchemaGenerator
Correct. But it would be best if data team gave us better error to map from.
| @@ -641,6 +641,13 @@ npm error enoent`, | |||
| errorName: 'CloudFormationDeletionError', | |||
| expectedDownstreamErrorMessage: undefined, | |||
| }, | |||
| { | |||
| errorMessage: `No valid tables found. Make sure at least one table has a primary key.`, | |||
There was a problem hiding this comment.
I have 2 questions:
- took a brief look into this error, it comes from the data team's repo, from here: https://github.com/aws-amplify/amplify-category-api/blob/e4d16911236c5751e02609c8c3b069543535883a/packages/amplify-graphql-schema-generator/src/ts-schema-generator/helpers.ts#L176
I think we have a few other non-cdk errors that are getting mapped in this file as well, just want to confirm that this is ok. - You are committing some changes to package.lock. Is that intended?
There was a problem hiding this comment.
looking into this more (thanks to Kamil's comment), it does seem that we don't have tooling in place for classifying anything other than what is coming from backend-deployer
also yes it seems package.lock is being updated on clean build but i can try again
|
closing for now, we need an alternative way to map these kind of errors |
Problem
At this time, 100% of errors with 'no valid tables found' error message are coming from the 'generate schema-from-database' command. This reclassifies that error as it is a user error, and resolution is to make sure a primary key is defined on at least one table in the database that is being imported/generated from.
see: https://docs.amplify.aws/javascript/build-a-backend/data/connect-to-existing-data-sources/connect-postgres-mysql-database/#step-2---generate-typescript-representation-of-your-database-schema
Issue number, if available:
Changes
Corresponding docs PR, if applicable:
Validation
Validated that this is not reproducible with standard deployment commands, and found that all errors were coming from the generate schema-from-database command, which requires primary keys on at least one table.
Checklist
run-e2elabel set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.