Skip to content

Commit 6f37eac

Browse files
committed
fix(errors): findPrioritizedAwsError stops at first AWSError
Problem: `findPrioritizedAwsError` does not search the full cause-chain for the best possible error. Solution: - Search the full cause-chain. - Rename to `findBestErrorInChain`.
1 parent a83c471 commit 6f37eac

File tree

3 files changed

+214
-181
lines changed

3 files changed

+214
-181
lines changed

packages/core/src/shared/errors.ts

Lines changed: 53 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ function formatDetails(err: Error): string | undefined {
280280
}
281281
} else if (isAwsError(err)) {
282282
details['statusCode'] = String(err.statusCode ?? '')
283-
details['requestId'] = err.requestId
283+
details['requestId'] = getRequestId(err)
284284
details['extendedRequestId'] = err.extendedRequestId
285285
}
286286

@@ -324,7 +324,7 @@ export function getTelemetryReason(error: unknown | undefined): string | undefin
324324
} else if (error instanceof CancellationError) {
325325
return error.agent
326326
} else if (error instanceof ToolkitError) {
327-
// TODO: prefer the error.error field if present? (see comment in `getTelemetryReasonDesc`)
327+
// TODO: prefer the error.error field if present? (see comment in `getErrorMsg`)
328328
return getTelemetryReason(error.cause) ?? error.code ?? error.name
329329
} else if (error instanceof Error) {
330330
return (error as { code?: string }).code ?? error.name
@@ -334,24 +334,22 @@ export function getTelemetryReason(error: unknown | undefined): string | undefin
334334
}
335335

336336
/**
337-
* Determines the appropriate error message to display to the user.
337+
* Tries to build the most intuitive/relevant message to show to the user.
338338
*
339-
* We do not want to display every error message to the user, this
340-
* resolves what we actually want to show them based off the given
341-
* input.
339+
* User can see the full error chain in the logs Output channel.
342340
*/
343341
export function resolveErrorMessageToDisplay(error: unknown, defaultMessage: string): string {
344-
const mainMessage = error instanceof ToolkitError ? error.message : defaultMessage
345-
// We want to explicitly show certain AWS Error messages if they are raised
346-
const awsError = findPrioritizedAwsError(error)
347-
const prioritizedMessage = getErrorMsg(awsError)
348-
return prioritizedMessage ? `${mainMessage}: ${prioritizedMessage}` : mainMessage
342+
const mainMsg = error instanceof ToolkitError ? error.message : defaultMessage
343+
// Try to find the most useful/relevant error in the `cause` chain.
344+
const bestErr = error ? findBestErrorInChain(error as Error) : undefined
345+
const bestMsg = getErrorMsg(bestErr)
346+
return bestMsg && bestMsg !== mainMsg ? `${mainMsg}: ${bestMsg}` : mainMsg
349347
}
350348

351349
/**
352350
* Patterns that match the value of {@link AWSError.code}
353351
*/
354-
export const prioritizedAwsErrors: RegExp[] = [
352+
const _preferredErrors: RegExp[] = [
355353
/^ConflictException$/,
356354
/^ValidationException$/,
357355
/^ResourceNotFoundException$/,
@@ -360,47 +358,52 @@ export const prioritizedAwsErrors: RegExp[] = [
360358
]
361359

362360
/**
363-
* Tries to find the most useful/relevant error to surface to the user.
361+
* Searches the `cause` chain (if any) for the most useful/relevant {@link AWSError} to surface to
362+
* the user, preferring "deeper" errors (lower-level, closer to the root cause) when all else is equal.
364363
*
365-
* Errors may be wrapped anywhere in the codepath, which may prevent the root cause from being
366-
* reported to the user.
367-
*/
368-
export function findPrioritizedAwsError(
369-
error: unknown,
370-
prioritizedErrors = prioritizedAwsErrors
371-
): AWSError | undefined {
372-
const awsError = findAwsErrorInCausalChain(error)
373-
374-
if (awsError === undefined || !prioritizedErrors.some(regex => regex.test(awsError.code))) {
375-
return undefined
376-
}
377-
378-
return awsError
379-
}
380-
381-
/**
382-
* Searches through the chain of errors (if any) until it finds an {@link AWSError}.
364+
* These conditions determine precedence (in order):
365+
* - required: AWSError type
366+
* - `error_description` field
367+
* - `code` matches one of `preferredErrors`
368+
* - cause chain depth (the deepest error wins)
383369
*
384-
* {@link ToolkitError} instances can wrap a 'cause', which is the underlying
385-
* error that caused it.
370+
* @param error Error whose `cause` chain will be searched.
371+
* @param preferredErrors Error `code` field must match one of these, else it is discarded. Pass `[/./]` to match any AWSError.
386372
*
387-
* @returns AWSError, or undefined
373+
* @returns Best match, or `error` if a better match is not found.
388374
*/
389-
export function findAwsErrorInCausalChain(error: unknown): AWSError | undefined {
390-
let currentError = error
375+
export function findBestErrorInChain(error: Error, preferredErrors = _preferredErrors): Error | undefined {
376+
// TODO: Base Error has 'cause' in ES2022. So does our own `ToolkitError`.
377+
// eslint-disable-next-line @typescript-eslint/naming-convention
378+
let bestErr: Error & { cause?: Error; error_description?: string } = error
379+
let err: typeof bestErr | undefined
380+
381+
for (let i = 0; (err || i === 0) && i < 100; i++) {
382+
err = i === 0 ? bestErr.cause : err?.cause
383+
384+
if (isAwsError(err)) {
385+
if (!isAwsError(bestErr)) {
386+
bestErr = err // Prefer AWSError.
387+
continue
388+
}
391389

392-
for (let i = 0; currentError && i < 100; i++) {
393-
if (isAwsError(currentError)) {
394-
return currentError
395-
}
390+
const errDesc = err.error_description
391+
if (typeof errDesc === 'string' && errDesc.trim() !== '') {
392+
bestErr = err // Prefer (deepest) error with error_description.
393+
continue
394+
}
396395

397-
// Note: Base Error has 'cause' in ES2022. So does our own `ToolkitError`.
398-
if ((currentError as any).cause !== undefined) {
399-
currentError = (currentError as any).cause
396+
// const bestErrCode = bestErr.code?.trim() ?? ''
397+
// const bestErrMatches = bestErrCode !== '' && preferredErrors.some(re => re.test(bestErrCode))
398+
const errCode = err.code?.trim() ?? ''
399+
const errMatches = errCode !== '' && preferredErrors.some(re => re.test(errCode))
400+
if (!bestErr.error_description && errMatches) {
401+
bestErr = err
402+
}
400403
}
401404
}
402405

403-
return undefined
406+
return bestErr
404407
}
405408

406409
export function isCodeWhispererStreamingServiceException(
@@ -426,7 +429,8 @@ function hasName<T>(error: T): error is T & { name: string } {
426429
return typeof (error as { name?: unknown }).name === 'string'
427430
}
428431

429-
export function isAwsError(error: unknown): error is AWSError {
432+
// eslint-disable-next-line @typescript-eslint/naming-convention
433+
export function isAwsError(error: unknown): error is AWSError & { error_description?: string } {
430434
if (error === undefined) {
431435
return false
432436
}
@@ -454,15 +458,15 @@ export function isClientFault(error: ServiceException): boolean {
454458
}
455459

456460
export function getRequestId(err: unknown): string | undefined {
457-
if (isAwsError(err)) {
458-
return err.requestId
459-
}
460-
461461
// XXX: Checking `err instanceof ServiceException` fails for `SSOOIDCServiceException` even
462462
// though it subclasses @aws-sdk/smithy-client.ServiceException
463463
if (typeof (err as any)?.$metadata?.requestId === 'string') {
464464
return (err as any).$metadata.requestId
465465
}
466+
467+
if (isAwsError(err)) {
468+
return err.requestId
469+
}
466470
}
467471

468472
export function isFileNotFoundError(err: unknown): boolean {

packages/core/src/shared/utilities/logAndShowUtils.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,12 @@ import { Logging } from '../logger/commands'
1414
* Logs the error. Then determines what kind of error message should be shown, if
1515
* at all.
1616
*
17+
* TODO: Currently only used for errors from commands and webview. Use in more places (explorer,
18+
* nodes, ...). Must be guaranteed to initialize prior to every other Toolkit component.
19+
*
1720
* @param error The error itself
1821
* @param topic The prefix of the error message
1922
* @param defaultMessage The message to show if once cannot be resolved from the given error
20-
*
21-
* SIDE NOTE:
22-
* This is only being used for errors from commands and webview, there's plenty of other places
23-
* (explorer, nodes, ...) where it could be used. It needs to be apart of some sort of `core`
24-
* module that is guaranteed to initialize prior to every other Toolkit component.
25-
* Logging and telemetry would fit well within this core module.
2623
*/
2724
export async function logAndShowError(
2825
localize: nls.LocalizeFunc,

0 commit comments

Comments
 (0)