Page MenuHomePhabricator
Differential D12369

[lib][native][web] Recover from socket crash loop using fetchPendingUpdates
ClosedPublic
Actions

Authored by ashoat on Sat, Jun 8, 3:46 PM.
Tags
None
Referenced Files
F2177307: D12369.id41149.diff
Wed, Jul 3, 7:03 AM
Unknown Object (File)
Tue, Jul 2, 5:33 AM
Unknown Object (File)
Sat, Jun 29, 8:13 PM
Unknown Object (File)
Fri, Jun 28, 2:43 AM
Unknown Object (File)
Thu, Jun 27, 9:56 PM
Unknown Object (File)
Tue, Jun 25, 9:22 AM
Unknown Object (File)
Sat, Jun 22, 5:24 AM
Unknown Object (File)
Fri, Jun 21, 6:43 PM
View All 30 Files
Subscribers
inka
kamil

Details

Reviewers
tomek
kamil
Commits
rCOMMfaedcb586410: [lib][native][web] Recover from socket crash loop using fetchPendingUpdates
Summary

This addresses ENG-8089.

Depends on D12368

Test Plan

In combination with later diffs, I tested as follows:

  1. I created a socket crash loop on a physical iOS device using @inka's create-many-threads-to-trigger-crash-loop.js script (see ENG-8090
  2. I confirmed that the socket was unable to connect prior to my diff stack
  3. I confirmed that after applying my diff stack, the SUCCESS action was dispatched, and the socket was able to connect afterwards

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat created this revision.Sat, Jun 8, 3:46 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Sat, Jun 8, 4:05 PM
Harbormaster failed remote builds in B29559: Diff 41146!
ashoat updated this revision to Diff 41149.Sat, Jun 8, 6:32 PM

Rebase

Harbormaster completed remote builds in B29562: Diff 41149.Sat, Jun 8, 6:50 PM
ashoat requested review of this revision.Sat, Jun 8, 6:50 PM
ashoat added a subscriber: kamil.Sun, Jun 9, 6:10 AM
ashoat added inline comments.
lib/socket/socket.react.js
700–708 ↗(On Diff #41149)

Thinking about this more, I wonder if we should just get rid of this older mechanism. It has the potential of logging the user out if recovery is not possible (web, or ETH user on native). Removing it would also let us deprecate showSocketCrashLoopAlert, which only exists to warn the user about the potential logout.

I think the new mechanism should allow us to heal from any socket crash loop, including one that is the result of a policy acknowledgment after login. cc @kamil

ashoat added a reviewer: kamil.Sun, Jun 9, 6:13 AM
tomek accepted this revision.Mon, Jun 10, 7:02 AM
tomek added inline comments.
lib/socket/socket.react.js
714 ↗(On Diff #41149)

Should we somehow remember that this operation is in progress? I worry that it might be possible to start this action multiple times.

This revision is now accepted and ready to land.Mon, Jun 10, 7:02 AM
kamil added inline comments.Mon, Jun 10, 10:04 AM
lib/socket/socket.react.js
700–708 ↗(On Diff #41149)

Agree with that - I think we can remove it

ashoat added inline comments.Mon, Jun 10, 1:48 PM
lib/socket/socket.react.js
700–708 ↗(On Diff #41149)

Great – I'll remove it in a follow-up

714 ↗(On Diff #41149)

Good call. Will add something for this

ashoat updated this revision to Diff 41177.Mon, Jun 10, 1:51 PM

Make sure we only ever call fetchPendingUpdates once at a time

This revision was landed with ongoing or failed builds.Mon, Jun 10, 2:03 PM
Closed by commit rCOMMfaedcb586410: [lib][native][web] Recover from socket crash loop using fetchPendingUpdates (authored by ashoat). · Explain Why
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMMfaedcb586410: [lib][native][web] Recover from socket crash loop using fetchPendingUpdates.
Harbormaster completed remote builds in B29586: Diff 41177.Mon, Jun 10, 2:08 PM

Revision Contents
Changeset List

PathSize
lib/
socket/
31 lines
native/
9 lines
web/
8 lines

Diff 41192

View Options

lib/socket/socket.react.js

Loading...
View Options

native/socket.react.js

Loading...
View Options

web/socket.react.js

Loading...