Skip to content

Commit f192573

Browse files
authored
fix(amazonq): codefix does not do error handling (aws#6307)
## Problem Code fix errors are not handled correctly, only shows a generic error notification and webview is not updated. ## Solution Surface the server error to the UI <img width="658" alt="image" src="https://github.com/user-attachments/assets/6e0e72d1-7de1-415d-a4d4-891d94e6d7ea" /> --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 5a97c89 commit f192573

File tree

9 files changed

+220
-189
lines changed

9 files changed

+220
-189
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "/review: Improved error handling for code fix operations"
4+
}

packages/core/package.nls.json

+1
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@
310310
"AWS.amazonq.scans.projectScanInProgress": "Workspace review is in progress...",
311311
"AWS.amazonq.scans.fileScanInProgress": "File review is in progress...",
312312
"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.",
313+
"AWS.amazonq.codefix.error.monthlyLimitReached": "Maximum code fix count reached for this month.",
313314
"AWS.amazonq.scans.severity": "Severity",
314315
"AWS.amazonq.scans.fileLocation": "File Location",
315316
"AWS.amazonq.scans.groupIssues": "Group Issues",

packages/core/resources/css/securityIssue.css

+7
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,13 @@ pre.center {
524524

525525
pre.error {
526526
color: var(--vscode-diffEditorOverview-removedForeground);
527+
background-color: var(--vscode-diffEditor-removedTextBackground);
528+
white-space: initial;
529+
}
530+
531+
a.cursor {
532+
cursor: pointer;
533+
text-decoration: none;
527534
}
528535

529536
.dot-typing {

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

+14-11
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import { once } from '../../shared/utilities/functionUtils'
5050
import { focusAmazonQPanel } from '../../codewhispererChat/commands/registerCommands'
5151
import { removeDiagnostic } from '../service/diagnosticsProvider'
5252
import { SsoAccessTokenProvider } from '../../auth/sso/ssoAccessTokenProvider'
53-
import { ToolkitError, getTelemetryReason, getTelemetryReasonDesc } from '../../shared/errors'
53+
import { ToolkitError, getErrorMsg, getTelemetryReason, getTelemetryReasonDesc } from '../../shared/errors'
5454
import { isRemoteWorkspace } from '../../shared/vscode/env'
5555
import { isBuilderIdConnection } from '../../auth/connection'
5656
import globals from '../../shared/extensionGlobals'
@@ -681,7 +681,8 @@ export const generateFix = Commands.declare(
681681
})
682682
await updateSecurityIssueWebview({
683683
isGenerateFixLoading: true,
684-
isGenerateFixError: false,
684+
// eslint-disable-next-line unicorn/no-null
685+
generateFixError: null,
685686
context: context.extensionContext,
686687
filePath: targetFilePath,
687688
shouldRefreshView: false,
@@ -738,25 +739,27 @@ export const generateFix = Commands.declare(
738739
SecurityIssueProvider.instance.updateIssue(updatedIssue, targetFilePath)
739740
SecurityIssueTreeViewProvider.instance.refresh()
740741
} catch (err) {
742+
const error = err instanceof Error ? err : new TypeError('Unexpected error')
741743
await updateSecurityIssueWebview({
742744
issue: targetIssue,
743745
isGenerateFixLoading: false,
744-
isGenerateFixError: true,
746+
generateFixError: getErrorMsg(error, true),
745747
filePath: targetFilePath,
746748
context: context.extensionContext,
747-
shouldRefreshView: true,
749+
shouldRefreshView: false,
748750
})
749751
SecurityIssueProvider.instance.updateIssue(targetIssue, targetFilePath)
750752
SecurityIssueTreeViewProvider.instance.refresh()
751753
throw err
754+
} finally {
755+
telemetry.record({
756+
component: targetSource,
757+
detectorId: targetIssue.detectorId,
758+
findingId: targetIssue.findingId,
759+
ruleId: targetIssue.ruleId,
760+
variant: refresh ? 'refresh' : undefined,
761+
})
752762
}
753-
telemetry.record({
754-
component: targetSource,
755-
detectorId: targetIssue.detectorId,
756-
findingId: targetIssue.findingId,
757-
ruleId: targetIssue.ruleId,
758-
variant: refresh ? 'refresh' : undefined,
759-
})
760763
})
761764
}
762765
)

packages/core/src/codewhisperer/models/errors.ts

+10
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,13 @@ export class CodeFixJobStoppedError extends CodeFixError {
172172
super('Code fix generation stopped by user.', 'CodeFixCancelled', defaultCodeFixErrorMessage)
173173
}
174174
}
175+
176+
export class MonthlyCodeFixLimitError extends CodeFixError {
177+
constructor() {
178+
super(
179+
i18n('AWS.amazonq.codefix.error.monthlyLimitReached'),
180+
MonthlyCodeFixLimitError.name,
181+
defaultCodeFixErrorMessage
182+
)
183+
}
184+
}

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
import { CodeWhispererUserClient } from '../indexNode'
77
import * as CodeWhispererConstants from '../models/constants'
88
import { codeFixState } from '../models/model'
9-
import { getLogger, sleep } from '../../shared'
9+
import { getLogger, isAwsError, sleep } from '../../shared'
1010
import { ArtifactMap, CreateUploadUrlRequest, DefaultCodeWhispererClient } from '../client/codewhisperer'
1111
import {
1212
CodeFixJobStoppedError,
1313
CodeFixJobTimedOutError,
1414
CreateCodeFixError,
1515
CreateUploadUrlError,
16+
MonthlyCodeFixLimitError,
1617
} from '../models/errors'
1718
import { uploadArtifactToS3 } from './securityScanHandler'
1819

@@ -28,8 +29,8 @@ export async function getPresignedUrlAndUpload(
2829
}
2930
getLogger().verbose(`Prepare for uploading src context...`)
3031
const srcResp = await client.createUploadUrl(srcReq).catch((err) => {
31-
getLogger().error(`Failed getting presigned url for uploading src context. Request id: ${err.requestId}`)
32-
throw new CreateUploadUrlError(err)
32+
getLogger().error('Failed getting presigned url for uploading src context. %O', err)
33+
throw new CreateUploadUrlError(err.message)
3334
})
3435
getLogger().verbose(`CreateUploadUrlRequest requestId: ${srcResp.$response.requestId}`)
3536
getLogger().verbose(`Complete Getting presigned Url for uploading src context.`)
@@ -60,7 +61,10 @@ export async function createCodeFixJob(
6061
}
6162

6263
const resp = await client.startCodeFixJob(req).catch((err) => {
63-
getLogger().error(`Failed creating code fix job. Request id: ${err.requestId}`)
64+
getLogger().error('Failed creating code fix job. %O', err)
65+
if (isAwsError(err) && err.code === 'ThrottlingException' && err.message.includes('reached for this month')) {
66+
throw new MonthlyCodeFixLimitError()
67+
}
6468
throw new CreateCodeFixError()
6569
})
6670
getLogger().info(`AmazonQ generate fix Request id: ${resp.$response.requestId}`)

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

+12-12
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ export class SecurityIssueWebview extends VueWebview {
2727
public readonly onChangeIssue = new vscode.EventEmitter<CodeScanIssue | undefined>()
2828
public readonly onChangeFilePath = new vscode.EventEmitter<string | undefined>()
2929
public readonly onChangeGenerateFixLoading = new vscode.EventEmitter<boolean>()
30-
public readonly onChangeGenerateFixError = new vscode.EventEmitter<boolean>()
30+
public readonly onChangeGenerateFixError = new vscode.EventEmitter<string | null | undefined>()
3131

3232
private issue: CodeScanIssue | undefined
3333
private filePath: string | undefined
3434
private isGenerateFixLoading: boolean = false
35-
private isGenerateFixError: boolean = false
35+
private generateFixError: string | null | undefined = undefined
3636

3737
public constructor() {
3838
super(SecurityIssueWebview.sourcePath)
@@ -99,13 +99,13 @@ export class SecurityIssueWebview extends VueWebview {
9999
this.onChangeGenerateFixLoading.fire(isGenerateFixLoading)
100100
}
101101

102-
public getIsGenerateFixError() {
103-
return this.isGenerateFixError
102+
public getGenerateFixError() {
103+
return this.generateFixError
104104
}
105105

106-
public setIsGenerateFixError(isGenerateFixError: boolean) {
107-
this.isGenerateFixError = isGenerateFixError
108-
this.onChangeGenerateFixError.fire(isGenerateFixError)
106+
public setGenerateFixError(generateFixError: string | null | undefined) {
107+
this.generateFixError = generateFixError
108+
this.onChangeGenerateFixError.fire(generateFixError)
109109
}
110110

111111
public generateFix() {
@@ -201,7 +201,7 @@ export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, iss
201201
activePanel.server.setIssue(issue)
202202
activePanel.server.setFilePath(filePath)
203203
activePanel.server.setIsGenerateFixLoading(false)
204-
activePanel.server.setIsGenerateFixError(false)
204+
activePanel.server.setGenerateFixError(undefined)
205205

206206
const webviewPanel = await activePanel.show({
207207
title: amazonqCodeIssueDetailsTabTitle,
@@ -247,15 +247,15 @@ type WebviewParams = {
247247
issue?: CodeScanIssue
248248
filePath?: string
249249
isGenerateFixLoading?: boolean
250-
isGenerateFixError?: boolean
250+
generateFixError?: string | null
251251
shouldRefreshView: boolean
252252
context: vscode.ExtensionContext
253253
}
254254
export async function updateSecurityIssueWebview({
255255
issue,
256256
filePath,
257257
isGenerateFixLoading,
258-
isGenerateFixError,
258+
generateFixError,
259259
shouldRefreshView,
260260
context,
261261
}: WebviewParams): Promise<void> {
@@ -271,8 +271,8 @@ export async function updateSecurityIssueWebview({
271271
if (isGenerateFixLoading !== undefined) {
272272
activePanel.server.setIsGenerateFixLoading(isGenerateFixLoading)
273273
}
274-
if (isGenerateFixError !== undefined) {
275-
activePanel.server.setIsGenerateFixError(isGenerateFixError)
274+
if (generateFixError !== undefined) {
275+
activePanel.server.setGenerateFixError(generateFixError)
276276
}
277277
if (shouldRefreshView && filePath && issue) {
278278
await showSecurityIssueWebview(context, issue, filePath)

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

+9-11
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,14 @@
4848
</div>
4949

5050
<div
51-
v-if="isFixAvailable || isGenerateFixLoading || isGenerateFixError || isFixDescriptionAvailable"
51+
v-if="isFixAvailable || isGenerateFixLoading || generateFixError || isFixDescriptionAvailable"
5252
ref="codeFixSection"
5353
>
5454
<hr />
5555

5656
<h3>Suggested code fix preview</h3>
5757
<pre v-if="isGenerateFixLoading" class="center"><div class="dot-typing"></div></pre>
58-
<pre v-if="isGenerateFixError" class="center error">
59-
Something went wrong. <a @click="regenerateFix">Retry</a>
60-
</pre>
58+
<pre v-if="generateFixError" class="center error">{{ generateFixError }}</pre>
6159
<div class="code-block">
6260
<span v-if="isFixAvailable" v-html="suggestedFixHtml"></span>
6361
<div v-if="isFixAvailable" class="code-diff-actions" ref="codeFixAction">
@@ -195,7 +193,7 @@ export default defineComponent({
195193
endLine: 0,
196194
relativePath: '',
197195
isGenerateFixLoading: false,
198-
isGenerateFixError: false,
196+
generateFixError: undefined as string | null | undefined,
199197
languageId: 'plaintext',
200198
fixedCode: '',
201199
referenceText: '',
@@ -218,8 +216,8 @@ export default defineComponent({
218216
const relativePath = await client.getRelativePath()
219217
this.updateRelativePath(relativePath)
220218
const isGenerateFixLoading = await client.getIsGenerateFixLoading()
221-
const isGenerateFixError = await client.getIsGenerateFixError()
222-
this.updateGenerateFixState(isGenerateFixLoading, isGenerateFixError)
219+
const generateFixError = await client.getGenerateFixError()
220+
this.updateGenerateFixState(isGenerateFixLoading, generateFixError)
223221
const languageId = await client.getLanguageId()
224222
if (languageId) {
225223
this.updateLanguageId(languageId)
@@ -249,16 +247,16 @@ export default defineComponent({
249247
this.isGenerateFixLoading = isGenerateFixLoading
250248
this.scrollTo('codeFixSection')
251249
})
252-
client.onChangeGenerateFixError((isGenerateFixError) => {
253-
this.isGenerateFixError = isGenerateFixError
250+
client.onChangeGenerateFixError((generateFixError) => {
251+
this.generateFixError = generateFixError
254252
})
255253
},
256254
updateRelativePath(relativePath: string) {
257255
this.relativePath = relativePath
258256
},
259-
updateGenerateFixState(isGenerateFixLoading: boolean, isGenerateFixError: boolean) {
257+
updateGenerateFixState(isGenerateFixLoading: boolean, generateFixError: string | null | undefined) {
260258
this.isGenerateFixLoading = isGenerateFixLoading
261-
this.isGenerateFixError = isGenerateFixError
259+
this.generateFixError = generateFixError
262260
},
263261
updateLanguageId(languageId: string) {
264262
this.languageId = languageId

0 commit comments

Comments
 (0)