Skip to content

user: Expand checks in getHaveServerData.#4588

Merged
chrisbobbe merged 1 commit intozulip:masterfrom
gnprice:pr-data-desync
Apr 2, 2021
Merged

user: Expand checks in getHaveServerData.#4588
chrisbobbe merged 1 commit intozulip:masterfrom
gnprice:pr-data-desync

Conversation

@gnprice
Copy link
Copy Markdown
Member

@gnprice gnprice commented Apr 2, 2021

The one check we had would be perfectly sufficient in a world where
we used a reliable form of storage for keeping data from the server
on the device between runs of the app. It does not suffice given
our model with redux-persist.

This doesn't fix the problem -- that will take something more
fundamental -- but it hopefully mitigates the worst consequences.

In particular, empirically we already reasonably gracefully handle
in the unreads view and the PM-conversations view a situation where
either state.unreads or state.pmConversations refers to streams,
subscriptions, or users that we don't have data about:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/Android.3A.20White.20screen/near/1151982

Fixes: #4587

The one check we had would be perfectly sufficient in a world where
we used a reliable form of storage for keeping data from the server
on the device between runs of the app.  It does not suffice given
our model with redux-persist.

This doesn't fix the problem -- that will take something more
fundamental -- but it hopefully mitigates the worst consequences.

In particular, empirically we already reasonably gracefully handle
in the unreads view and the PM-conversations view a situation where
either `state.unreads` or `state.pmConversations` refers to streams,
subscriptions, or users that we don't have data about:
  https://chat.zulip.org/#narrow/stream/48-mobile/topic/Android.3A.20White.20screen/near/1151982

Fixes: zulip#4587
@gnprice gnprice added a-multi-org a-redux severe: crash The app quits, or stops responding. P0 critical Highest priority labels Apr 2, 2021
@gnprice gnprice requested a review from chrisbobbe April 2, 2021 22:35
@chrisbobbe chrisbobbe merged commit 916dc4b into zulip:master Apr 2, 2021
@chrisbobbe
Copy link
Copy Markdown
Contributor

Thanks, LGTM! Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a-multi-org a-redux P0 critical Highest priority severe: crash The app quits, or stops responding.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App perma-crashes on partial update to persisted state

2 participants