diff --git a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts index aad70595366..2073b8e1359 100644 --- a/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts +++ b/packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts @@ -44,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' @@ -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,11 @@ export class FeatureDevController { break } - await session.sendMetricDataTelemetry(MetricDataOperationName.EndCodeGeneration, result) + await session.sendMetricDataTelemetry( + MetricDataOperationName.EndCodeGeneration, + result, + 'stack trace: ' + getStackTraceForError(err) + ) throw err } finally { // Finish processing the event diff --git a/packages/core/src/amazonqFeatureDev/errors.ts b/packages/core/src/amazonqFeatureDev/errors.ts index 06e994080f3..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 { 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 { +export class ConversationIdNotFoundError extends SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { 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 SafeMessageToolkitError { constructor(message: string, code: string) { super(message, { code }) } } -export class PrepareRepoFailedError extends ToolkitError { +export class PrepareRepoFailedError extends SafeMessageToolkitError { 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 SafeMessageToolkitError { constructor(statusCode: string) { super(uploadCodeError, { code: `UploadCode-${statusCode}` }) } } -export class UploadURLExpired extends ToolkitError { +export class UploadURLExpired extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.uploadURLExpired'), { code: 'UploadURLExpired' }) } } -export class IllegalStateTransition extends ToolkitError { +export class IllegalStateTransition extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.illegalStateTransition'), { code: 'IllegalStateTransition' }) } } -export class ContentLengthError extends ToolkitError { +export class ContentLengthError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.contentLengthError'), { code: ContentLengthError.name }) } } -export class ZipFileError extends ToolkitError { +export class ZipFileError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.zipFileError'), { code: ZipFileError.name }) } } -export class CodeIterationLimitError extends ToolkitError { +export class CodeIterationLimitError extends SafeMessageToolkitError { constructor() { super(i18n('AWS.amazonq.featureDev.error.codeIterationLimitError'), { code: CodeIterationLimitError.name }) } diff --git a/packages/core/src/amazonqFeatureDev/session/session.ts b/packages/core/src/amazonqFeatureDev/session/session.ts index 0f6d13bc0e9..338727b6f82 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 be emitted only to CloudWatch Logs + ...(log + ? [ + { + name: 'log', + value: log, + }, + ] + : []), ], }) } diff --git a/packages/core/src/shared/errors.ts b/packages/core/src/shared/errors.ts index 54841b7622f..c50bab1c726 100644 --- a/packages/core/src/shared/errors.ts +++ b/packages/core/src/shared/errors.ts @@ -368,6 +368,57 @@ export function getTelemetryResult(error: unknown | undefined): Result { return 'Failed' } +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, + * 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 SafeMessageToolkitError) { + 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/amazonqFeatureDev/controllers/chat/controller.test.ts b/packages/core/src/test/amazonqFeatureDev/controllers/chat/controller.test.ts index a6d36942840..af900e5d985 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,16 @@ describe('Controller', () => { ) const metricResult = getMetricResult(error) assert.ok( - sendMetricDataTelemetrySpy.calledWith(MetricDataOperationName.EndCodeGeneration, metricResult) + sendMetricDataTelemetrySpy.calledWith( + MetricDataOperationName.EndCodeGeneration, + metricResult, + sinon.match( + (str) => + typeof str === 'string' && + str.includes('stack trace: ' + error.constructor.name) && + str.includes('at ') + ) + ) ) } diff --git a/packages/core/src/test/shared/errors.test.ts b/packages/core/src/test/shared/errors.test.ts index 32d18186912..6e68e1170a9 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, + getStackTraceForError, + SafeMessageToolkitError, } 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 SafeMessageToolkitError {} + 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 () {