Crash while logging into second account

[gnprice]

This is the same issue as #4092, but I want to restart that thread now that we understand the cause better and also have sorted out some issues with our debugging data. (Some material copied from comments there.)

Here's a repro recipe:

• Open the app; log into some account, if not already logged in.
• Visit the streams tab.
• Now go to the profile tab, and hit "Switch account".
• Pick some other account, where not logged in.
• Rotate the device, so it goes from portrait to landscape. (Or vice versa is probably just as effective.)

This recipe has a 100% crash rate in my testing so far.

On iOS, the whole app crashes. On Android, the symptom is a bit different: the app becomes a white screen.

I've reproduced this a number of times on iOS at 26.28.151, the current beta. Also reproduced on Android at that version, and on iOS at 26.27.150 and at 26.23.146, which is a release a few months old and the earliest release still conveniently available through TestFlight.

For the crash report as seen in Sentry, see this event. The error message is "Error: Error: Error: Error: Active account not logged in".

The stack trace is in getAuth (as expected from the error message)... and outside that, it's in the mapStateToProps callback in the connect call around StreamListCard.

export default connect(state => ({
  auth: getAuth(state),
  canCreateStreams: getCanCreateStreams(state),
  streams: getStreams(state),
  subscriptions: getSubscriptions(state),
}))(StreamListCard);

So. It seems like:

• we still have a StreamListCard component mounted somewhere;
• and we've switched the active account to one that's not logged in -- so that StreamListCard is doomed not to work.

I think one thing that's going on is that when you hit "Switch account", and also when you then pick an account to switch to, we just push a new screen onto the navigation stack -- we don't replace the existing main route, which is the screen that has the StreamListCard component underneath it.

The solution may just be to stop doing that, and instead to replace the existing main route.

[chrisbobbe]

I wonder if we were missing something in 699f65f (awesome to see this in its own commit! I'll take note for my own commit discipline 🙂)? If so, it would make sense why we started seeing this fairly recently. And it makes sense that we might have done the testing described there without doing seemingly-unrelated things like rotating the screen orientation.

I'm puzzled why the error even waits for something like rotating the screen — you don't get the error immediately on the ACCOUNT_SWITCH action, as I'd expect.

[chrisbobbe]

Ah, no! You confirmed here that this wasn't a recent regression:

Now going back to see if this is a regression. The testing below is on my iPad mini 4.

• The recipe crashes on 26.23.146. That's the oldest still available through TestFlight.
• It also crashes on 26.27.150, which is the release currently in prod.

OK, so this appears not to be a regression in 26.28.151. Based on that, I think we are clear to send 26.28.151 to prod (though I'll do a quick check first that there isn't something else we have reason to believe is wrong with it.) Should still fix this bug, though.

I guess that disproves my above hypothesis (at #4135 (comment))! 699f65f and the rest of PR #4078 was released in 26.28.151. Hmm.

[gnprice]

Ah, no! You confirmed here that this wasn't a recent regression:

Yeah. This is certainly in the spirit of the kind of thing that the blank-state hack eliminated in that series is meant to paper over! But that hack affected a couple of properties of state.realm, whereas this crash is from looking at properties of state.accounts[0], so it was never able to paper over this particular crash.

Certainly it'd be possible to have more and more blank-state hacks, supplying fake blank values for more and more of our state when we don't have real data, to paper over more and more issues like this one to keep them from causing crashes. And if we'd had a hack like this and removed it and that started suddenly causing crashes, I'd revert to put the hack back until we fixed the underlying bug. But given that that turns out not to be the situation here, I'd rather simply fix the bug 🙂

[agrawal-d]

I tried to fix this, with the suggested change replace, as well as reset:

export const navigateToAccountPicker = (): NavigationAction =>
  StackActions.reset({
    index: 0,
    actions: [{ routeName: 'account', type: 'Navigation/NAVIGATE' }],
  });

but I'm still getting the error:

Possible Unhandled Promise Rejection (id: 0):
Error: Active account not logged in
Error: Active account not logged in
    at getAuth (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:128332:13)
    at _callee3$ (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:292694:45)
    at tryCatch (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23470:19)
    at Generator.invoke [as _invoke] (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23646:24)
    at Generator.prototype.<computed> [as next] (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23513:23)
    at tryCatch (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23470:19)
    at invoke (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23546:22)
    at blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:23576:13
    at tryCallTwo (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:24782:7)
    at doResolve (blob:http://localhost:8081/c483aecd-c41f-40e8-b797-5274c9baf550:24946:15)

On current master, with Remote JS debugging turned on, Chrome 83

[chrisbobbe]

We think that #4603 will fix this if it hasn't been fixed yet (e.g., by #4491); in any case, I should investigate and confirm.

[gnprice]

We think that #4603 will fix this if it hasn't been fixed yet (e.g., by #4491)

I've just attempted the repro (from the issue description) on v26.153, which has both those PRs. Indeed it no longer reproduces. So this seems to have been successfully resolved!