Skip to content

Commit ed5bebd

Browse files
auth: add more network related errors to check (#4621)
Problem: We have a function isNetworkError that determines if a an error is network related. The issue is that we can see other network related errors in our telemetry metric aws_refreshCredentials that are not caught by isNetworkError. Solution: Add some of the network errors that we see in telemetry to the isNetworkError check. This will improve false token invalidations during the sso token refresh process since we were previously invalidating incorrectly on certain network issues Signed-off-by: Nikolas Komonen <[email protected]>
1 parent c4767fc commit ed5bebd

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

packages/core/src/auth/auth.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,6 @@ export class Auth implements AuthService, ConnectionManager {
550550
}
551551

552552
private async handleSsoTokenError(id: Connection['id'], err: unknown) {
553-
// Bubble-up networking issues so we don't treat the session as invalid
554553
this.throwOnNetworkError(err)
555554

556555
this.#validationErrors.set(id, UnknownError.cast(err))
@@ -725,7 +724,6 @@ export class Auth implements AuthService, ConnectionManager {
725724
private readonly getToken = keyedDebounce(this._getToken.bind(this))
726725
private async _getToken(id: Connection['id'], provider: SsoAccessTokenProvider): Promise<SsoToken> {
727726
const token = await provider.getToken().catch(err => {
728-
// Bubble-up networking issues so we don't treat the session as invalid
729727
this.throwOnNetworkError(err)
730728

731729
this.#validationErrors.set(id, err)
@@ -735,11 +733,11 @@ export class Auth implements AuthService, ConnectionManager {
735733
}
736734

737735
/**
738-
* Auth processes can fail if there are network issues, and we do not
739-
* want to intepret these failures as invalid/expired auth tokens.
736+
* Auth processes can fail due to reasons outside of authentication actually being expired.
737+
* We do not want to intepret these failures as invalid/expired auth tokens.
740738
*
741-
* We use this to check if the given error is network related and then
742-
* throw, expecting the caller to not change the state of the connection.
739+
* We use this to check if the given error is unanticipated. If it is we throw,
740+
* expecting the caller to not change the state of the connection.
743741
*/
744742
private throwOnNetworkError(e: unknown) {
745743
if (isNetworkError(e)) {

packages/core/src/shared/errors.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,5 +532,18 @@ export function isNetworkError(err?: unknown): err is Error & { code: string } {
532532
return false
533533
}
534534

535-
return ['ENOTFOUND', 'EAI_AGAIN', 'ECONNRESET', 'ECONNREFUSED', 'ETIMEDOUT'].includes(err.code)
535+
return [
536+
'ENOTFOUND',
537+
'EAI_AGAIN',
538+
'ECONNRESET',
539+
'ECONNREFUSED',
540+
'ETIMEDOUT',
541+
'ENETUNREACH',
542+
'ERR_TLS_CERT_ALTNAME_INVALID', // dns issue?
543+
'EPROTO', // due to legacy server "unsafe legacy renegotiation"?
544+
'EHOSTUNREACH',
545+
'EADDRINUSE',
546+
'ENOBUFS', // client side memory issue during http request?
547+
'EADDRNOTAVAIL', // port not available/allowed?
548+
].includes(err.code)
536549
}

0 commit comments

Comments
 (0)