Fix: Don't retry IAM display if 410 is received from backend#2158
Merged
jennantilla merged 3 commits intomainfrom Aug 5, 2024
Merged
Fix: Don't retry IAM display if 410 is received from backend#2158jennantilla merged 3 commits intomainfrom
jennantilla merged 3 commits intomainfrom
Conversation
- Changed `messages` from an immutable list to a mutable list
jennantilla
force-pushed
the
fix/iam_410
branch
2 times, most recently
from
d4b4fce to
1896f10
Compare
- Added synchronization to `makeRedisplayMessagesAvailableWithTriggers` and `attemptToShowInAppMessage` to prevent concurrent modification issues - Refactored `evaluateInAppMessages` to collect messages for display inside a synchronized block and process them outside to avoid suspension points within critical sections
jinliu9508
reviewed
Aug 1, 2024
Contributor
jinliu9508
left a comment
jinliu9508
left a comment
There was a problem hiding this comment.
Overall, the change works and I am no longer getting repeating 410 errors. Nice work!
| private val fetchIAMMutex = Mutex() | ||
| private var lastTimeFetchedIAMs: Long? = null | ||
|
|
||
| private val lock = Any() |
Contributor
There was a problem hiding this comment.
It looks like this lock is only for synchronizing 'messages'. In this case we can just call 'synchronized(messages)' instead of introducing a temp lock instance.
- Changed `synchronized(lock)` to `synchronized(messages)`
jinliu9508
approved these changes
Aug 5, 2024
Merged
This was referenced Aug 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
Don't retry IAM display if 410 is received from backend
Details
Motivation
In instances where an IAM is disabled mid-session, a 410 is received and is caught in a retrying loop. This change will prevent the retry and just not display the IAM, since it just become disabled.
Scope
Changes to
InAppMessagesManageronly.Since this change involves changing the
messageslist, synchronization was added tomakeRedisplayMessagesAvailableWithTriggersandattemptToShowInAppMessageto prevent concurrent modification issues.evaluateInAppMessageswas further updated to collect messages for display inside a synchronized block and process them outside to avoid suspension points within critical sections.Testing
Manual testing
Forced reproduction by setting breakpoint right before IAM is set to display, pausing the IAM from the dashboard, then resuming app flow. A 410 will result.
After updating the code, tested on an Android 14 emulator to see that app resumes normally after IAM is paused in the dashboard. Subsequent 410s are not received and app resumes as expected.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is