Skip to content

Commit 9d8ddbd

Browse files
nkomonen-amazonnkomonen-amazon
authored
fix(auth): handle SyntaxError edge case in refresh credentials (#5526)
## Problem: There is a small edge case where there are SOMETIMES missing fields in a SyntaxError, in this case the $response or its reason field do not exist. Due to this those cases are causing a connection to become invalid when refresh credentials fails. ## Solution: Handle the case where those fields are missing and gracefully handle them by still marking it as an AwsClientResponseError. Since we cannot extract the underlying error message, we set a custom errors message for those cases. Now this will not cause a failed refresh credential to invalidate. --- <!--- 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]>
1 parent ebb0c92 commit 9d8ddbd

File tree

4 files changed

+83
-17
lines changed

4 files changed

+83
-17
lines changed

‎packages/amazonq/.changes/next-release/Bug Fix-b8b46fa8-b467-4b35-9741-ce6962c8f674.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Fix SyntaxError causing premature expiration (edge case)"
4+
}

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,7 @@ export class AwsClientResponseError extends Error {
941941
*/
942942
static instanceIf<T>(err: T): AwsClientResponseError | T {
943943
const reason = AwsClientResponseError.tryExtractReasonFromSyntaxError(err)
944-
if (reason && reason.startsWith('SDK Client unexpected error response: data response code:')) {
944+
if (reason) {
945945
getLogger().debug(`Creating AwsClientResponseError from SyntaxError: %O`, err)
946946
return new AwsClientResponseError(err)
947947
}
@@ -953,16 +953,31 @@ export class AwsClientResponseError extends Error {
953953
* Otherwise returning undefined.
954954
*/
955955
static tryExtractReasonFromSyntaxError(err: unknown): string | undefined {
956-
if (!(err instanceof SyntaxError)) {
956+
if (
957+
!(
958+
err instanceof SyntaxError &&
959+
err.message.includes('inspect the hidden field {error}.$response on this object')
960+
)
961+
) {
957962
return undefined
958963
}
959964

960965
// See the class docstring to explain how we know the existence of the following keys
961-
if (hasKey(err, '$response')) {
966+
if (hasKey(err, '$response') && err['$response'] !== undefined) {
962967
const response = err['$response']
963-
if (response && hasKey(response, 'reason')) {
964-
return response['reason'] as string
968+
if (response) {
969+
if (hasKey(response, 'reason') && response['reason'] !== undefined) {
970+
return response['reason'] as string
971+
} else {
972+
// We were seeing some cases in telemetry where a syntax error made it all the way to this point
973+
// but then may have not had a 'reason'.
974+
return `No 'reason' field in '$response' | ${JSON.stringify(response)} | ${err.message}`
975+
}
965976
}
977+
} else {
978+
// We were seeing some cases in telemetry where a syntax error made it all the way to this point
979+
// but then may have not had a '$response'.
980+
return `No '$response' field in SyntaxError | ${err.message}`
966981
}
967982

968983
return undefined

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

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,10 @@ describe('util', function () {
484484
})
485485

486486
function makeSyntaxErrorWithSdkClientError() {
487-
const syntaxError: Error = new SyntaxError('The SyntaxError message')
487+
// The following error messages are not arbitrary, changing them can break functionality
488+
const syntaxError: Error = new SyntaxError(
489+
'Unexpected token \'<\', "<html><bod"... is not valid JSON Deserialization error: to see the raw response, inspect the hidden field {error}.$response on this object.'
490+
)
488491
// Under the hood of a SyntaxError may be a hidden field with the real reason for the failure
489492
;(syntaxError as any)['$response'] = { reason: 'SDK Client unexpected error response: data response code: 500' }
490493
return syntaxError
@@ -537,18 +540,58 @@ describe('util', function () {
537540
)
538541
})
539542

540-
it('AwsClientResponseError', function () {
541-
const syntaxError = makeSyntaxErrorWithSdkClientError()
543+
describe('AwsClientResponseError', function () {
544+
it('handles the happy path cases', function () {
545+
const syntaxError = makeSyntaxErrorWithSdkClientError()
542546

543-
assert.deepStrictEqual(
544-
AwsClientResponseError.tryExtractReasonFromSyntaxError(syntaxError),
545-
'SDK Client unexpected error response: data response code: 500'
546-
)
547-
const responseError = AwsClientResponseError.instanceIf(syntaxError)
548-
assert(!(responseError instanceof SyntaxError))
549-
assert(responseError instanceof Error)
550-
assert(responseError instanceof AwsClientResponseError)
551-
assert(responseError.message === 'SDK Client unexpected error response: data response code: 500')
547+
assert.deepStrictEqual(
548+
AwsClientResponseError.tryExtractReasonFromSyntaxError(syntaxError),
549+
'SDK Client unexpected error response: data response code: 500'
550+
)
551+
const responseError = AwsClientResponseError.instanceIf(syntaxError)
552+
assert(!(responseError instanceof SyntaxError))
553+
assert(responseError instanceof Error)
554+
assert(responseError instanceof AwsClientResponseError)
555+
assert(responseError.message === 'SDK Client unexpected error response: data response code: 500')
556+
})
557+
558+
it('gracefully handles a SyntaxError with missing fields', function () {
559+
let syntaxError = makeSyntaxErrorWithSdkClientError()
560+
561+
// No message about a '$response' field existing
562+
syntaxError.message = 'This does not mention a "$response" field existing'
563+
assert(!(AwsClientResponseError.instanceIf(syntaxError) instanceof AwsClientResponseError))
564+
assert.equal(syntaxError, AwsClientResponseError.instanceIf(syntaxError))
565+
566+
// No '$response' field in SyntaxError
567+
syntaxError = makeSyntaxErrorWithSdkClientError()
568+
delete (syntaxError as any)['$response']
569+
assertIsAwsClientResponseError(syntaxError, `No '$response' field in SyntaxError | ${syntaxError.message}`)
570+
syntaxError = makeSyntaxErrorWithSdkClientError()
571+
;(syntaxError as any)['$response'] = undefined
572+
assertIsAwsClientResponseError(syntaxError, `No '$response' field in SyntaxError | ${syntaxError.message}`)
573+
574+
// No 'reason' in '$response'
575+
syntaxError = makeSyntaxErrorWithSdkClientError()
576+
let response = (syntaxError as any)['$response']
577+
delete response['reason']
578+
assertIsAwsClientResponseError(
579+
syntaxError,
580+
`No 'reason' field in '$response' | ${JSON.stringify(response)} | ${syntaxError.message}`
581+
)
582+
syntaxError = makeSyntaxErrorWithSdkClientError()
583+
response = (syntaxError as any)['$response']
584+
response['reason'] = undefined
585+
assertIsAwsClientResponseError(
586+
syntaxError,
587+
`No 'reason' field in '$response' | ${JSON.stringify(response)} | ${syntaxError.message}`
588+
)
589+
590+
function assertIsAwsClientResponseError(e: Error, expectedMessage: string) {
591+
assert.deepStrictEqual(AwsClientResponseError.tryExtractReasonFromSyntaxError(e), expectedMessage)
592+
assert(AwsClientResponseError.instanceIf(e) instanceof AwsClientResponseError)
593+
}
594+
})
552595
})
553596

554597
it('scrubNames()', async function () {

‎packages/toolkit/.changes/next-release/Bug Fix-9ae19be3-32f5-475c-8206-cb82f4ffe30e.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Fix SyntaxError causing premature expiration (edge case)"
4+
}

0 commit comments

Comments
 (0)