Blank screen on MessageReactionList when returning to app after 10 minutes

[chrisbobbe]

There's a scenario on the 'message-reactions' route (MessageReactionList.js) in which state.messages[props.navigation.state.params.messageId] is undefined, resulting in a crash. Concretely, this means that the messageId param (passed either from a long-press on an existing reaction in the MessageList, or from tapping "See who reacted" from the message action sheet) refers to a message that is not in state.messages at that time.

This can be reproduced by going to that screen, putting the app in the background for 10 minutes or so -- as long as it takes for the BAD_EVENT_QUEUE_ID error to appear, indicating that the app has stale data that needs to be discarded -- and returning to the app. The DEAD_QUEUE action discards all messages from state, so no message is found for the messageId param and the TypeError: undefined is not an object (evaluating 'n.reactions') error occurs.

This will likely be mostly fixed with a fix for #3802. It's hoped that we can avoid a period when state.messages is empty following the BAD_EVENT_QUEUE_ID error; perhaps this can be done by discarding and repopulating the messages in the same action, REALM_INIT, as mentioned in that issue, instead of discarding in DEAD_QUEUE. A fix for the MessageReactionList problem (the present issue) would depend on the selected message still being present after the repopulation. There may be a case or two where this would be a concern.

We've identified this as the cause of 9 bug reports in Sentry affecting 7 users at https://sentry.io/organizations/zulip/issues/1390866965/?project=191284 and https://sentry.io/organizations/zulip/issues/1445808226/?project=191284 (the latter is affected by the "Source code was not found" error; not sure why that's been happening).

[rk-for-zulip]

(the latter is affected by the "Source code was not found" error; not sure why that's been happening).

(This is most likely because something went wrong when uploading the source map for the iOS build to Sentry, at build time.)

[gnprice]

(the latter is affected by the "Source code was not found" error; not sure why that's been happening).

(This is most likely because something went wrong when uploading the source map for the iOS build to Sentry, at build time.)

Hmm, and that was on v26.20.143 -- the latest version. So that seems like something we should debug.

I went and filed #3809 for it.

[chrisbobbe]

Ah, I incorrectly assumed that the proposed fix for #3802 would also fix this issue. From the chat:

It does not fix #M3803. MessageReactionList for a messageId still assumes the message exists in state.messages, but there's still that period between REALM_INIT and when -- if, actually! -- the message reappears in MESSAGE_FETCH_COMPLETE, which causes the same error as before. A few ways I think we could go with this --

1. If the message is not in state.messages (whether because the fetch is in progress or it wasn't found in the fetch), navigate back, and forget about displaying the MessageReactionList.
2. Show a loading indicator when state.session.loading is true, i.e., when the initial fetch is in progress. The initial fetch is always triggered by a DEAD_QUEUE, which is what we're addressing now.
3. Show a loading indicator when state.fetching[SOME_NARROW].older or .newer is true. These change on MESSAGE_FETCH_START and MESSAGE_FETCH_COMPLETE, which are always triggered by a DEAD_QUEUE. They are also triggered elsewhere, but "elsewhere" does not currently include a purge of messages from state, as far as I'm aware.

If #3, we need SOME_NARROW to be correct every time.

[chrisbobbe]

Never mind #2; if we're showing a loading indicator, it needs to correspond to state.fetching[SOME_NARROW]. I think #3 is best, to reduce unprompted navigations. If the message doesn't exist after the message fetch, we can still navigate back.

[chrisbobbe]

Actually doing #1, which is technically easier and doesn't make a future change to #3 more difficult (#3 would already require some more plumbing). It's also unlikely for someone to leave the app for 10 minutes and come back expecting to check on the reactions to a particular message.