AmazonQFeatureDev: Handle canned errors during code generation #4537
Conversation
| retryable: false, | ||
| } | ||
| } | ||
|
|
||
| throw new ToolkitError('Code generation failed', { code: 'CodeGenFailed' }) |
There was a problem hiding this comment.
Won't we also want to know that code generation failed? I think this code will make it so the telemetry failure event never gets reported when we get a canned error
There was a problem hiding this comment.
This function is invoked from inside a amazonq_codeGenerationInvoke telemetry block, so I assume we log the failure if we throw inside the block.
For the canned error case that we don't throw, we set
telemetry.setCodeGenerationResult(codegenResult.codeGenerationStatus.status)
so we will have the information of generations that failed.
| @@ -411,9 +435,13 @@ export class FeatureDevController { | |||
| let session | |||
| try { | |||
| session = await this.sessionStorage.getSession(message.tabID) | |||
| if (session.state.codeGenerationResult?.result !== 'success') { | |||
| throw new ToolkitError('Invalid state in code generation') | |||
There was a problem hiding this comment.
Is it worth extending a toolkiterror and defining a code just in case these cases are being hit and sending this data to telemetry? I see a couple places like these in the changes that might give us some potential insights if something is going very wrong on the users end
There was a problem hiding this comment.
Yeah good point! Those pieces of code should normally not be reachable, so it makes sense for us to get special telemetry on them.
I'll extend the ToolkitError for this.
| @@ -144,7 +145,7 @@ export class FeatureDevController { | |||
| }) | |||
| } | |||
|
|
|||
| private async processChatItemVotedMessage(tabId: string, messageId: string, vote: string) { | |||
| private async processChatItemVotedMessage(tabId: string, _messageId: string, vote: string) { | |||
There was a problem hiding this comment.
probably just makes sense to remove it rather then underscore it if its not being used, but that can be a different PR
There was a problem hiding this comment.
question: Why so many model changes are needed for this handling?
There was a problem hiding this comment.
They're not all related to this change.
This json is auto generated, so this here is the latest version of the models.
| @@ -300,6 +306,29 @@ export class FeatureDevController { | |||
| this.messenger.sendAsyncEventProgress(tabID, false, undefined) | |||
| } | |||
|
|
|||
| private printFailedCodeGenMessage(session: Session, message: string, tabID: string, retryable: boolean) { | |||
There was a problem hiding this comment.
question: isn't this function missing the chat prompt blocking?
There was a problem hiding this comment.
The prompt is blocked on line 397. It's the last step of the function that calls this one.
| this.messenger.sendChatInputEnabled(tabID, false) | ||
| this.messenger.sendChatInputEnabled(tabID, enableInput) | ||
| if (enableInput) { | ||
| this.messenger.sendUpdatePlaceholder(tabID, 'How can this plan be improved?') |
There was a problem hiding this comment.
question: has this message been approved by johanna?
I think we need to synchronize to understand what message can be a follow up for retryable errors.
| const deletedFiles = session.state.deletedFiles ?? [] | ||
|
|
||
| if (session.state.codeGenerationResult?.result === 'pending') { | ||
| throw new ToolkitError('Unexpected state in code generation') |
There was a problem hiding this comment.
question: shouldn't here be thrown InvalidCodeGenStateError?
| if (session.state.codeGenerationResult?.result === 'failed') { | ||
| // enable the input if the error is not retryable. | ||
| // we're expecting a new prompt in this case | ||
| enableInput = !session.state.codeGenerationResult.retryable |
There was a problem hiding this comment.
question: will there be failed code generation results that are non retryable?
Only canned errors are at the moment handled and they are all non retryable.
Problem
FeatureDev's code generation will sometimes return canned errors with detailed information about the problems that happened in the process. These errors are not retryable and should be worked around by the customer, using the information provided.
Solution
This CR adds the support for these canned errors, and adds dynamic logic for showing or hiding the
Retrybutton whenever the code generation step fails.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.