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 () {