With a valid refresh token, if signInWithStoredCredentials() fails with anything other than a 401, authenticationState stays on inProgress forever which can lead an app sits on a loading screen with no way out.
In _refreshSession's catch block, the success path sets _loading = false (via setSession), and the 401 path resets it via signOut() / clearSession(), but the rest of the block leaves _loading true and just rethrows. Since the getter returns inProgress while _loading is true, the state machine never moves forward. #178 actually surfaced this (previously, when _loading started at false, a bootstrap failure at least landed at signedOut though obviously that had other problems).
One way to fix would be to reset _loading and fire the state-changed callback on the non-401 path, which just preserves the current "throw on failure" behavior. Something like:
} on Exception catch (e, st) {
if (e is ApiException && e.statusCode == unauthorizedStatus) {
log.finest('Unauthorized refresh token. Forcing signout.');
await signOut();
} else if (_loading) {
// NEW: set loading false on non-401 failures so consumers
// observing addAuthStateChangedCallback can move forward.
_loading = false;
_onAuthStateChanged(authenticationState);
}
log.severe('Exception during token refresh', e, st);
_sessionCompleter?.completeError(e, st);
_onTokenRefreshFailure(e, st);
rethrow;
}However, looking at nhost-js, it seems that on a non-expired session, nhost-js actually just returns the cached session instead of throwing (see refreshSession.ts#L123-L129). This would be a bigger behavior change but if nhost-js represents the desired behavior then maybe nhost-dart should match it?
I'm happy to make a PR but not sure which direction is preferred.
With a valid refresh token, if
signInWithStoredCredentials()fails with anything other than a 401,authenticationStatestays oninProgressforever which can lead an app sits on a loading screen with no way out.In
_refreshSession's catch block, the success path sets_loading = false(viasetSession), and the 401 path resets it viasignOut()/clearSession(), but the rest of the block leaves_loadingtrue and just rethrows. Since the getter returnsinProgresswhile_loadingis true, the state machine never moves forward. #178 actually surfaced this (previously, when_loadingstarted at false, a bootstrap failure at least landed atsignedOutthough obviously that had other problems).One way to fix would be to reset
_loadingand fire the state-changed callback on the non-401 path, which just preserves the current "throw on failure" behavior. Something like:However, looking at nhost-js, it seems that on a non-expired session, nhost-js actually just returns the cached session instead of throwing (see
refreshSession.ts#L123-L129). This would be a bigger behavior change but if nhost-js represents the desired behavior then maybe nhost-dart should match it?I'm happy to make a PR but not sure which direction is preferred.