Opening a notification for nonexistent user/stream causes crash

[gnprice]

This is a situation that's only possible in the first place when caused by some bug in either the app or the server. But such bugs happen, and particularly because it involves the server, we should be robust enough not to crash on it.

I don't think there's a known way to trigger this with a current version from master (or with today's release v27.157, now in alpha) and a stock server. But one way to trigger it is by reproducing #4290 on the current production release, v27.156. As described at #4290 (comment) , the app crashes, and (on Android) adb logcat shows output like this:

11-12 14:38:49.964 27558 27719 W ReactNativeJS: notification realm_uri not found in accounts
11-12 14:38:49.965 27558 27719 W ReactNativeJS:     realm_uri: "https://chat.zulip.org"
11-12 14:38:49.965 27558 27719 W ReactNativeJS:     known_urls: …
11-12 14:38:50.002 27558 27719 E ReactNativeJS: Error: getUserForEmail: missing user: email …

That is, we try to navigate to the right conversation as if it were on the active account... and when there isn't a user by that email, we crash.

Instead, we should handle this in getNarrowFromNotificationData -- that's the place where we take the raw names/IDs the server put in the notification and turn them into values we'll pass around to the "soft center" of our normal app code which assumes the data it gets is well-formed.

In getNarrowFromNotificationData, we should confirm that the specified user(s) or stream exist in our data. If they don't, then treat it much like we do in getAccountFromNotificationData:

• Log a warning (to Sentry.)
• Return null, to skip trying to do anything with the data.

That way the app will be open and running, and not crashed.

[gnprice]

I don't think there's a known way to trigger this with a current version from master (or with today's release v27.157, now in alpha) and a stock server.

A correction: this bug can be triggered on v27.157 by interacting with this remaining piece of #2295 (quoting my description at #2295 (comment) ):

• If the user has several known accounts in the same realm, we
  don't switch to either of them, because we can't tell which.

So if you've been using one realm/org; and you get a notification for a different one; and you have multiple known accounts in that other realm; then we don't switch to it, we stay in the wrong realm/org, and so we end up exercising this bug and crashing.

[gnprice]

I believe this is now fixed for PMs, by commits bed2f2a and e9941c0 (PRs #4335 and #4346). If we don't have data on one of the participants in the PM conversation, getNarrowFromNotificationData will return null and we simply won't attempt the narrow.

I haven't tested that empirically, though.

Still open for notifications for stream messages -- we'll attempt to navigate to the given conversation, and if we can't find the stream (by name), then I expect that will crash.

Before we close this issue, we should also have a comment in the code saying it's important that it checks for the users existing. As it stands, that check for PMs is basically a side effect of the fact that our Narrow constructors for PM conversations demand both emails and user IDs.

[chrisbobbe]

I've just posted a few questions on CZO.

[gnprice]

Looking in Sentry, this isn't something that's happening to lots of users.

But it is pretty bad when it happens, so would still be good to fix. And I think with the plan at the end of the issue description above, it won't be very complicated to fix.

[chrisbobbe]

I've just posted a few questions on CZO.

I think progress doesn't have to block on reading and understanding this thread.

[WesleyAC]

Is this this issue in Sentry? If so, it's happening pretty often.

I did find a interesting example — a selfhosted realm, where the URL known_urls gives a 301 redirect to the realm_uri/parsed_url. (link, and another).

So, that sort of makes sense as an explanation for this. The server got moved, but we didn't update known_urls when that happened.

Another thing I see is where the URL in known_urls is https://example.com/, but the realm_uri is https://example.com (with no slash). That seems like something we should detect as well. (link)

Here's an odd case where known_urls had https://example.com/join/<invite link hash stuff>, the passed_url is https://example.com/, and the realm_uri is https://example.com.