Skip to content

Commit 4f2d93d

Browse files
authored
fix(telemetry): try to handle SyntaxError for createToken requests (#5414)
Problem: Rare and unreproducible case where SDK client calls to create an SSO token return an html page instead of JSON data. It is probably some error page indicating an interference with the request. Solution: Try to extract the reason and status code from the page if possible, and report it in telemetry.
1 parent 2270a48 commit 4f2d93d

File tree

3 files changed

+47
-22
lines changed

3 files changed

+47
-22
lines changed

packages/core/src/auth/sso/clients.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { pageableToCollection, partialClone } from '../../shared/utilities/colle
2828
import { assertHasProps, isNonNullable, RequiredProps, selectFrom } from '../../shared/utilities/tsUtils'
2929
import { getLogger } from '../../shared/logger'
3030
import { SsoAccessTokenProvider } from './ssoAccessTokenProvider'
31-
import { isClientFault } from '../../shared/errors'
31+
import { ToolkitError, getHttpStatusCode, getReasonFromSyntaxError, isClientFault } from '../../shared/errors'
3232
import { DevSettings } from '../../shared/settings'
3333
import { SdkError } from '@aws-sdk/types'
3434
import { HttpRequest, HttpResponse } from '@smithy/protocol-http'
@@ -86,7 +86,25 @@ export class OidcClient {
8686
}
8787

8888
public async createToken(request: CreateTokenRequest) {
89-
const response = await this.client.createToken(request as CreateTokenRequest)
89+
let response
90+
try {
91+
response = await this.client.createToken(request as CreateTokenRequest)
92+
} catch (err) {
93+
// In rare cases the SDK client may get unexpected data from its API call.
94+
// This will throw a SyntaxError that contains the returned data.
95+
if (err instanceof SyntaxError) {
96+
getLogger().error(`createToken: SSOIDC Client received an unexpected non-JSON response: %O`, err)
97+
throw new ToolkitError(
98+
`SDK Client unexpected error response: data response code: ${getHttpStatusCode(err)}, data reason: ${getReasonFromSyntaxError(err)}`,
99+
{
100+
code: `${getHttpStatusCode(err)}`,
101+
cause: err,
102+
}
103+
)
104+
} else {
105+
throw err
106+
}
107+
}
90108
assertHasProps(response, 'accessToken', 'expiresIn')
91109

92110
return {

packages/core/src/codewhispererChat/controllers/chat/controller.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import { randomUUID } from '../../../shared/crypto'
4747
import { LspController } from '../../../amazonq/lsp/lspController'
4848
import { CodeWhispererSettings } from '../../../codewhisperer/util/codewhispererSettings'
4949
import { getSelectedCustomization } from '../../../codewhisperer/util/customizationUtil'
50-
import { getHttpStatusCode } from '../../../shared/errors'
50+
import { getHttpStatusCode, getReasonFromSyntaxError } from '../../../shared/errors'
5151

5252
export interface ChatControllerMessagePublishers {
5353
readonly processPromptChatMessage: MessagePublisher<PromptMessage>
@@ -298,24 +298,7 @@ export class ChatController {
298298
errorMessage = e.toUpperCase()
299299
} else if (e instanceof SyntaxError) {
300300
// Workaround to handle case when LB returns web-page with error and our client doesn't return proper exception
301-
// eslint-disable-next-line no-prototype-builtins
302-
if (e.hasOwnProperty('$response')) {
303-
type exceptionKeyType = keyof typeof e
304-
const responseKey = '$response' as exceptionKeyType
305-
const response = e[responseKey]
306-
307-
let reason
308-
309-
if (response) {
310-
type responseKeyType = keyof typeof response
311-
const reasonKey = 'reason' as responseKeyType
312-
reason = response[reasonKey]
313-
}
314-
315-
errorMessage = reason ? (reason as string) : defaultMessage
316-
} else {
317-
errorMessage = defaultMessage
318-
}
301+
errorMessage = getReasonFromSyntaxError(e) ?? defaultMessage
319302
} else if (e instanceof CodeWhispererStreamingServiceException) {
320303
errorMessage = e.message
321304
requestID = e.$metadata.requestId

packages/core/src/shared/errors.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { ServiceException } from '@aws-sdk/smithy-client'
99
import { isThrottlingError, isTransientError } from '@smithy/service-error-classification'
1010
import { Result } from './telemetry/telemetry'
1111
import { CancellationError } from './utilities/timeoutUtils'
12-
import { isNonNullable } from './utilities/tsUtils'
12+
import { hasKey, isNonNullable } from './utilities/tsUtils'
1313
import type * as nodefs from 'fs'
1414
import type * as os from 'os'
1515
import { CodeWhispererStreamingServiceException } from '@amzn/codewhisperer-streaming'
@@ -830,3 +830,27 @@ function isVSCodeProxyError(err: Error): boolean {
830830
function isSocketTimeoutError(err: Error): boolean {
831831
return err.name === 'TimeoutError' && err.message.includes('Connection timed out after')
832832
}
833+
834+
/**
835+
* AWS SDK clients may rarely make requests that results in something other than JSON data
836+
* being returned (e.g html). This will cause the client to throw a SyntaxError as a result
837+
* of attempt to deserialize the non-JSON data.
838+
* While the contents of the response may contain sensitive information, there may be a reason
839+
* for failure embedded. This function attempts to extract that reason.
840+
*
841+
* If the reason cannot be found or the error is not a SyntaxError, return undefined.
842+
*/
843+
export function getReasonFromSyntaxError(err: Error): string | undefined {
844+
if (!(err instanceof SyntaxError)) {
845+
return undefined
846+
}
847+
848+
if (hasKey(err, '$response')) {
849+
const response = err['$response']
850+
if (response && hasKey(response, 'reason')) {
851+
return response['reason'] as string
852+
}
853+
}
854+
855+
return undefined
856+
}

0 commit comments

Comments
 (0)