New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OAuth functionality doesn't retry because a 401/403 doesn't throw an error #6
Comments
I'm interested to see it. |
The way I read the code validation occurs on line #44 in the initial send function. I’m guessing it should be actuallySend though. Am I misreading it completely? |
You are right, it's in the wrong place. The validation should happen in actuallySend for authentication to work. I was combining multiple samples and ended up placing them in the wrong place. |
Addressed in 0.0.4 |
Here's a slightly quick and dirty example of my usual approach. Sorry, I've never had to write text about this approach before, so it got a bit long winded. 😄 - The callAsFunction is a matter of taste. It could just be a function, but this shifts a tiny bit of cognitive load. Pro: All potential retries are visual at the call site. Pro: You can pretty easily wrap different kinds of retries inside each other, and you'll have fall backs that are structured. If the outer one fails, it won't retry to the first one again. So it's deterministic, you will never have an infinite loop of retries. Pro: It'll fit the standard JWT + 2FA system neatly and can salvage calls all the way down. Most common scenario would be: 1. User has stale refreshToken, 2. User glitches login, 3. User needs to re-auth with some 2FA system. This is normally pretty complex and hard to read. Usually it's just not built with a final retry of the original call that failed in mind. And the code for doing this "without a retry" is often messier than handling the retry I think. Because you'll need a method to exit to the login screen from a failed call. struct Retry {
let client:APIClient
let doBeforeRetry:() async throws -> Void
let retryIfTrue:(Error) -> Bool
func callAsFunction<T:Decodable>(_ f:(APIClient) async throws ->(T)) async rethrows -> T {
do {
return try await f(client)
} catch {
guard retryIfTrue(error) else {
throw error
}
try await doBeforeRetry()
return try await f(client)
}
}
}
let SharedAPIClient = APIClient(configuration:
.init(host: Configuration.API.API_BASE_URL_STRING,
port: Configuration.API.API_PORT,
isInsecure: Configuration.API.API_SECURE == false,
delegate: APIDelegate()
))
struct APIPath {
static func with(ext:String) -> String {
return "\(Configuration.API.API_BASE_PATH)\(ext)"
}
}
let Auth = Retry(client: SharedAPIClient) {
fatalError() // Refresh token
} retryIfTrue: { error in
fatalError() // Check if error indicates that token needs to be refreshed
}
let Relogin = Retry(client: SharedAPIClient) {
fatalError() // Trigger the app to popover a login screen and log the user in via that
} retryIfTrue: { error in
fatalError() // Check if error indicates that login was wrong
}
struct User:Codable {}
func getUser() async throws -> User {
try await Relogin { _ in
try await Auth { client in
try await client.send(.get(APIPath.with(ext: "/users"))).value
}
}
} |
Got it. Yeah, I used a similar approach in one of the apps I worked on in the past. But I think centralization has more advantages:
|
Line #43 of APIClient has this:
Perhaps I'm mistaken, but a 401/403 doesn't actually throw an error here, it'll just continue a normal right? The
shouldClientRetry
would only be called if the network had some actually throwing error here, right?I usually approach OAuth in a completely different way, and if you agree I'd love to submit a PR with a different approach where auth'ed endpoints are wrapped at the call site, rather than handled inside of the API framework.
The text was updated successfully, but these errors were encountered: