Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@siakmun-aws
siakmun-aws committed Jan 25, 2025
1 parent 917708a commit 20513ea
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 72 deletions.
58 changes: 2 additions & 56 deletions packages/core/src/amazonqFeatureDev/controllers/chat/controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ import { createSingleFileDialog } from '../../../shared/ui/common/openDialog'
import {
CodeIterationLimitError,
ContentLengthError,
ConversationIdNotFoundError,
createUserFacingErrorMessage,
denyListedErrors,
FeatureDevServiceError,
IllegalStateTransition,
MonthlyConversationLimitError,
NoChangeRequiredException,
PrepareRepoFailedError,
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Error>()
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')
}
}
33 changes: 17 additions & 16 deletions packages/core/src/amazonqFeatureDev/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,112 +3,113 @@
* 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',
})
}
}

export class TabIdNotFoundError extends ToolkitError {
export class TabIdNotFoundError extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.tabIdNotFoundError'), {
code: 'TabIdNotFound',
})
}
}

export class WorkspaceFolderNotFoundError extends ToolkitError {
export class WorkspaceFolderNotFoundError extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.workspaceFolderNotFoundError'), {
code: 'WorkspaceFolderNotFound',
})
}
}

export class UserMessageNotFoundError extends ToolkitError {
export class UserMessageNotFoundError extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.userMessageNotFoundError'), {
code: 'MessageNotFound',
})
}
}

export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError {
export class SelectedFolderNotInWorkspaceFolderError extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.selectedFolderNotInWorkspaceFolderError'), {
code: 'SelectedFolderNotInWorkspaceFolder',
})
}
}

export class PromptRefusalException extends ToolkitError {
export class PromptRefusalException extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.promptRefusalException'), {
code: 'PromptRefusalException',
})
}
}

export class NoChangeRequiredException extends ToolkitError {
export class NoChangeRequiredException extends ToolkitError implements SafeMessageError {
constructor() {
super(i18n('AWS.amazonq.featureDev.error.noChangeRequiredException'), {
code: 'NoChangeRequiredException',
})
}
}

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',
})
}
}

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 })
}
Expand Down
47 changes: 47 additions & 0 deletions packages/core/src/shared/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Error>()
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.
*
Expand Down
47 changes: 47 additions & 0 deletions packages/core/src/test/shared/errors.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 () {
Expand Down

0 comments on commit 20513ea

Please sign in to comment.