AuthError error description should be exhaustive

[peterkos]

Chore

Describe the chore

Given that every error is "known", it should be possible to always have an errorDescription. This is all correctly implemented, BUT errorDescription is still optional -- there is no reason to be optional if we never return nil, no?

supabase-swift/Sources/Auth/AuthError.swift

Lines 53 to 65 in 4c966ea

	public var errorDescription: String? {
	switch self {
	case let .api(error): error.errorDescription ?? error.msg ?? error.error
	case .missingExpClaim: "Missing expiration claim in the access token."
	case .malformedJWT: "A malformed JWT received."
	case .sessionNotFound: "Unable to get a valid session."
	case let .pkce(reason): reason.errorDescription
	case .invalidImplicitGrantFlowURL: "Not a valid implicit grant flow url."
	case .missingURL: "Missing URL."
	case .invalidRedirectScheme: "Invalid redirect scheme."
	}
	}
	}

Additional context

This would allow for consistent error handling:

let supabaseResponse: Supabase.AuthResponse
do {
    supabaseResponse = try await client.auth.signUp(/* ... */)
} catch let error as AuthError {
    return .failure(.supabaseError(error.errorDescription))
} catch {
    return .failure(.supabaseError(error.localizedDescription))
}

Versus currently, where this workaround is needed:

let supabaseResponse: Supabase.AuthResponse
do {
    supabaseResponse = try await client.auth.signUp(/* ... */)
} catch let error as AuthError {
    return .failure(.supabaseError(error.errorDescription ?? "Who the hell knows what happened"))
} catch {
    return .failure(.supabaseError(error.localizedDescription))
}

Ideally once Swift 6 comes out, typed throws are implemented too but that's probably best for a separate issue :)

[grdsdev]

Hi, I understand it but the errorDescription here is the LocalizedError''s protocol requirement, and it is defined as optional. So, not sure how we can improve on that.

[peterkos]

Oh! Apllogies -- I was looking at Error's protocol, not LocalizedError. Good point!

Maybe this could be folded in as part of #396? Or, some of the duplicated properties could be cleaned up in AuthError.APIError:

public struct APIError: Error, Decodable, Sendable, Equatable {
    // These three should really be one
    public var msg: String?
    public var error: String?
    public var errorDescription: String? 

    public var code: Int?
    public var weakPassword: WeakPassword? // ~= [Reasons]
}

// AuthError
public var errorDescription: String? {
    switch self {
    case let .api(error): // ...
    // ...
    }
}

And what's the intended way to filter the errors? Some properties (like msg) say "usually missing if error is present", so is this the intention?

do { 
  // ...
} catch let error as AuthError {
    guard case let .api(apiError) = error else {
        // Custom error result handler
        return .failure(.supabaseError(error.localizedDescription))
    }

    return if let error = apiError.error {
        .failure(.supabaseError(error))
    } else if let msg = apiError.msg {
        .failure(.supabaseError(msg))
    } else {
        .failure(.supabaseError("who knows"))
    }
}

[grdsdev]

Hi @peterkos I refactored the whole AuthError type, and added support for error codes in the PR.

I'd appreciate if you could take it for a spin, and let me know if that improves error handling for you.

[grdsdev]

I'll consider this got fixed by #518