[App Check] Return error if refreshed token is nil #11625
Conversation
| [self.tokenRefresher updateWithRefreshResult:refreshResult]; | ||
| [self postTokenUpdateNotificationWithToken:token]; | ||
| return token; | ||
| if (token) { |
There was a problem hiding this comment.
Maybe I've been doing Swift too long, but why are lines 321 and 324 Nullable?
Should null be detected earlier and pass along nonnulls?
There was a problem hiding this comment.
I think you're right, but I suspect that there may have been a case where line 321 was justified. In that case, setting a nil token in storage would wipe storage– which may be desireable.
I don't however think that line 324 should use a non-nil token.
There was a problem hiding this comment.
I'm not sure that line 324 should be non-nil. [self.storage setToken:token] on line 322 resolves the promise with nil unless there's an error removing the token from storage here.
|
Please close, merge, or comment. We plan to close stale PRs on November 28, 2023. |
|
This is still an active crasher for us as of 10.13.0. Is there a reason why this was closed vs. merged in? |
@bryansum Would you mind filing a new bug with any debugging info you can provide? My main hesitation with merging this is that it doesn't fix the underlying problem, just hides it, since the providers should never return |
|
@bryansum Do you still see the crash with 10.17.0 or newer? |
|
I'll upgrade soon and let you know. Thanks for the response. |
|
Updated to 10.17.0 and still seeing some instances of this crash: Unfortunately no debugging information that I can provide easily. We have a fair number of active users, so the rate is definitely low. |
Added a
nilcheck inFIRAppCheck'srefreshTokenmethod.This handles an edge-case (still under investigation) where
FIRAppCheckProvidergetTokenWithCompletion:'s handler is called withnilfor both thetokenanderror, which results in a crash inpostTokenUpdateNotificationWithToken:(attempting to populate anNSDictionaryvalue withnil).