-
Notifications
You must be signed in to change notification settings - Fork 31
[MOB-11813] - JWT in background - Rework #931
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
base: master
Are you sure you want to change the base?
Conversation
2667920
to
f3b1b26
Compare
At current state, I could still see refresh not happening when app is active & jwt in memory is within its refresh period. |
87e7f7b
to
6c0df8a
Compare
6c0df8a
to
2ce0ee8
Compare
Background/Foreground Auth Token ManagementBackground Prevention: JWT requests are now prevented when the app goes to background to avoid unnecessary network calls and improve performance.Foreground Token Refresh Logic:
Retry Policy Integration: The existing retry policy logic remains unchanged:
|
if (!isInForeground) { | ||
IterableLogger.w(TAG, "Auth token request skipped - app is in background"); | ||
pendingAuth = false; | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will now prevent the Auth token form getting requested to the app. This also does not increase the retryCount
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to call the handleAuthTokenSuccess in this scenario?
Is it possible that the client's app is waiting for the sdk to return something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because no token will be requested until this point, we wont have to call any success or failure callback.
The function is void method which only SDK internally calls for scheduling a request. Once we make a request to app, we call either a success or failure callback
@@ -151,7 +165,7 @@ public void queueExpirationRefresh(String encodedJWT) { | |||
if (triggerExpirationRefreshTime > 0) { | |||
scheduleAuthTokenRefresh(triggerExpirationRefreshTime, true, null); | |||
} else { | |||
IterableLogger.w(TAG, "The expiringAuthTokenRefreshPeriod has already passed for the current JWT"); | |||
scheduleAuthTokenRefresh(getNextRetryInterval(), true, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SDK was missing the opportunity to refresh the token when the trigger time had already passed the configured refresh window
String currentAuthToken = api.getAuthToken(); | ||
if (currentAuthToken != null) { | ||
// Reuse existing queueExpirationRefresh logic to check and schedule refresh if needed | ||
queueExpirationRefresh(currentAuthToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When App comes back to foreground, it continues with its usual scheduling
// Reuse existing queueExpirationRefresh logic to check and schedule refresh if needed | ||
queueExpirationRefresh(currentAuthToken); | ||
} else { | ||
IterableLogger.d(TAG, "No auth token available for expiration check"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For cases when auth token will be null in memory,
Like for Instances:
- App's server is overloaded and not responding
- App has not configured/ using JWT based API key
The app will not immediately follow scheduling auth Token request. When SDK makes a request and realizes missing of an auth token from backend is when it will continue to ask for new Auth token and following retry policy
1. ActivityMonitor removing callback doesnt need to be done. 2. encodedJWT can be null sometimes. SDK should still attempt to request jwt as per RetryPolicy (only when in foreground) 3. SDK to check email OR userID existence before proceeding to auth token check and retries
558eb97
to
f4aedbf
Compare
public void onSwitchToBackground() { | ||
IterableLogger.d(TAG, "App switched to background - disabling auth token requests"); | ||
isInForeground = false; | ||
clearRefreshTimer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we wrap in try catch blocks
IterableLogger.d(TAG, "App switched to foreground - enabling auth token requests"); | ||
isInForeground = true; | ||
|
||
checkAndHandleAuthRefresh(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we wrap in try catch blocks
Current state, the Test fails are not flaky. local running the tests are making it run properly but not when running the entire suite |
e550af7
to
b445b0c
Compare
@sumeruchat @davidtruong updated with try catch and review and comments on discussions initiated |
} catch (Exception e) { | ||
IterableLogger.e(TAG, "Error in onSwitchToBackground", e); | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we throwing the exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find! Thanks. No runtime exception needed. Replacing with Error log.
Im also seeing that isTimerScheduled is also checked within `scheduleAuthTokenRefresh` method. But somehow, without checking calling makes the test cases fail. So adding it as added checks are not harmful. Apart from that, surrounded background and foreground tasks in try catch as suggested in PR Reviews
b445b0c
to
50e002a
Compare
🔹 Jira Ticket(s) if any
✏️ Description