fix(android): avoid dropping the notification when a messaging style person times out - #68
Open
drhops wants to merge 4 commits into
Open
fix(android): avoid dropping the notification when a messaging style person times out#68drhops wants to merge 4 commits into
drhops wants to merge 4 commits into
Conversation
…on times out getMessagingStyleTask awaits each person future with a 20s timeout it never catches -- the only image-loading path in this file that doesn't. When the deadline expires the TimeoutException escapes the callable, NotificationManager awaits the style with a bare get(), and displayNotification rejects, so nothing is posted at all. getPerson already bounds its own icon fetch at 10s and degrades to an icon-less Person, so this outer deadline expires only when the process was frozen mid-fetch (Android's cached-app freezer, common on 15/16 for headless FCM handlers). Guava reports it as "Waited 20 seconds (plus 200 seconds ... delay)" -- the waiting thread descheduled long past its deadline. Catch it and degrade the same way the BigPicture, LargeIcon and person-icon paths already do: fall back to the person built from everything in the bundle except the remotely fetched icon, so the sender keeps their name, key and uri and only the avatar is lost.
Injects an executor whose person futures always time out on their timed get, which is what the caller observes when the process was frozen mid-fetch. A slow network cannot reach this path -- getPerson bounds its own icon fetch at 10s and degrades -- so the stub is the only way to exercise it without a real freeze, and it keeps the test instant rather than burning the 20s deadline. Three of the four cases fail against the unpatched getMessagingStyleTask; the fourth guards the builder now shared by the normal and timed-out paths.
mapsEveryBundleField skipped getPersonBuilder's bot branch, and the timeout cases only checked name/key/uri -- so nothing proved the fallback applies the full builder rather than just the name.
The icon decode sits outside getPerson()'s try block, so the lookup can fail outright as well as time out. Both cost an avatar; both were costing the whole notification. InterruptedException still propagates: it means the thread is being torn down, not that the person is unavailable.
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
1 participant
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.
Summary
getMessagingStyleTaskawaits eachgetPerson()future with a 20s timeout that isn't caught. When it expires, theTimeoutExceptionescapes the callable,NotificationManagerawaits the style with a barestyleTask.get(), anddisplayNotificationrejects it, dropping the notification.Why the deadline expires
getPerson()already bounds its icon fetch at 10s and degrades to an icon-lessPerson, so it's likely a process stall, and we see the exception occur 200s past its deadline:Fix
Catch it and degrade in a similar way the BigPicture, LargeIcon and person-icon paths do by logging and continuing without the image. The fallback keeps everything except the icon. By extracting
getPersonBuilder(Bundle)fromgetPersonso the timeout path reuses the same field mapping rather than duplicating it, which means the sender retains their name, key and uri.