Merge master into feature/emr

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/package.nls.json

@@ -310,6 +310,7 @@
     "AWS.amazonq.scans.projectScanInProgress": "Workspace review is in progress...",
     "AWS.amazonq.scans.fileScanInProgress": "File review is in progress...",
     "AWS.amazonq.scans.noGitRepo": "Your workspace is not in a git repository. I'll review your project files for security issues, and your in-flight changes for code quality issues.",
+    "AWS.amazonq.codefix.error.monthlyLimitReached": "Maximum code fix count reached for this month.",
     "AWS.amazonq.scans.severity": "Severity",
     "AWS.amazonq.scans.fileLocation": "File Location",
     "AWS.amazonq.scans.groupIssues": "Group Issues",

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'
@@ -681,7 +681,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,
@@ -738,25 +739,27 @@ export const generateFix = Commands.declare(
                     SecurityIssueProvider.instance.updateIssue(updatedIssue, targetFilePath)
                     SecurityIssueTreeViewProvider.instance.refresh()
                 } catch (err) {
+                    const error = err instanceof Error ? err : new TypeError('Unexpected error')
                     await updateSecurityIssueWebview({
                         issue: targetIssue,
                         isGenerateFixLoading: false,
-                        isGenerateFixError: true,
+                        generateFixError: getErrorMsg(error, 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/models/errors.ts

@@ -172,3 +172,13 @@ export class CodeFixJobStoppedError extends CodeFixError {
         super('Code fix generation stopped by user.', 'CodeFixCancelled', defaultCodeFixErrorMessage)
     }
 }
+
+export class MonthlyCodeFixLimitError extends CodeFixError {
+    constructor() {
+        super(
+            i18n('AWS.amazonq.codefix.error.monthlyLimitReached'),
+            MonthlyCodeFixLimitError.name,
+            defaultCodeFixErrorMessage
+        )
+    }
+}

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

@@ -6,13 +6,14 @@
 import { CodeWhispererUserClient } from '../indexNode'
 import * as CodeWhispererConstants from '../models/constants'
 import { codeFixState } from '../models/model'
-import { getLogger, sleep } from '../../shared'
+import { getLogger, isAwsError, sleep } from '../../shared'
 import { ArtifactMap, CreateUploadUrlRequest, DefaultCodeWhispererClient } from '../client/codewhisperer'
 import {
     CodeFixJobStoppedError,
     CodeFixJobTimedOutError,
     CreateCodeFixError,
     CreateUploadUrlError,
+    MonthlyCodeFixLimitError,
 } from '../models/errors'
 import { uploadArtifactToS3 } from './securityScanHandler'
 
@@ -28,8 +29,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 new CreateUploadUrlError(err.message)
     })
     getLogger().verbose(`CreateUploadUrlRequest requestId: ${srcResp.$response.requestId}`)
     getLogger().verbose(`Complete Getting presigned Url for uploading src context.`)
@@ -60,7 +61,10 @@ export async function createCodeFixJob(
     }
 
     const resp = await client.startCodeFixJob(req).catch((err) => {
-        getLogger().error(`Failed creating code fix job. Request id: ${err.requestId}`)
+        getLogger().error('Failed creating code fix job. %O', err)
+        if (isAwsError(err) && err.code === 'ThrottlingException' && err.message.includes('reached for this month')) {
+            throw new MonthlyCodeFixLimitError()
+        }
         throw new CreateCodeFixError()
     })
     getLogger().info(`AmazonQ generate fix Request id: ${resp.$response.requestId}`)

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