From 4e55a3982000c3eee6835b7802bae71e10167bfe Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Thu, 26 Dec 2024 13:49:00 -0800 Subject: [PATCH 1/8] telemetry(amazonq): attach stack trace in onCodeGeneration failures --- .../amazonqFeatureDev/controllers/chat/controller.ts | 6 +++++- .../core/src/amazonqFeatureDev/session/session.ts | 12 +++++++++++- .../controllers/chat/controller.test.ts | 12 +++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index aad70595366..a7073fcc5f2 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -504,6 +504,10 @@ export class FeatureDevController { result = MetricDataResult.Fault } break + case ContentLengthError.name: + case MonthlyConversationLimitError.name: + case CodeIterationLimitError.name: + case UploadURLExpired.name: case PromptRefusalException.name: case NoChangeRequiredException.name: result = MetricDataResult.Error @@ -513,7 +517,7 @@ export class FeatureDevController { break } - await session.sendMetricDataTelemetry(MetricDataOperationName.EndCodeGeneration, result) + await session.sendMetricDataTelemetry(MetricDataOperationName.EndCodeGeneration, result, err.stack) throw err } finally { // Finish processing the event diff --git a/packages/core/src/amazonqFeatureDev/session/session.ts b/packages/core/src/amazonqFeatureDev/session/session.ts index 0f6d13bc0e9..51b5f211670 100644 --- a/packages/core/src/amazonqFeatureDev/session/session.ts +++ b/packages/core/src/amazonqFeatureDev/session/session.ts @@ -285,7 +285,7 @@ export class Session { return { leftPath, rightPath, ...diff } } - public async sendMetricDataTelemetry(operationName: string, result: string) { + public async sendMetricDataTelemetry(operationName: string, result: string, log?: string) { await this.proxyClient.sendMetricData({ metricName: 'Operation', metricValue: 1, @@ -300,6 +300,16 @@ export class Session { name: 'result', value: result, }, + + // The log dimension will not be emitted to CloudWatch + ...(log + ? [ + { + name: 'log', + value: log, + }, + ] + : []), ], }) } diff --git a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts index a6d36942840..c25948e01c2 100644 --- a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts +++ b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts @@ -464,6 +464,10 @@ describe('Controller', () => { const errorResultMapping = new Map([ ['EmptyPatchException', MetricDataResult.LlmFailure], + [ContentLengthError.name, MetricDataResult.Error], + [MonthlyConversationLimitError.name, MetricDataResult.Error], + [CodeIterationLimitError.name, MetricDataResult.Error], + [UploadURLExpired.name, MetricDataResult.Error], [PromptRefusalException.name, MetricDataResult.Error], [NoChangeRequiredException.name, MetricDataResult.Error], ]) @@ -519,7 +523,13 @@ describe('Controller', () => { ) const metricResult = getMetricResult(error) assert.ok( - sendMetricDataTelemetrySpy.calledWith(MetricDataOperationName.EndCodeGeneration, metricResult) + sendMetricDataTelemetrySpy.calledWith( + MetricDataOperationName.EndCodeGeneration, + metricResult, + + // Stack trace should include the name of the test framework + sinon.match((str) => typeof str === 'string' && str.includes('Mocha')) + ) ) } From e74c19c1458b4a9bffc7e08414bc711b266001db Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Fri, 27 Dec 2024 10:51:50 -0800 Subject: [PATCH 2/8] clarified log dimension --- packages/core/src/amazonqFeatureDev/session/session.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/amazonqFeatureDev/session/session.ts b/packages/core/src/amazonqFeatureDev/session/session.ts index 51b5f211670..338727b6f82 100644 --- a/packages/core/src/amazonqFeatureDev/session/session.ts +++ b/packages/core/src/amazonqFeatureDev/session/session.ts @@ -301,7 +301,7 @@ export class Session { value: result, }, - // The log dimension will not be emitted to CloudWatch + // The log dimension will be emitted only to CloudWatch Logs ...(log ? [ { From 3ee9f764ad102dbaa8787fbbb36a681cb403a1b9 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Fri, 27 Dec 2024 12:03:23 -0800 Subject: [PATCH 3/8] style --- .../src/amazonqFeatureDev/controllers/chat/controller.ts | 6 +++++- .../amazonqFeatureDev/controllers/chat/controller.test.ts | 4 +--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index a7073fcc5f2..a50d624ed8f 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -517,7 +517,11 @@ export class FeatureDevController { break } - await session.sendMetricDataTelemetry(MetricDataOperationName.EndCodeGeneration, result, err.stack) + await session.sendMetricDataTelemetry( + MetricDataOperationName.EndCodeGeneration, + result, + 'stack trace: ' + (err.stack ?? '') + ) throw err } finally { // Finish processing the event diff --git a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts index c25948e01c2..5c0f86ecbc2 100644 --- a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts +++ b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts @@ -526,9 +526,7 @@ describe('Controller', () => { sendMetricDataTelemetrySpy.calledWith( MetricDataOperationName.EndCodeGeneration, metricResult, - - // Stack trace should include the name of the test framework - sinon.match((str) => typeof str === 'string' && str.includes('Mocha')) + sinon.match((str) => typeof str === 'string' && str.includes('stack trace: ')) ) ) } From ba1b8e9ceb3fdd91c4c5d6e9de8f5943b8d7d498 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Thu, 23 Jan 2025 12:09:30 -0800 Subject: [PATCH 4/8] Whitelist --- .../controllers/chat/controller.ts | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index a50d624ed8f..5f6fa3ca365 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -12,9 +12,11 @@ import { createSingleFileDialog } from '../../../shared/ui/common/openDialog' import { CodeIterationLimitError, ContentLengthError, + ConversationIdNotFoundError, createUserFacingErrorMessage, denyListedErrors, FeatureDevServiceError, + IllegalStateTransition, MonthlyConversationLimitError, NoChangeRequiredException, PrepareRepoFailedError, @@ -520,7 +522,7 @@ export class FeatureDevController { await session.sendMetricDataTelemetry( MetricDataOperationName.EndCodeGeneration, result, - 'stack trace: ' + (err.stack ?? '') + 'stack trace: ' + this.getStackTraceForError(err) ) throw err } finally { @@ -1017,4 +1019,54 @@ export class FeatureDevController { }) } } + + // Should include error messages only for whitelisted exceptions + // i.e. exceptions with deterministic error messages and do not include sensitive data + private getStackTraceForError(error: Error): string { + const seenExceptions = new Set() + const lines: string[] = [] + + function printExceptionDetails(err: Error, prefix: string = '') { + if (seenExceptions.has(err)) { + return + } + seenExceptions.add(err) + + if ( + err instanceof FeatureDevServiceError || + err instanceof ConversationIdNotFoundError || + err instanceof TabIdNotFoundError || + err instanceof WorkspaceFolderNotFoundError || + err instanceof UserMessageNotFoundError || + err instanceof SelectedFolderNotInWorkspaceFolderError || + err instanceof PromptRefusalException || + // err instanceof NoChangeRequiredException || + err instanceof PrepareRepoFailedError || + err instanceof UploadCodeError || + err instanceof UploadURLExpired || + err instanceof IllegalStateTransition || + err instanceof ContentLengthError || + err instanceof ZipFileError || + err instanceof CodeIterationLimitError + ) { + lines.push(`${prefix}${err.constructor.name}: ${err.message}`) + } else { + lines.push(`${prefix}${err.constructor.name}`) + } + + if (err.stack) { + const callStack = err.stack.substring(err.stack.indexOf(err.message) + err.message.length + 1) + lines.push(`${prefix}${callStack}`) + } + + const cause = (err as any).cause + if (cause instanceof Error) { + lines.push(`${prefix}\tCaused by: `) + printExceptionDetails(cause, `${prefix}\t`) + } + } + + printExceptionDetails(error) + return lines.join('\n') + } } From b4533b1749dd326a55266a9a5cd87b24fe81d274 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Thu, 23 Jan 2025 13:30:46 -0800 Subject: [PATCH 5/8] Whitelist --- .../src/amazonqFeatureDev/controllers/chat/controller.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index 5f6fa3ca365..7a3a23b6527 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -1040,7 +1040,7 @@ export class FeatureDevController { err instanceof UserMessageNotFoundError || err instanceof SelectedFolderNotInWorkspaceFolderError || err instanceof PromptRefusalException || - // err instanceof NoChangeRequiredException || + err instanceof NoChangeRequiredException || err instanceof PrepareRepoFailedError || err instanceof UploadCodeError || err instanceof UploadURLExpired || @@ -1055,7 +1055,8 @@ export class FeatureDevController { } if (err.stack) { - const callStack = err.stack.substring(err.stack.indexOf(err.message) + err.message.length + 1) + const startStr = err.stack.substring('Error: '.length) + const callStack = startStr.substring(startStr.indexOf(err.message) + err.message.length + 1) lines.push(`${prefix}${callStack}`) } From 19b4311243829a6c61e430931332261620e3c5c8 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Thu, 23 Jan 2025 14:51:18 -0800 Subject: [PATCH 6/8] Safe --- .../amazonqFeatureDev/controllers/chat/controller.ts | 11 ++++++----- .../controllers/chat/controller.test.ts | 7 ++++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index 7a3a23b6527..00cf8806314 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -1020,14 +1020,15 @@ export class FeatureDevController { } } - // Should include error messages only for whitelisted exceptions + // Should include error messages only for safe exceptions // i.e. exceptions with deterministic error messages and do not include sensitive data private getStackTraceForError(error: Error): string { + const recursionLimit = 3 const seenExceptions = new Set() const lines: string[] = [] - function printExceptionDetails(err: Error, prefix: string = '') { - if (seenExceptions.has(err)) { + function printExceptionDetails(err: Error, depth: number, prefix: string = '') { + if (depth >= recursionLimit || seenExceptions.has(err)) { return } seenExceptions.add(err) @@ -1063,11 +1064,11 @@ export class FeatureDevController { const cause = (err as any).cause if (cause instanceof Error) { lines.push(`${prefix}\tCaused by: `) - printExceptionDetails(cause, `${prefix}\t`) + printExceptionDetails(cause, depth + 1, `${prefix}\t`) } } - printExceptionDetails(error) + printExceptionDetails(error, 0) return lines.join('\n') } } diff --git a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts index 5c0f86ecbc2..af900e5d985 100644 --- a/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts +++ b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts @@ -526,7 +526,12 @@ describe('Controller', () => { sendMetricDataTelemetrySpy.calledWith( MetricDataOperationName.EndCodeGeneration, metricResult, - sinon.match((str) => typeof str === 'string' && str.includes('stack trace: ')) + sinon.match( + (str) => + typeof str === 'string' && + str.includes('stack trace: ' + error.constructor.name) && + str.includes('at ') + ) ) ) } From 9a1d9646ba41ab21a3f120af0d0a143401b856e1 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Fri, 24 Jan 2025 16:19:28 -0800 Subject: [PATCH 7/8] Fix --- .../controllers/chat/controller.ts | 58 +------------------ packages/core/src/amazonqFeatureDev/errors.ts | 33 ++++++----- packages/core/src/shared/errors.ts | 47 +++++++++++++++ packages/core/src/test/shared/errors.test.ts | 47 +++++++++++++++ 4 files changed, 113 insertions(+), 72 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index 00cf8806314..2073b8e1359 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -12,11 +12,9 @@ import { createSingleFileDialog } from '../../../shared/ui/common/openDialog' import { CodeIterationLimitError, ContentLengthError, - ConversationIdNotFoundError, createUserFacingErrorMessage, denyListedErrors, FeatureDevServiceError, - IllegalStateTransition, MonthlyConversationLimitError, NoChangeRequiredException, PrepareRepoFailedError, @@ -46,7 +44,7 @@ import { getWorkspaceFoldersByPrefixes } from '../../../shared/utilities/workspa import { openDeletedDiff, openDiff } from '../../../amazonq/commons/diff' import { i18n } from '../../../shared/i18n-helper' import globals from '../../../shared/extensionGlobals' -import { randomUUID } from '../../../shared' +import { getStackTraceForError, randomUUID } from '../../../shared' import { FollowUpTypes } from '../../../amazonq/commons/types' import { Messenger } from '../../../amazonq/commons/connector/baseMessenger' import { BaseChatSessionStorage } from '../../../amazonq/commons/baseChatStorage' @@ -522,7 +520,7 @@ export class FeatureDevController { await session.sendMetricDataTelemetry( MetricDataOperationName.EndCodeGeneration, result, - 'stack trace: ' + this.getStackTraceForError(err) + 'stack trace: ' + getStackTraceForError(err) ) throw err } finally { @@ -1019,56 +1017,4 @@ export class FeatureDevController { }) } } - - // Should include error messages only for safe exceptions - // i.e. exceptions with deterministic error messages and do not include sensitive data - private getStackTraceForError(error: Error): string { - const recursionLimit = 3 - const seenExceptions = new Set() - const lines: string[] = [] - - function printExceptionDetails(err: Error, depth: number, prefix: string = '') { - if (depth >= recursionLimit || seenExceptions.has(err)) { - return - } - seenExceptions.add(err) - - if ( - err instanceof FeatureDevServiceError || - err instanceof ConversationIdNotFoundError || - err instanceof TabIdNotFoundError || - err instanceof WorkspaceFolderNotFoundError || - err instanceof UserMessageNotFoundError || - err instanceof SelectedFolderNotInWorkspaceFolderError || - err instanceof PromptRefusalException || - err instanceof NoChangeRequiredException || - err instanceof PrepareRepoFailedError || - err instanceof UploadCodeError || - err instanceof UploadURLExpired || - err instanceof IllegalStateTransition || - err instanceof ContentLengthError || - err instanceof ZipFileError || - err instanceof CodeIterationLimitError - ) { - lines.push(`${prefix}${err.constructor.name}: ${err.message}`) - } else { - lines.push(`${prefix}${err.constructor.name}`) - } - - if (err.stack) { - const startStr = err.stack.substring('Error: '.length) - const callStack = startStr.substring(startStr.indexOf(err.message) + err.message.length + 1) - lines.push(`${prefix}${callStack}`) - } - - const cause = (err as any).cause - if (cause instanceof Error) { - lines.push(`${prefix}\tCaused by: `) - printExceptionDetails(cause, depth + 1, `${prefix}\t`) - } - } - - printExceptionDetails(error, 0) - return lines.join('\n') - } } diff --git a/packages/core/src/amazonqFeatureDev/errors.ts b/packages/core/src/amazonqFeatureDev/errors.ts index 06e994080f3..dee7d5683cb 100644 --- a/packages/core/src/amazonqFeatureDev/errors.ts +++ b/packages/core/src/amazonqFeatureDev/errors.ts @@ -3,12 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ToolkitError } from '../shared/errors' +import { SafeMessageError, ToolkitError } from '../shared/errors' import { featureName } from './constants' import { uploadCodeError } from './userFacingText' import { i18n } from '../shared/i18n-helper' -export class ConversationIdNotFoundError extends ToolkitError { +export class ConversationIdNotFoundError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.conversationIdNotFoundError'), { code: 'ConversationIdNotFound', @@ -16,7 +16,7 @@ export class ConversationIdNotFoundError extends ToolkitError { } } -export class TabIdNotFoundError extends ToolkitError { +export class TabIdNotFoundError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.tabIdNotFoundError'), { code: 'TabIdNotFound', @@ -24,7 +24,7 @@ export class TabIdNotFoundError extends ToolkitError { } } -export class WorkspaceFolderNotFoundError extends ToolkitError { +export class WorkspaceFolderNotFoundError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.workspaceFolderNotFoundError'), { code: 'WorkspaceFolderNotFound', @@ -32,7 +32,7 @@ export class WorkspaceFolderNotFoundError extends ToolkitError { } } -export class UserMessageNotFoundError extends ToolkitError { +export class UserMessageNotFoundError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.userMessageNotFoundError'), { code: 'MessageNotFound', @@ -40,7 +40,7 @@ export class UserMessageNotFoundError extends ToolkitError { } } -export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError { +export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.selectedFolderNotInWorkspaceFolderError'), { code: 'SelectedFolderNotInWorkspaceFolder', @@ -48,7 +48,7 @@ export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError { } } -export class PromptRefusalException extends ToolkitError { +export class PromptRefusalException extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.promptRefusalException'), { code: 'PromptRefusalException', @@ -56,7 +56,7 @@ export class PromptRefusalException extends ToolkitError { } } -export class NoChangeRequiredException extends ToolkitError { +export class NoChangeRequiredException extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.noChangeRequiredException'), { code: 'NoChangeRequiredException', @@ -64,13 +64,14 @@ export class NoChangeRequiredException extends ToolkitError { } } -export class FeatureDevServiceError extends ToolkitError { +// To prevent potential security issues, message passed in should be predictably safe for telemetry +export class FeatureDevServiceError extends ToolkitError implements SafeMessageError { constructor(message: string, code: string) { super(message, { code }) } } -export class PrepareRepoFailedError extends ToolkitError { +export class PrepareRepoFailedError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.prepareRepoFailedError'), { code: 'PrepareRepoFailed', @@ -78,37 +79,37 @@ export class PrepareRepoFailedError extends ToolkitError { } } -export class UploadCodeError extends ToolkitError { +export class UploadCodeError extends ToolkitError implements SafeMessageError { constructor(statusCode: string) { super(uploadCodeError, { code: `UploadCode-${statusCode}` }) } } -export class UploadURLExpired extends ToolkitError { +export class UploadURLExpired extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.uploadURLExpired'), { code: 'UploadURLExpired' }) } } -export class IllegalStateTransition extends ToolkitError { +export class IllegalStateTransition extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.illegalStateTransition'), { code: 'IllegalStateTransition' }) } } -export class ContentLengthError extends ToolkitError { +export class ContentLengthError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.contentLengthError'), { code: ContentLengthError.name }) } } -export class ZipFileError extends ToolkitError { +export class ZipFileError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.zipFileError'), { code: ZipFileError.name }) } } -export class CodeIterationLimitError extends ToolkitError { +export class CodeIterationLimitError extends ToolkitError implements SafeMessageError { constructor() { super(i18n('AWS.amazonq.featureDev.error.codeIterationLimitError'), { code: CodeIterationLimitError.name }) } diff --git a/packages/core/src/shared/errors.ts b/packages/core/src/shared/errors.ts index 54841b7622f..5683412ef92 100644 --- a/packages/core/src/shared/errors.ts +++ b/packages/core/src/shared/errors.ts @@ -368,6 +368,53 @@ export function getTelemetryResult(error: unknown | undefined): Result { return 'Failed' } +export class SafeMessageError extends Error {} + +/** + * This function constructs a string similar to error.printStackTrace() for telemetry, + * but include error messages only for safe exceptions, + * i.e. exceptions with deterministic error messages and do not include sensitive data. + * + * @param error The error to get stack trace string from + */ +export function getStackTraceForError(error: Error): string { + const recursionLimit = 3 + const seenExceptions = new Set() + const lines: string[] = [] + + function printExceptionDetails(err: Error, depth: number, prefix: string = '') { + if (depth >= recursionLimit || seenExceptions.has(err)) { + return + } + seenExceptions.add(err) + + if (error instanceof SafeMessageError) { + lines.push(`${prefix}${err.constructor.name}: ${err.message}`) + } else { + lines.push(`${prefix}${err.constructor.name}`) + } + + if (err.stack) { + const startStr = err.stack.substring('Error: '.length) + const callStack = startStr.substring(startStr.indexOf(err.message) + err.message.length + 1) + for (const line of callStack.split('\n')) { + const scrubbed = scrubNames(line, _username) + lines.push(`${prefix}\t${scrubbed}`) + } + } + + const cause = (err as any).cause + if (cause instanceof Error) { + lines.push(`${prefix}\tCaused by: `) + printExceptionDetails(cause, depth + 1, `${prefix}\t`) + } + } + + printExceptionDetails(error, 0) + getLogger().info(lines.join('\n')) + return lines.join('\n') +} + /** * Removes potential PII from a string, for logging/telemetry. * diff --git a/packages/core/src/test/shared/errors.test.ts b/packages/core/src/test/shared/errors.test.ts index 32d18186912..c33afb69ffe 100644 --- a/packages/core/src/test/shared/errors.test.ts +++ b/packages/core/src/test/shared/errors.test.ts @@ -20,6 +20,8 @@ import { tryRun, UnknownError, getErrorId, + SafeMessageError, + getStackTraceForError, } from '../../shared/errors' import { CancellationError } from '../../shared/utilities/timeoutUtils' import { UnauthorizedException } from '@aws-sdk/client-sso' @@ -347,6 +349,51 @@ describe('Telemetry', function () { assert.strictEqual(getTelemetryReason(error2), 'ErrorCode') }) }) + + describe('getStackTraceForError', () => { + class SafeError extends ToolkitError implements SafeMessageError {} + class UnsafeError extends ToolkitError {} + + it('includes message for safe exceptions', () => { + const safeError = new SafeError('Safe error message') + const result = getStackTraceForError(safeError) + + assert.ok(result.includes('Safe error message')) + assert.ok(result.includes('SafeError: ')) + }) + + it('excludes message for unsafe exceptions', () => { + const unsafeError = new UnsafeError('Sensitive information') + const result = getStackTraceForError(unsafeError) + + assert.ok(!result.includes('Sensitive information')) + assert.ok(result.includes('UnsafeError')) + }) + + it('respects recursion limit for nested exceptions', () => { + const recursionLimit = 3 + const exceptions: ToolkitError[] = [] + + let currentError = new SafeError(`depth ${recursionLimit + 2}`) + exceptions.push(currentError) + + for (let i = recursionLimit + 1; i >= 0; i--) { + currentError = new SafeError(`depth ${i}`, { cause: currentError }) + exceptions.push(currentError) + } + const stackTrace = getStackTraceForError(exceptions[exceptions.length - 1]) + + // Assert exceptions within limit are included + for (let i = 0; i < recursionLimit; i++) { + assert(stackTrace.includes(`depth ${i}`), `Stack trace should include error at depth ${i}`) + } + + // Assert exceptions beyond limit are not included + for (let i = recursionLimit; i <= exceptions.length; i++) { + assert(!stackTrace.includes(`depth ${i}`), `Stack trace should not include error at depth ${i}`) + } + }) + }) }) describe('resolveErrorMessageToDisplay()', function () { From 977af160b1d77a02e316f235089e7fdf57ef2617 Mon Sep 17 00:00:00 2001 From: Zhuowen Chen Date: Mon, 27 Jan 2025 10:45:29 -0800 Subject: [PATCH 8/8] Inheritance --- packages/core/src/amazonqFeatureDev/errors.ts | 32 +++++++++---------- packages/core/src/shared/errors.ts | 8 +++-- packages/core/src/test/shared/errors.test.ts | 4 +-- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/core/src/amazonqFeatureDev/errors.ts b/packages/core/src/amazonqFeatureDev/errors.ts index dee7d5683cb..dba3785c9d4 100644 --- a/packages/core/src/amazonqFeatureDev/errors.ts +++ b/packages/core/src/amazonqFeatureDev/errors.ts @@ -3,12 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { SafeMessageError, ToolkitError } from '../shared/errors' +import { SafeMessageToolkitError, ToolkitError } from '../shared/errors' import { featureName } from './constants' import { uploadCodeError } from './userFacingText' import { i18n } from '../shared/i18n-helper' -export class ConversationIdNotFoundError extends ToolkitError implements SafeMessageError { +export class ConversationIdNotFoundError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.conversationIdNotFoundError'), { code: 'ConversationIdNotFound', @@ -16,7 +16,7 @@ export class ConversationIdNotFoundError extends ToolkitError implements SafeMes } } -export class TabIdNotFoundError extends ToolkitError implements SafeMessageError { +export class TabIdNotFoundError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.tabIdNotFoundError'), { code: 'TabIdNotFound', @@ -24,7 +24,7 @@ export class TabIdNotFoundError extends ToolkitError implements SafeMessageError } } -export class WorkspaceFolderNotFoundError extends ToolkitError implements SafeMessageError { +export class WorkspaceFolderNotFoundError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.workspaceFolderNotFoundError'), { code: 'WorkspaceFolderNotFound', @@ -32,7 +32,7 @@ export class WorkspaceFolderNotFoundError extends ToolkitError implements SafeMe } } -export class UserMessageNotFoundError extends ToolkitError implements SafeMessageError { +export class UserMessageNotFoundError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.userMessageNotFoundError'), { code: 'MessageNotFound', @@ -40,7 +40,7 @@ export class UserMessageNotFoundError extends ToolkitError implements SafeMessag } } -export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError implements SafeMessageError { +export class SelectedFolderNotInWorkspaceFolderError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.selectedFolderNotInWorkspaceFolderError'), { code: 'SelectedFolderNotInWorkspaceFolder', @@ -48,7 +48,7 @@ export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError implem } } -export class PromptRefusalException extends ToolkitError implements SafeMessageError { +export class PromptRefusalException extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.promptRefusalException'), { code: 'PromptRefusalException', @@ -56,7 +56,7 @@ export class PromptRefusalException extends ToolkitError implements SafeMessageE } } -export class NoChangeRequiredException extends ToolkitError implements SafeMessageError { +export class NoChangeRequiredException extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.noChangeRequiredException'), { code: 'NoChangeRequiredException', @@ -65,13 +65,13 @@ export class NoChangeRequiredException extends ToolkitError implements SafeMessa } // To prevent potential security issues, message passed in should be predictably safe for telemetry -export class FeatureDevServiceError extends ToolkitError implements SafeMessageError { +export class FeatureDevServiceError extends SafeMessageToolkitError { constructor(message: string, code: string) { super(message, { code }) } } -export class PrepareRepoFailedError extends ToolkitError implements SafeMessageError { +export class PrepareRepoFailedError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.prepareRepoFailedError'), { code: 'PrepareRepoFailed', @@ -79,37 +79,37 @@ export class PrepareRepoFailedError extends ToolkitError implements SafeMessageE } } -export class UploadCodeError extends ToolkitError implements SafeMessageError { +export class UploadCodeError extends SafeMessageToolkitError { constructor(statusCode: string) { super(uploadCodeError, { code: `UploadCode-${statusCode}` }) } } -export class UploadURLExpired extends ToolkitError implements SafeMessageError { +export class UploadURLExpired extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.uploadURLExpired'), { code: 'UploadURLExpired' }) } } -export class IllegalStateTransition extends ToolkitError implements SafeMessageError { +export class IllegalStateTransition extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.illegalStateTransition'), { code: 'IllegalStateTransition' }) } } -export class ContentLengthError extends ToolkitError implements SafeMessageError { +export class ContentLengthError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.contentLengthError'), { code: ContentLengthError.name }) } } -export class ZipFileError extends ToolkitError implements SafeMessageError { +export class ZipFileError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.zipFileError'), { code: ZipFileError.name }) } } -export class CodeIterationLimitError extends ToolkitError implements SafeMessageError { +export class CodeIterationLimitError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.codeIterationLimitError'), { code: CodeIterationLimitError.name }) } diff --git a/packages/core/src/shared/errors.ts b/packages/core/src/shared/errors.ts index 5683412ef92..c50bab1c726 100644 --- a/packages/core/src/shared/errors.ts +++ b/packages/core/src/shared/errors.ts @@ -368,7 +368,11 @@ export function getTelemetryResult(error: unknown | undefined): Result { return 'Failed' } -export class SafeMessageError extends Error {} +export class SafeMessageToolkitError extends ToolkitError { + public constructor(message: string, info: ErrorInformation = {}) { + super(message, info) + } +} /** * This function constructs a string similar to error.printStackTrace() for telemetry, @@ -388,7 +392,7 @@ export function getStackTraceForError(error: Error): string { } seenExceptions.add(err) - if (error instanceof SafeMessageError) { + if (error instanceof SafeMessageToolkitError) { lines.push(`${prefix}${err.constructor.name}: ${err.message}`) } else { lines.push(`${prefix}${err.constructor.name}`) diff --git a/packages/core/src/test/shared/errors.test.ts b/packages/core/src/test/shared/errors.test.ts index c33afb69ffe..6e68e1170a9 100644 --- a/packages/core/src/test/shared/errors.test.ts +++ b/packages/core/src/test/shared/errors.test.ts @@ -20,8 +20,8 @@ import { tryRun, UnknownError, getErrorId, - SafeMessageError, getStackTraceForError, + SafeMessageToolkitError, } from '../../shared/errors' import { CancellationError } from '../../shared/utilities/timeoutUtils' import { UnauthorizedException } from '@aws-sdk/client-sso' @@ -351,7 +351,7 @@ describe('Telemetry', function () { }) describe('getStackTraceForError', () => { - class SafeError extends ToolkitError implements SafeMessageError {} + class SafeError extends SafeMessageToolkitError {} class UnsafeError extends ToolkitError {} it('includes message for safe exceptions', () => {