refactor(telemetry): prepend the error code/name to the reasonDesc …

…field (#5557)

## Problem:

In telemetry we sometimes add the error message to the `reasonDesc`
field.
Additionally when using `getTelemetryReasonDesc()` it will recurse
through the causal
chain of the error and return a value composed of all the error
messages.

Eg: `Reason A | Reason B | Reason C`

The issue is that the nested errors may have useful context (in addition
to the message) in their
id (code or name). And currently this is lost.

## Solution:

Prepend the code/name to the error messages when using
`getTelemetryReasonDesc()`

Eg: `CodeA: Reason A | CodeB: Reason B | NameC: Reason C`

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Signed-off-by: Nikolas Komonen <[email protected]>

packages/core/src/shared/errors.ts

@@ -251,16 +251,11 @@ export class ToolkitError extends Error implements ErrorInformation {
  * @param withCause Append the message(s) from the cause chain, recursively.
  *                  The message(s) are delimited by ' | '. Eg: msg1 | causeMsg1 | causeMsg2
  */
-export function getErrorMsg(err: Error | undefined, withCause = false): string | undefined {
+export function getErrorMsg(err: Error | undefined, withCause: boolean = false): string | undefined {
     if (err === undefined) {
         return undefined
     }
 
-    const cause = (err as any).cause
-    if (withCause && cause) {
-        return `${err.message}${cause ? ' | ' + getErrorMsg(cause, true) : ''}`
-    }
-
     // Non-standard SDK fields added by the OIDC service, to conform to the OAuth spec
     // (https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1) :
     // - error: code per the OAuth spec
@@ -286,12 +281,25 @@ export function getErrorMsg(err: Error | undefined, withCause = false): string |
     //      }
     const anyDesc = (err as any).error_description
     const errDesc = typeof anyDesc === 'string' ? anyDesc.trim() : ''
-    const msg = errDesc !== '' ? errDesc : err.message?.trim()
+    let msg = errDesc !== '' ? errDesc : err.message?.trim()
 
     if (typeof msg !== 'string') {
         return undefined
     }
 
+    // append the cause's message
+    if (withCause) {
+        const errorId = getErrorId(err)
+        // - prepend id to message
+        // - If a generic error does not have the `name` field explicitly set, it returns a generic 'Error' name. So skip since it is useless.
+        if (errorId && errorId !== 'Error') {
+            msg = `${errorId}: ${msg}`
+        }
+
+        const cause = (err as any).cause
+        return `${msg}${cause ? ' | ' + getErrorMsg(cause, withCause) : ''}`
+    }
+
     return msg
 }
 
@@ -420,8 +428,8 @@ export function getTelemetryReasonDesc(err: unknown | undefined): string | undef
     const m = typeof err === 'string' ? err : getErrorMsg(err as Error, true) ?? ''
     const msg = scrubNames(m, _username)
 
-    // Truncate to 200 chars.
-    return msg && msg.length > 0 ? msg.substring(0, 200) : undefined
+    // Truncate message as these strings can be very long.
+    return msg && msg.length > 0 ? msg.substring(0, 350) : undefined
 }
 
 export function getTelemetryReason(error: unknown | undefined): string | undefined {
@@ -568,6 +576,16 @@ function hasCode<T>(error: T): error is T & { code: string } {
     return typeof (error as { code?: unknown }).code === 'string'
 }
 
+/**
+ * Returns the identifier the given error.
+ * Depending on the implementation, the identifier may exist on a
+ * different property.
+ */
+export function getErrorId(error: Error): string {
+    // prioritize code over the name
+    return hasCode(error) ? error.code : error.name
+}
+
 function hasTime(error: Error): error is typeof error & { time: Date } {
     return (error as { time?: unknown }).time instanceof Date
 }

packages/core/src/test/codewhisperer/commands/startSecurityScan.test.ts

@@ -385,7 +385,7 @@ describe('startSecurityScan', function () {
             codewhispererCodeScanScope: 'PROJECT',
             result: 'Failed',
             reason: 'CodeScanJobFailedError',
-            reasonDesc: 'Security scan failed.',
+            reasonDesc: 'CodeScanJobFailedError: Security scan failed.',
             passive: false,
         } as unknown as CodewhispererSecurityScan)
     })
@@ -413,7 +413,7 @@ describe('startSecurityScan', function () {
             codewhispererCodeScanScope: 'PROJECT',
             result: 'Failed',
             reason: 'ThrottlingException',
-            reasonDesc: 'Maximum project scan count reached for this month.',
+            reasonDesc: 'ThrottlingException: Maximum project scan count reached for this month.',
             passive: false,
         } as unknown as CodewhispererSecurityScan)
     })
@@ -444,7 +444,7 @@ describe('startSecurityScan', function () {
             codewhispererCodeScanScope: 'FILE',
             result: 'Failed',
             reason: 'ThrottlingException',
-            reasonDesc: 'Maximum auto-scans count reached for this month.',
+            reasonDesc: 'ThrottlingException: Maximum auto-scans count reached for this month.',
             passive: true,
         } as unknown as CodewhispererSecurityScan)
     })

packages/core/src/test/shared/errors.test.ts

@@ -20,6 +20,7 @@ import {
     tryRun,
     UnknownError,
     DiskCacheError,
+    getErrorId,
 } from '../../shared/errors'
 import { CancellationError } from '../../shared/utilities/timeoutUtils'
 import { UnauthorizedException } from '@aws-sdk/client-sso'
@@ -447,6 +448,22 @@ describe('util', function () {
         )
     })
 
+    it('getErrorId', function () {
+        let error = new Error()
+        assert.deepStrictEqual(getErrorId(error), 'Error')
+
+        error = new Error()
+        error.name = 'MyError'
+        assert.deepStrictEqual(getErrorId(error), 'MyError')
+
+        error = new ToolkitError('', { code: 'MyCode' })
+        assert.deepStrictEqual(getErrorId(error), 'MyCode')
+
+        // `code` takes priority over `name`
+        error = new ToolkitError('', { code: 'MyCode', name: 'MyError' })
+        assert.deepStrictEqual(getErrorId(error), 'MyCode')
+    })
+
     it('getErrorMsg()', function () {
         assert.deepStrictEqual(
             getErrorMsg(
@@ -465,23 +482,36 @@ describe('util', function () {
         assert.deepStrictEqual(getErrorMsg(awsErr), 'aws error desc 1')
 
         // Arg withCause=true
-        awsErr = new TestAwsError('ValidationException', 'aws validation msg 1', new Date())
         let toolkitError = new ToolkitError('ToolkitError Message')
         assert.deepStrictEqual(getErrorMsg(toolkitError, true), 'ToolkitError Message')
 
+        awsErr = new TestAwsError('ValidationException', 'aws validation msg 1', new Date())
         toolkitError = new ToolkitError('ToolkitError Message', { cause: awsErr })
-        assert.deepStrictEqual(getErrorMsg(toolkitError, true), `ToolkitError Message | aws validation msg 1`)
+        assert.deepStrictEqual(
+            getErrorMsg(toolkitError, true),
+            `ToolkitError Message | ValidationException: aws validation msg 1`
+        )
 
-        const nestedNestedToolkitError = new ToolkitError('C')
-        const nestedToolkitError = new ToolkitError('B', { cause: nestedNestedToolkitError })
-        toolkitError = new ToolkitError('A', { cause: nestedToolkitError })
-        assert.deepStrictEqual(getErrorMsg(toolkitError, true), `A | B | C`)
+        const nestedNestedToolkitError = new Error('C')
+        nestedNestedToolkitError.name = 'NameC'
+        const nestedToolkitError = new ToolkitError('B', { cause: nestedNestedToolkitError, code: 'CodeB' })
+        toolkitError = new ToolkitError('A', { cause: nestedToolkitError, code: 'CodeA' })
+        assert.deepStrictEqual(getErrorMsg(toolkitError, true), `CodeA: A | CodeB: B | NameC: C`)
+
+        // Arg withCause=true excludes the generic 'Error' id
+        const errorWithGenericName = new Error('A') // note this does not set a value for `name`, by default it is 'Error'
+        assert.deepStrictEqual(getErrorMsg(errorWithGenericName, true), `A`)
+        errorWithGenericName.name = 'NameA' // now we set a `name`
+        assert.deepStrictEqual(getErrorMsg(errorWithGenericName, true), `NameA: A`)
     })
 
     it('getTelemetryReasonDesc()', () => {
         const err = new Error('Cause Message a/b/c/d.txt')
-        const toolkitError = new ToolkitError('ToolkitError Message', { cause: err })
-        assert.deepStrictEqual(getTelemetryReasonDesc(toolkitError), 'ToolkitError Message | Cause Message x/x/x/x.txt')
+        const toolkitError = new ToolkitError('ToolkitError Message', { cause: err, code: 'CodeA' })
+        assert.deepStrictEqual(
+            getTelemetryReasonDesc(toolkitError),
+            'CodeA: ToolkitError Message | Cause Message x/x/x/x.txt'
+        )
     })
 
     function makeSyntaxErrorWithSdkClientError() {

packages/core/src/test/shared/telemetry/spans.test.ts

@@ -258,7 +258,7 @@ describe('TelemetryTracer', function () {
             assertTelemetry('aws_loginWithBrowser', {
                 result: 'Failed',
                 reason: 'InvalidRequestException',
-                reasonDesc: 'Invalid client ID provided',
+                reasonDesc: 'InvalidRequestException: Invalid client ID provided',
                 httpStatusCode: '400',
             })
             const metric = getMetrics('aws_loginWithBrowser')[0]