refactor: Strongly type and simplify auth error logic

* Use separate errors for init auth check throws and root auth error throws (to be implemented in each component)
* Consolidate auth issue discovery to one reusable function
* rethrow on testAuth based on auth error type and unrecoverable flag

src/backend/common/AbstractInitializable.ts

@@ -3,12 +3,10 @@ import {truncateStringToLength } from "../../core/StringUtils.ts";
 import { hasNodeNetworkException } from "./errors/NodeErrors.ts";
 import { hasUpstreamError } from "./errors/UpstreamError.ts";
 import type {WebhookPayload} from "./infrastructure/config/health/webhooks.ts";
-import { AuthCheckError, BuildDataError, ConnectionCheckError, ParseCacheError, PostInitError, StageError } from "./errors/MSErrors.ts";
+import { AuthCheckError, AuthError, BuildDataError, ConnectionCheckError, findAuthIssue, ParseCacheError, PostInitError, StageError, type AuthErrorMap } from "./errors/MSErrors.ts";
 import { messageWithCausesTruncatedDefault } from "../../core/ErrorUtils.ts";
 import { spawn } from 'abort-controller-x';
 import { COMPONENT_AUTH_TYPE, type ComponentAuthType } from "../../core/Atomic.ts";
-import { findCauseByFunc } from "../utils/ErrorUtils.ts";
-
 export default abstract class AbstractInitializable {
     requiresAuth: boolean = false;
     requiresAuthInteraction: boolean = false;
@@ -290,26 +288,19 @@ export default abstract class AbstractInitializable {
 
     canTryAuth = () => this.isUsable() && this.authGated() && !this.hasUnrecoverableAuthFailure()
 
-    findAuthFailure = () => this.errors.find(x => findCauseByFunc(x, (e) => e.name === 'Authentication Check' || e instanceof AuthCheckError));
-
-    hasAuthFailure = () => this.findAuthFailure !== undefined;
-
-    findUnrecoverableAuthFailure = (): AuthCheckError | undefined => {
-        const err = this.findAuthFailure();
-        if(err !== undefined) {
-            const authErr = findCauseByFunc(err, (e) => e.name === 'Authentication Check' || e instanceof AuthCheckError);
-            if(authErr === undefined) {
-                // this shouldn't happen?
-                return undefined;
-            }
-            if((authErr as AuthCheckError).unrecoverable) {
-                return authErr as AuthCheckError;
+    findAuthIssue = <T extends keyof AuthErrorMap = 'auth'>(opts: Parameters<typeof findAuthIssue<T>>[1] = {}): AuthErrorMap[T] | undefined => {
+        for(const err of this.errors) {
+            const authError = findAuthIssue(err, opts);
+            if(authError !== undefined) {
+                return authError;
             }
         }
         return undefined;
     }
 
-    hasUnrecoverableAuthFailure = () => this.findUnrecoverableAuthFailure() !== undefined;
+    hasAuthIssue = () => this.findAuthIssue() !== undefined;
+
+    hasUnrecoverableAuthFailure = () => this.findAuthIssue({unrecoverable: true}) !== undefined;
 
     canAuthUnattended = () => !this.authGated() || this.authType === COMPONENT_AUTH_TYPE.unattended || (this.authType === COMPONENT_AUTH_TYPE.interactive && !this.hasUnrecoverableAuthFailure()) ;
 
@@ -327,18 +318,26 @@ export default abstract class AbstractInitializable {
             this.logger.debug('Auth OK but step was forced');
         }
 
-        const unrecoverableAuth = this.findUnrecoverableAuthFailure();
+        // only throw *before* testing if we have previously tested and the error was unrecoverable
+        // there is no reason to constantly retry auth when we already know it won't succeed
+        //
+        // test retries should be forced by api calls and auth callback flows *after* we have made changes to component credentials
+        const unrecoverableAuth = this.findAuthIssue({unrecoverable: true});
         if(unrecoverableAuth !== undefined) {
+            let unrecoverable: boolean | undefined,
+            cause: Error;
             if(!force) {
-                if(unrecoverableAuth.cause === undefined || !(unrecoverableAuth.cause instanceof AuthCheckError)) {
-                    if(this.requiresAuthInteraction) {
-                        throw new AuthCheckError('Authentication failure: Will not retry auth because user interaction is required for authentication', {cause: unrecoverableAuth, unrecoverable: true});
-                    }
-                    throw new AuthCheckError('Authentication failure: Will not retry auth because authentication previously failed and must be reauthenticated', {cause: unrecoverableAuth, unrecoverable: true});
+                if(unrecoverableAuth instanceof AuthCheckError) {
+                    // we threw a fallback AuthCheckError
+                    unrecoverable = true;
+                    cause = unrecoverableAuth.cause as Error;
                 } else {
-                    // this is one of the errors above and we retrying n+1 times so just keep throwing the same error
-                    throw unrecoverableAuth;
+                    cause = unrecoverableAuth;
                 }
+                if(this.authType === COMPONENT_AUTH_TYPE.interactive) {
+                    throw new AuthCheckError('Authentication failure: Will not retry auth because user interaction is required for authentication', {cause, unrecoverable});
+                }
+                throw new AuthCheckError('Authentication failure: Will not retry auth because authentication previously failed and must be reauthenticated', {cause, unrecoverable});
             }
             this.logger.debug('Auth previously failed for non upstream/network reasons but retry is being forced');
         }
@@ -347,10 +346,18 @@ export default abstract class AbstractInitializable {
             await this.doAuthentication();
             this.authed = true;
         } catch (e) {
-            // only signal as auth failure if error was NOT either a node network error or a non-showstopping upstream error
-            const unrecoverable = !(hasNodeNetworkException(e) || hasUpstreamError(e, false));
+            let unrecoverableMsg: boolean,
+            unrecoverable: boolean | undefined;
+            if(e instanceof AuthError) {
+                unrecoverableMsg = e.unrecoverable;
+            } else {
+                // if component auth test didn't throw AuthError (it should have!)
+                // then fallback teo determining by these conditions
+                unrecoverable = !(hasNodeNetworkException(e) || hasUpstreamError(e, false));
+                unrecoverableMsg = unrecoverable;
+            }
             this.authed = false;
-            throw new AuthCheckError(`Authentication test failed!${unrecoverable === false ? ' Due to a network issue. Will retry authentication on next heartbeat.' : ''}`, {cause: e, unrecoverable});
+            throw new AuthCheckError(`Authentication test failed!${unrecoverableMsg === false ? ' Due to a network issue. Will retry authentication on next heartbeat.' : ''}`, {cause: e, unrecoverable});
         }
     }
 

src/backend/common/errors/MSErrors.ts

@@ -33,8 +33,8 @@ export class ConnectionCheckError extends StageError {
 }
 addKnownErrorConstructor(ConnectionCheckError);
 
-export class AuthCheckError extends StageError {
-    override name = 'Authentication Check';
+export class AuthError extends NamedError {
+    override name = 'Authentication Error';
     unrecoverable: boolean = false;
 
     public constructor(msg?: string, opts: ErrorOptions & {unrecoverable?: boolean} = {}) {
@@ -42,7 +42,19 @@ export class AuthCheckError extends StageError {
         this.unrecoverable = opts.unrecoverable ?? false;
     }
 }
+addKnownErrorConstructor(AuthError);
+export const isAuthError = (e: Error): e is AuthError => e.name === 'Authentication Error' || e instanceof AuthError;
+export class AuthCheckError extends StageError {
+    override name = 'Authentication Check';
+    unrecoverable?: boolean;
+
+    public constructor(msg?: string, opts: ErrorOptions & {unrecoverable?: boolean} = {}) {
+        super(msg, opts);
+        this.unrecoverable = opts.unrecoverable;
+    }
+}
 addKnownErrorConstructor(AuthCheckError);
+export const isAuthCheckError = (e: Error): e is AuthCheckError => e.name === 'Authentication Check' || e instanceof AuthCheckError;
 
 export class PostInitError extends StageError {
     name = 'Post Initialization';
@@ -206,3 +218,33 @@ export class InvalidRegexError extends SimpleError {
     }
 }
 addKnownErrorConstructor(InvalidRegexError, () => new InvalidRegexError(new RegExp(/1/)));
+
+export type AuthErrorMap = {
+    root: AuthError
+    check: AuthCheckError
+    auth: AuthError | AuthCheckError
+    parent: Error
+}
+
+export const findAuthIssue = <T extends keyof AuthErrorMap = 'auth'>(err: Error, opts: { type?: T, unrecoverable?: boolean } = {}): AuthErrorMap[T] | undefined => {
+    const {
+        type = 'auth',
+        unrecoverable
+    } = opts;
+    const authError = findCauseByFunc(err, (e) => {
+        if (['check','auth'].includes(type) && isAuthCheckError(e)) {
+            return unrecoverable === undefined || e.unrecoverable === unrecoverable;
+        }
+        if (['root','auth'].includes(type) && isAuthError(e)) {
+            return unrecoverable === undefined || e.unrecoverable === unrecoverable;
+        }
+        return false;
+    });
+    if (authError !== undefined) {
+        if (type === 'parent') {
+            return err as AuthErrorMap[T];
+        }
+        return authError as AuthErrorMap[T];
+    }
+    return undefined;
+}