Skip to content

Commit 4d07f18

Browse files
ashishrp-awskaranA-aws
authored andcommitted
telemetry(amazonq): unit test generation aws#6386
## Problem Incorrect handling of errors for 4xx and 5xx on IDE. Proper error messages for users and telemetry needs to differentiate between service errors. ## Solution - Adding 4XX vs 5XX `httpStatusCode` field to `amazonq_utgGenerateTests` event. - Improving error handling in unit test generation.
1 parent d8b34f1 commit 4d07f18

File tree

9 files changed

+201
-83
lines changed

9 files changed

+201
-83
lines changed

packages/core/src/amazonqTest/chat/controller/controller.ts

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
TelemetryHelper,
2121
TestGenerationBuildStep,
2222
testGenState,
23+
tooManyRequestErrorMessage,
2324
unitTestGenerationCancelMessage,
2425
UserWrittenCodeTracker,
2526
} from '../../../codewhisperer'
@@ -242,72 +243,76 @@ export class TestController {
242243
// eslint-disable-next-line unicorn/no-null
243244
this.messenger.sendUpdatePromptProgress(data.tabID, null)
244245
const session = this.sessionStorage.getSession()
245-
const isCancel = data.error.message === unitTestGenerationCancelMessage
246-
246+
const isCancel = data.error.uiMessage === unitTestGenerationCancelMessage
247+
let telemetryErrorMessage = getTelemetryReasonDesc(data.error)
248+
if (session.stopIteration) {
249+
telemetryErrorMessage = getTelemetryReasonDesc(data.error.uiMessage.replaceAll('```', ''))
250+
}
247251
TelemetryHelper.instance.sendTestGenerationToolkitEvent(
248252
session,
249253
true,
250254
true,
251255
isCancel ? 'Cancelled' : 'Failed',
252256
session.startTestGenerationRequestId,
253257
performance.now() - session.testGenerationStartTime,
254-
getTelemetryReasonDesc(data.error),
258+
telemetryErrorMessage,
255259
session.isCodeBlockSelected,
256260
session.artifactsUploadDuration,
257261
session.srcPayloadSize,
258262
session.srcZipFileSize
259263
)
260-
261264
if (session.stopIteration) {
262265
// Error from Science
263-
this.messenger.sendMessage(data.error.message.replaceAll('```', ''), data.tabID, 'answer')
266+
this.messenger.sendMessage(data.error.uiMessage.replaceAll('```', ''), data.tabID, 'answer')
264267
} else {
265268
isCancel
266-
? this.messenger.sendMessage(data.error.message, data.tabID, 'answer')
269+
? this.messenger.sendMessage(data.error.uiMessage, data.tabID, 'answer')
267270
: this.sendErrorMessage(data)
268271
}
269272
await this.sessionCleanUp()
270273
return
271274
}
272275
// Client side error messages
273-
private sendErrorMessage(data: { tabID: string; error: { code: string; message: string } }) {
276+
private sendErrorMessage(data: {
277+
tabID: string
278+
error: { uiMessage: string; message: string; code: string; statusCode: string }
279+
}) {
274280
const { error, tabID } = data
275281

282+
// If user reached monthly limit for builderId
283+
if (error.code === 'CreateTestJobError') {
284+
if (error.message.includes(CodeWhispererConstants.utgLimitReached)) {
285+
getLogger().error('Monthly quota reached for QSDA actions.')
286+
return this.messenger.sendMessage(
287+
i18n('AWS.amazonq.featureDev.error.monthlyLimitReached'),
288+
tabID,
289+
'answer'
290+
)
291+
}
292+
if (error.message.includes('Too many requests')) {
293+
getLogger().error(error.message)
294+
return this.messenger.sendErrorMessage(tooManyRequestErrorMessage, tabID)
295+
}
296+
}
276297
if (isAwsError(error)) {
277298
if (error.code === 'ThrottlingException') {
278-
// TODO: use the explicitly modeled exception reason for quota vs throttle
279-
if (error.message.includes(CodeWhispererConstants.utgLimitReached)) {
280-
getLogger().error('Monthly quota reached for QSDA actions.')
281-
return this.messenger.sendMessage(
282-
i18n('AWS.amazonq.featureDev.error.monthlyLimitReached'),
283-
tabID,
284-
'answer'
285-
)
286-
} else {
287-
getLogger().error('Too many requests.')
288-
// TODO: move to constants file
289-
this.messenger.sendErrorMessage('Too many requests. Please wait before retrying.', tabID)
290-
}
291-
} else {
292-
// other service errors:
293-
// AccessDeniedException - should not happen because access is validated before this point in the client
294-
// ValidationException - shouldn't happen because client should not send malformed requests
295-
// ConflictException - should not happen because the client will maintain proper state
296-
// InternalServerException - shouldn't happen but needs to be caught
297-
getLogger().error('Other error message: %s', error.message)
298-
this.messenger.sendErrorMessage(
299-
'Encountered an unexpected error when generating tests. Please try again',
300-
tabID
301-
)
299+
// TODO: use the explicitly modeled exception reason for quota vs throttle{
300+
getLogger().error(error.message)
301+
this.messenger.sendErrorMessage(tooManyRequestErrorMessage, tabID)
302+
return
302303
}
303-
} else {
304-
// other unexpected errors (TODO enumerate all other failure cases)
304+
// other service errors:
305+
// AccessDeniedException - should not happen because access is validated before this point in the client
306+
// ValidationException - shouldn't happen because client should not send malformed requests
307+
// ConflictException - should not happen because the client will maintain proper state
308+
// InternalServerException - shouldn't happen but needs to be caught
305309
getLogger().error('Other error message: %s', error.message)
306-
this.messenger.sendErrorMessage(
307-
'Encountered an unexpected error when generating tests. Please try again',
308-
tabID
309-
)
310+
this.messenger.sendErrorMessage('', tabID)
311+
return
310312
}
313+
// other unexpected errors (TODO enumerate all other failure cases)
314+
getLogger().error('Other error message: %s', error.uiMessage)
315+
this.messenger.sendErrorMessage('', tabID)
311316
}
312317

313318
// This function handles actions if user clicked on any Button one of these cases will be executed
@@ -730,6 +735,9 @@ export class TestController {
730735
// this.messenger.sendMessage('Accepted', message.tabID, 'prompt')
731736
telemetry.ui_click.emit({ elementId: 'unitTestGeneration_acceptDiff' })
732737

738+
getLogger().info(
739+
`Generated unit tests are accepted for ${session.fileLanguage ?? 'plaintext'} language with jobId: ${session.listOfTestGenerationJobId[0]}, jobGroupName: ${session.testGenerationJobGroupName}, result: Succeeded`
740+
)
733741
TelemetryHelper.instance.sendTestGenerationToolkitEvent(
734742
session,
735743
true,
@@ -751,7 +759,6 @@ export class TestController {
751759
)
752760

753761
await this.endSession(message, FollowUpTypes.SkipBuildAndFinish)
754-
await this.sessionCleanUp()
755762
return
756763

757764
if (session.listOfTestGenerationJobId.length === 1) {
@@ -876,16 +883,12 @@ export class TestController {
876883
session.numberOfTestsGenerated,
877884
session.linesOfCodeGenerated
878885
)
879-
880886
telemetry.ui_click.emit({ elementId: 'unitTestGeneration_rejectDiff' })
881887
}
882888

883889
await this.sessionCleanUp()
884-
// TODO: revert 'Accepted' to 'Skip build and finish' once supported
885-
const message = step === FollowUpTypes.RejectCode ? 'Rejected' : 'Accepted'
886890

887-
this.messenger.sendMessage(message, data.tabID, 'prompt')
888-
this.messenger.sendMessage(`Unit test generation workflow is completed.`, data.tabID, 'answer')
891+
// this.messenger.sendMessage(`Unit test generation workflow is completed.`, data.tabID, 'answer')
889892
this.messenger.sendChatInputEnabled(data.tabID, true)
890893
return
891894
}
@@ -1320,8 +1323,18 @@ export class TestController {
13201323
'Deleting output.log and temp result directory. testGenerationLogsDir: %s',
13211324
testGenerationLogsDir
13221325
)
1323-
await fs.delete(path.join(testGenerationLogsDir, 'output.log'))
1324-
await fs.delete(this.tempResultDirPath, { recursive: true })
1326+
const outputLogPath = path.join(testGenerationLogsDir, 'output.log')
1327+
if (await fs.existsFile(outputLogPath)) {
1328+
await fs.delete(outputLogPath)
1329+
}
1330+
if (
1331+
await fs
1332+
.stat(this.tempResultDirPath)
1333+
.then(() => true)
1334+
.catch(() => false)
1335+
) {
1336+
await fs.delete(this.tempResultDirPath, { recursive: true })
1337+
}
13251338
}
13261339

13271340
// TODO: return build command when product approves

packages/core/src/amazonqTest/chat/controller/messenger/messenger.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,21 @@ export class Messenger {
282282
'Cancelled',
283283
messageId,
284284
performance.now() - session.testGenerationStartTime,
285-
getTelemetryReasonDesc(CodeWhispererConstants.unitTestGenerationCancelMessage)
285+
getTelemetryReasonDesc(
286+
`TestGenCancelled: ${CodeWhispererConstants.unitTestGenerationCancelMessage}`
287+
),
288+
undefined,
289+
undefined,
290+
undefined,
291+
undefined,
292+
undefined,
293+
undefined,
294+
undefined,
295+
undefined,
296+
undefined,
297+
undefined,
298+
'TestGenCancelled'
286299
)
287-
288300
this.dispatcher.sendUpdatePromptProgress(
289301
new UpdatePromptProgressMessage(tabID, cancellingProgressField)
290302
)
@@ -296,9 +308,9 @@ export class Messenger {
296308
fileInWorkspace,
297309
'Succeeded',
298310
messageId,
299-
performance.now() - session.testGenerationStartTime
311+
performance.now() - session.testGenerationStartTime,
312+
undefined
300313
)
301-
302314
this.dispatcher.sendUpdatePromptProgress(
303315
new UpdatePromptProgressMessage(tabID, testGenCompletedField)
304316
)
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
import { ToolkitError } from '../shared/errors'
6+
7+
export const technicalErrorCustomerFacingMessage =
8+
'I am experiencing technical difficulties at the moment. Please try again in a few minutes.'
9+
const defaultTestGenErrorMessage = 'Amazon Q encountered an error while generating tests. Try again later.'
10+
export class TestGenError extends ToolkitError {
11+
constructor(
12+
error: string,
13+
code: string,
14+
public uiMessage: string
15+
) {
16+
super(error, { code })
17+
}
18+
}
19+
export class ProjectZipError extends TestGenError {
20+
constructor(error: string) {
21+
super(error, 'ProjectZipError', defaultTestGenErrorMessage)
22+
}
23+
}
24+
export class InvalidSourceZipError extends TestGenError {
25+
constructor() {
26+
super('Failed to create valid source zip', 'InvalidSourceZipError', defaultTestGenErrorMessage)
27+
}
28+
}
29+
export class CreateUploadUrlError extends TestGenError {
30+
constructor(errorMessage: string) {
31+
super(errorMessage, 'CreateUploadUrlError', technicalErrorCustomerFacingMessage)
32+
}
33+
}
34+
export class UploadTestArtifactToS3Error extends TestGenError {
35+
constructor(error: string) {
36+
super(error, 'UploadTestArtifactToS3Error', technicalErrorCustomerFacingMessage)
37+
}
38+
}
39+
export class CreateTestJobError extends TestGenError {
40+
constructor(error: string) {
41+
super(error, 'CreateTestJobError', technicalErrorCustomerFacingMessage)
42+
}
43+
}
44+
export class TestGenTimedOutError extends TestGenError {
45+
constructor() {
46+
super(
47+
'Test generation failed. Amazon Q timed out.',
48+
'TestGenTimedOutError',
49+
technicalErrorCustomerFacingMessage
50+
)
51+
}
52+
}
53+
export class TestGenStoppedError extends TestGenError {
54+
constructor() {
55+
super('Unit test generation cancelled.', 'TestGenCancelled', 'Unit test generation cancelled.')
56+
}
57+
}
58+
export class TestGenFailedError extends TestGenError {
59+
constructor(error?: string) {
60+
super(error ?? 'Test generation failed', 'TestGenFailedError', error ?? technicalErrorCustomerFacingMessage)
61+
}
62+
}
63+
export class ExportResultsArchiveError extends TestGenError {
64+
constructor(error?: string) {
65+
super(error ?? 'Test generation failed', 'ExportResultsArchiveError', technicalErrorCustomerFacingMessage)
66+
}
67+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { ChildProcess, spawn } from 'child_process' // eslint-disable-line no-re
2121
import { BuildStatus } from '../../amazonqTest/chat/session/session'
2222
import { fs } from '../../shared/fs/fs'
2323
import { TestGenerationJobStatus } from '../models/constants'
24-
import { TestGenFailedError } from '../models/errors'
24+
import { TestGenFailedError } from '../../amazonqTest/error'
2525
import { Range } from '../client/codewhispereruserclient'
2626

2727
// eslint-disable-next-line unicorn/no-null
@@ -75,8 +75,9 @@ export async function startTestGenerationProcess(
7575
try {
7676
artifactMap = await getPresignedUrlAndUploadTestGen(zipMetadata)
7777
} finally {
78-
if (await fs.existsFile(path.join(testGenerationLogsDir, 'output.log'))) {
79-
await fs.delete(path.join(testGenerationLogsDir, 'output.log'))
78+
const outputLogPath = path.join(testGenerationLogsDir, 'output.log')
79+
if (await fs.existsFile(outputLogPath)) {
80+
await fs.delete(outputLogPath)
8081
}
8182
await zipUtil.removeTmpFiles(zipMetadata)
8283
session.artifactsUploadDuration = performance.now() - uploadStartTime

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,8 @@ export const noOpenProjectsFoundChatTestGenMessage = `Sorry, I couldn\'t find a
726726

727727
export const unitTestGenerationCancelMessage = 'Unit test generation cancelled.'
728728

729+
export const tooManyRequestErrorMessage = 'Too many requests. Please wait before retrying.'
730+
729731
export const noJavaProjectsFoundChatMessage = `I couldn\'t find a project that I can upgrade. Currently, I support Java 8, Java 11, and Java 17 projects built on Maven. Make sure your project is open in the IDE. For more information, see the [Amazon Q documentation](${codeTransformPrereqDoc}).`
730732

731733
export const linkToDocsHome = 'https://docs.aws.amazon.com/amazonq/latest/aws-builder-use-ug/code-transformation.html'
@@ -861,7 +863,7 @@ export enum TestGenerationJobStatus {
861863
COMPLETED = 'COMPLETED',
862864
}
863865

864-
export enum ZipUseCase {
866+
export enum FeatureUseCase {
865867
TEST_GENERATION = 'TEST_GENERATION',
866868
CODE_SCAN = 'CODE_SCAN',
867869
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export async function getPresignedUrlAndUpload(
3434
getLogger().verbose(`CreateUploadUrlRequest requestId: ${srcResp.$response.requestId}`)
3535
getLogger().verbose(`Complete Getting presigned Url for uploading src context.`)
3636
getLogger().verbose(`Uploading src context...`)
37-
await uploadArtifactToS3(zipFilePath, srcResp)
37+
await uploadArtifactToS3(zipFilePath, srcResp, CodeWhispererConstants.FeatureUseCase.CODE_SCAN)
3838
getLogger().verbose(`Complete uploading src context.`)
3939
const artifactMap: ArtifactMap = {
4040
SourceCode: srcResp.uploadId,

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ import { getTelemetryReasonDesc } from '../../shared/errors'
4343
import { CodeWhispererSettings } from '../util/codewhispererSettings'
4444
import { detectCommentAboveLine } from '../../shared/utilities/commentUtils'
4545
import { runtimeLanguageContext } from '../util/runtimeLanguageContext'
46+
import { FeatureUseCase } from '../models/constants'
47+
import { UploadTestArtifactToS3Error } from '../../amazonqTest/error'
4648

4749
export async function listScanResults(
4850
client: DefaultCodeWhispererClient,
@@ -287,7 +289,7 @@ export async function getPresignedUrlAndUpload(
287289
logger.verbose(`CreateUploadUrlRequest request id: ${srcResp.$response.requestId}`)
288290
logger.verbose(`Complete Getting presigned Url for uploading src context.`)
289291
logger.verbose(`Uploading src context...`)
290-
await uploadArtifactToS3(zipMetadata.zipFilePath, srcResp, scope)
292+
await uploadArtifactToS3(zipMetadata.zipFilePath, srcResp, FeatureUseCase.CODE_SCAN, scope)
291293
logger.verbose(`Complete uploading src context.`)
292294
const artifactMap: ArtifactMap = {
293295
SourceCode: srcResp.uploadId,
@@ -343,6 +345,7 @@ export function throwIfCancelled(scope: CodeWhispererConstants.CodeAnalysisScope
343345
export async function uploadArtifactToS3(
344346
fileName: string,
345347
resp: CreateUploadUrlResponse,
348+
featureUseCase: FeatureUseCase,
346349
scope?: CodeWhispererConstants.CodeAnalysisScope
347350
) {
348351
const logger = getLoggerForScope(scope)
@@ -365,14 +368,23 @@ export async function uploadArtifactToS3(
365368
}).response
366369
logger.debug(`StatusCode: ${response.status}, Text: ${response.statusText}`)
367370
} catch (error) {
371+
let errorMessage = ''
372+
const isCodeScan = featureUseCase === FeatureUseCase.CODE_SCAN
373+
const featureType = isCodeScan ? 'security scans' : 'unit test generation'
374+
const defaultMessage = isCodeScan ? 'Security scan failed.' : 'Test generation failed.'
368375
getLogger().error(
369-
`Amazon Q is unable to upload workspace artifacts to Amazon S3 for security scans. For more information, see the Amazon Q documentation or contact your network or organization administrator.`
376+
`Amazon Q is unable to upload workspace artifacts to Amazon S3 for ${featureType}. ` +
377+
'For more information, see the Amazon Q documentation or contact your network or organization administrator.'
370378
)
371-
const errorMessage = getTelemetryReasonDesc(error)?.includes(`"PUT" request failed with code "403"`)
372-
? `"PUT" request failed with code "403"`
373-
: (getTelemetryReasonDesc(error) ?? 'Security scan failed.')
374-
375-
throw new UploadArtifactToS3Error(errorMessage)
379+
const errorDesc = getTelemetryReasonDesc(error)
380+
if (errorDesc?.includes('"PUT" request failed with code "403"')) {
381+
errorMessage = '"PUT" request failed with code "403"'
382+
} else if (errorDesc?.includes('"PUT" request failed with code "503"')) {
383+
errorMessage = '"PUT" request failed with code "503"'
384+
} else {
385+
errorMessage = errorDesc ?? defaultMessage
386+
}
387+
throw isCodeScan ? new UploadArtifactToS3Error(errorMessage) : new UploadTestArtifactToS3Error(errorMessage)
376388
}
377389
}
378390

0 commit comments

Comments
 (0)