Skip to content

Commit 706a3a1

Browse files
Timeout error (aws#5289)
* fix: Treat TimeoutError as a NetworkError Problem: When we attempt to refresh an SSO session, we sometimes saw a TimeoutError in our telemetry as the reason for refresh failure. Solution: We treat the TimeoutError as a network error. See isSocketTimeoutError() docstring for more details. Signed-off-by: Nikolas Komonen <[email protected]> * refactor: sso client retry - After some disccusion with the identity team it may not be necessary to retry on InvalidGrant. See comment for more info - Removed the previous comment regarding other possible exceptions. After going through them, their docstrings point to malformed request arguments, so retrying probably will not help. - Reduced requestTimeout so we do not block on very slow responses. Signed-off-by: Nikolas Komonen <[email protected]> --------- Signed-off-by: Nikolas Komonen <[email protected]>
1 parent 5c13bff commit 706a3a1

File tree

2 files changed

+24
-18
lines changed

2 files changed

+24
-18
lines changed

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,9 @@ export class OidcClient {
102102
return true
103103
}
104104

105-
// Sessions may "expire" earlier than expected due to network faults.
106-
// TODO: add more cases from node_modules/@aws-sdk/client-sso-oidc/dist-types/models/models_0.d.ts
107-
// ExpiredTokenException
108-
// InternalServerException
109-
// InvalidClientException
110-
// InvalidRequestException
111-
// SlowDownException
112-
// UnsupportedGrantTypeException
113-
// InvalidRequestRegionException
114-
// InvalidRedirectUriException
115-
// InvalidRedirectUriException
105+
// As part of SIM IDE-10703, there was an assumption that retrying on InvalidGrantException
106+
// may be useful. This may not be the case anymore and if more research is done, this may not be needed.
107+
// TODO: setup some telemetry to see if there are any successes on a subsequent retry for this case.
116108
return err.name === 'InvalidGrantException'
117109
}
118110
const client = new SSOOIDC({
@@ -124,7 +116,9 @@ export class OidcClient {
124116
),
125117
customUserAgent: getUserAgent({ includePlatform: true, includeClientId: true }),
126118
requestHandler: {
127-
requestTimeout: 30_000,
119+
// This field may have a bug: https://github.com/aws/aws-sdk-js-v3/issues/6271
120+
// If the bug is real but is fixed, then we can probably remove this field and just have no timeout by default
121+
requestTimeout: 5000,
128122
},
129123
})
130124

packages/core/src/shared/errors.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,11 @@ export class PermissionsError extends ToolkitError {
768768
}
769769

770770
export function isNetworkError(err?: unknown): err is Error & { code: string } {
771-
if (isVSCodeProxyError(err)) {
771+
if (!(err instanceof Error)) {
772+
return false
773+
}
774+
775+
if (isVSCodeProxyError(err) || isSocketTimeoutError(err)) {
772776
return true
773777
}
774778

@@ -798,10 +802,18 @@ export function isNetworkError(err?: unknown): err is Error & { code: string } {
798802
*
799803
* Setting ID: http.proxy
800804
*/
801-
function isVSCodeProxyError(err?: unknown): boolean {
802-
if (!(err instanceof Error)) {
803-
return false
804-
}
805-
805+
function isVSCodeProxyError(err: Error): boolean {
806806
return err.name === 'Error' && err.message.startsWith('Failed to establish a socket connection to proxies')
807807
}
808+
809+
/**
810+
* When making SSO OIDC calls, we were seeing errors in telemetry and narrowing it down brings us to:
811+
* https://github.com/smithy-lang/smithy-typescript/blob/6aac850af4b5b07b3941854d21e3b0158aefcacb/packages/node-http-handler/src/set-socket-timeout.ts#L7
812+
* This looks to be thrown during http requests, so we'll consider it a network error.
813+
*
814+
* The scenario where we are actually getting the error though might actually be a bug:
815+
* https://github.com/aws/aws-sdk-js-v3/issues/6271
816+
*/
817+
function isSocketTimeoutError(err: Error): boolean {
818+
return err.name === 'TimeoutError' && err.message.includes('Connection timed out after')
819+
}

0 commit comments

Comments
 (0)