fix(amazonq): codefix does not do error handling

Author: ctlai95

packages/amazonq/.changes/next-release/Bug Fix-38581ab6-e9f3-4b45-aef9-5f93e4a97a26.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "/review: Improved error handling for code fix operations"
+}

packages/core/resources/css/securityIssue.css

@@ -524,6 +524,13 @@ pre.center {
 
 pre.error {
     color: var(--vscode-diffEditorOverview-removedForeground);
+    background-color: var(--vscode-diffEditor-removedTextBackground);
+    white-space: initial;
+}
+
+a.cursor {
+    cursor: pointer;
+    text-decoration: none;
 }
 
 .dot-typing {

packages/core/src/codewhisperer/commands/basicCommands.ts

@@ -50,7 +50,7 @@ import { once } from '../../shared/utilities/functionUtils'
 import { focusAmazonQPanel } from '../../codewhispererChat/commands/registerCommands'
 import { removeDiagnostic } from '../service/diagnosticsProvider'
 import { SsoAccessTokenProvider } from '../../auth/sso/ssoAccessTokenProvider'
-import { ToolkitError, getTelemetryReason, getTelemetryReasonDesc } from '../../shared/errors'
+import { ToolkitError, getErrorMsg, getTelemetryReason, getTelemetryReasonDesc } from '../../shared/errors'
 import { isRemoteWorkspace } from '../../shared/vscode/env'
 import { isBuilderIdConnection } from '../../auth/connection'
 import globals from '../../shared/extensionGlobals'
@@ -67,6 +67,7 @@ import { startCodeFixGeneration } from './startCodeFixGeneration'
 import { DefaultAmazonQAppInitContext } from '../../amazonq/apps/initContext'
 import path from 'path'
 import { parsePatch } from 'diff'
+import type { AWSError } from 'aws-sdk'
 
 const MessageTimeOut = 5_000
 
@@ -677,7 +678,8 @@ export const generateFix = Commands.declare(
                         })
                     await updateSecurityIssueWebview({
                         isGenerateFixLoading: true,
-                        isGenerateFixError: false,
+                        // eslint-disable-next-line unicorn/no-null
+                        generateFixError: null,
                         context: context.extensionContext,
                         filePath: targetFilePath,
                         shouldRefreshView: false,
@@ -737,22 +739,23 @@ export const generateFix = Commands.declare(
                     await updateSecurityIssueWebview({
                         issue: targetIssue,
                         isGenerateFixLoading: false,
-                        isGenerateFixError: true,
+                        generateFixError: getErrorMsg(err as AWSError, true),
                         filePath: targetFilePath,
                         context: context.extensionContext,
-                        shouldRefreshView: true,
+                        shouldRefreshView: false,
                     })
                     SecurityIssueProvider.instance.updateIssue(targetIssue, targetFilePath)
                     SecurityIssueTreeViewProvider.instance.refresh()
                     throw err
+                } finally {
+                    telemetry.record({
+                        component: targetSource,
+                        detectorId: targetIssue.detectorId,
+                        findingId: targetIssue.findingId,
+                        ruleId: targetIssue.ruleId,
+                        variant: refresh ? 'refresh' : undefined,
+                    })
                 }
-                telemetry.record({
-                    component: targetSource,
-                    detectorId: targetIssue.detectorId,
-                    findingId: targetIssue.findingId,
-                    ruleId: targetIssue.ruleId,
-                    variant: refresh ? 'refresh' : undefined,
-                })
             })
         }
 )

packages/core/src/codewhisperer/service/codeFixHandler.ts

@@ -8,12 +8,7 @@ import * as CodeWhispererConstants from '../models/constants'
 import { codeFixState } from '../models/model'
 import { getLogger, sleep } from '../../shared'
 import { ArtifactMap, CreateUploadUrlRequest, DefaultCodeWhispererClient } from '../client/codewhisperer'
-import {
-    CodeFixJobStoppedError,
-    CodeFixJobTimedOutError,
-    CreateCodeFixError,
-    CreateUploadUrlError,
-} from '../models/errors'
+import { CodeFixJobStoppedError, CodeFixJobTimedOutError } from '../models/errors'
 import { uploadArtifactToS3 } from './securityScanHandler'
 
 export async function getPresignedUrlAndUpload(
@@ -28,8 +23,8 @@ export async function getPresignedUrlAndUpload(
     }
     getLogger().verbose(`Prepare for uploading src context...`)
     const srcResp = await client.createUploadUrl(srcReq).catch((err) => {
-        getLogger().error(`Failed getting presigned url for uploading src context. Request id: ${err.requestId}`)
-        throw new CreateUploadUrlError(err)
+        getLogger().error('Failed getting presigned url for uploading src context. %O', err)
+        throw err
     })
     getLogger().verbose(`CreateUploadUrlRequest requestId: ${srcResp.$response.requestId}`)
     getLogger().verbose(`Complete Getting presigned Url for uploading src context.`)
@@ -60,8 +55,8 @@ export async function createCodeFixJob(
     }
 
     const resp = await client.startCodeFixJob(req).catch((err) => {
-        getLogger().error(`Failed creating code fix job. Request id: ${err.requestId}`)
-        throw new CreateCodeFixError()
+        getLogger().error('Failed creating code fix job. %O', err)
+        throw err
     })
     getLogger().info(`AmazonQ generate fix Request id: ${resp.$response.requestId}`)
     return resp

packages/core/src/codewhisperer/views/securityIssue/securityIssueWebview.ts

@@ -27,12 +27,12 @@ export class SecurityIssueWebview extends VueWebview {
     public readonly onChangeIssue = new vscode.EventEmitter<CodeScanIssue | undefined>()
     public readonly onChangeFilePath = new vscode.EventEmitter<string | undefined>()
     public readonly onChangeGenerateFixLoading = new vscode.EventEmitter<boolean>()
-    public readonly onChangeGenerateFixError = new vscode.EventEmitter<boolean>()
+    public readonly onChangeGenerateFixError = new vscode.EventEmitter<string | null | undefined>()
 
     private issue: CodeScanIssue | undefined
     private filePath: string | undefined
     private isGenerateFixLoading: boolean = false
-    private isGenerateFixError: boolean = false
+    private generateFixError: string | null | undefined = undefined
 
     public constructor() {
         super(SecurityIssueWebview.sourcePath)
@@ -99,13 +99,13 @@ export class SecurityIssueWebview extends VueWebview {
         this.onChangeGenerateFixLoading.fire(isGenerateFixLoading)
     }
 
-    public getIsGenerateFixError() {
-        return this.isGenerateFixError
+    public getGenerateFixError() {
+        return this.generateFixError
     }
 
-    public setIsGenerateFixError(isGenerateFixError: boolean) {
-        this.isGenerateFixError = isGenerateFixError
-        this.onChangeGenerateFixError.fire(isGenerateFixError)
+    public setGenerateFixError(generateFixError: string | null | undefined) {
+        this.generateFixError = generateFixError
+        this.onChangeGenerateFixError.fire(generateFixError)
     }
 
     public generateFix() {
@@ -201,7 +201,7 @@ export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, iss
     activePanel.server.setIssue(issue)
     activePanel.server.setFilePath(filePath)
     activePanel.server.setIsGenerateFixLoading(false)
-    activePanel.server.setIsGenerateFixError(false)
+    activePanel.server.setGenerateFixError(undefined)
 
     const webviewPanel = await activePanel.show({
         title: amazonqCodeIssueDetailsTabTitle,
@@ -247,15 +247,15 @@ type WebviewParams = {
     issue?: CodeScanIssue
     filePath?: string
     isGenerateFixLoading?: boolean
-    isGenerateFixError?: boolean
+    generateFixError?: string | null
     shouldRefreshView: boolean
     context: vscode.ExtensionContext
 }
 export async function updateSecurityIssueWebview({
     issue,
     filePath,
     isGenerateFixLoading,
-    isGenerateFixError,
+    generateFixError,
     shouldRefreshView,
     context,
 }: WebviewParams): Promise<void> {
@@ -271,8 +271,8 @@ export async function updateSecurityIssueWebview({
     if (isGenerateFixLoading !== undefined) {
         activePanel.server.setIsGenerateFixLoading(isGenerateFixLoading)
     }
-    if (isGenerateFixError !== undefined) {
-        activePanel.server.setIsGenerateFixError(isGenerateFixError)
+    if (generateFixError !== undefined) {
+        activePanel.server.setGenerateFixError(generateFixError)
     }
     if (shouldRefreshView && filePath && issue) {
         await showSecurityIssueWebview(context, issue, filePath)

packages/core/src/codewhisperer/views/securityIssue/vue/root.vue

@@ -48,16 +48,14 @@
         </div>
 
         <div
-            v-if="isFixAvailable || isGenerateFixLoading || isGenerateFixError || isFixDescriptionAvailable"
+            v-if="isFixAvailable || isGenerateFixLoading || generateFixError || isFixDescriptionAvailable"
             ref="codeFixSection"
         >
             <hr />
 
             <h3>Suggested code fix preview</h3>
             <pre v-if="isGenerateFixLoading" class="center"><div class="dot-typing"></div></pre>
-            <pre v-if="isGenerateFixError" class="center error">
-                Something went wrong. <a @click="regenerateFix">Retry</a>
-            </pre>
+            <pre v-if="generateFixError" class="center error">{{ generateFixError }}</pre>
             <div class="code-block">
                 <span v-if="isFixAvailable" v-html="suggestedFixHtml"></span>
                 <div v-if="isFixAvailable" class="code-diff-actions" ref="codeFixAction">
@@ -195,7 +193,7 @@ export default defineComponent({
             endLine: 0,
             relativePath: '',
             isGenerateFixLoading: false,
-            isGenerateFixError: false,
+            generateFixError: undefined as string | null | undefined,
             languageId: 'plaintext',
             fixedCode: '',
             referenceText: '',
@@ -218,8 +216,8 @@ export default defineComponent({
             const relativePath = await client.getRelativePath()
             this.updateRelativePath(relativePath)
             const isGenerateFixLoading = await client.getIsGenerateFixLoading()
-            const isGenerateFixError = await client.getIsGenerateFixError()
-            this.updateGenerateFixState(isGenerateFixLoading, isGenerateFixError)
+            const generateFixError = await client.getGenerateFixError()
+            this.updateGenerateFixState(isGenerateFixLoading, generateFixError)
             const languageId = await client.getLanguageId()
             if (languageId) {
                 this.updateLanguageId(languageId)
@@ -249,16 +247,16 @@ export default defineComponent({
                 this.isGenerateFixLoading = isGenerateFixLoading
                 this.scrollTo('codeFixSection')
             })
-            client.onChangeGenerateFixError((isGenerateFixError) => {
-                this.isGenerateFixError = isGenerateFixError
+            client.onChangeGenerateFixError((generateFixError) => {
+                this.generateFixError = generateFixError
             })
         },
         updateRelativePath(relativePath: string) {
             this.relativePath = relativePath
         },
-        updateGenerateFixState(isGenerateFixLoading: boolean, isGenerateFixError: boolean) {
+        updateGenerateFixState(isGenerateFixLoading: boolean, generateFixError: string | null | undefined) {
             this.isGenerateFixLoading = isGenerateFixLoading
-            this.isGenerateFixError = isGenerateFixError
+            this.generateFixError = generateFixError
         },
         updateLanguageId(languageId: string) {
             this.languageId = languageId