RCORE-1982: Opening realm with cached user while offline results in fatal error and session does not retry connection

[michael-wb]

What, How & Why?

Moved the location update when a realm is opened at client App start with a cached user to be performed by the sync manager. Until the location has been updated, the opened sync sessions will be in the WaitingForLocation state. Once the location is updated, the active sessions in this state will be revived and either go to the Active or WaitingForAccessToken state as normal.

Also needed to fix the operation of the timers in the network::Service, since the event loop wait time was not updated when a new timer was created if there was no other post() or network activity in progress. Instead of resuming the event loop when the timer should have expired, the event loop continues to wait for the original duration. This was not normally seen during normal sync client operation since there is usually post() and/or network activity taking place regularly, which would trigger the event loop and update the wait time based on the current set of pending timers.

Fixes #7349, #7364

☑️ ToDos

• [ ] 📝 Changelog update
• [X] 🚦 Tests (or not relevant)
• ~~[ ] C-API, if public C++ API changed~~
• ~~[ ] bindgen/spec.yml, if public C++ API changed~~

[coveralls-official[bot]]

Pull Request Test Coverage Report for Build michael.wilkersonbarker_971

Details

• 209 of 226 (92.48%) changed or added relevant lines in 9 files are covered.
• 126 unchanged lines in 18 files lost coverage.
• Overall coverage decreased (-0.03%) to 91.835%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
test/test_util_network.cpp	37	39	94.87%
src/realm/object-store/sync/sync_session.cpp	25	31	80.65%
src/realm/object-store/sync/sync_manager.cpp	85	94	90.43%
<!--	Total:	209	226

Files with Coverage Reduction	New Missed Lines	%
src/realm/index_string.cpp	1	87.85%
src/realm/index_string.hpp	1	82.86%
test/object-store/sync/app.cpp	1	97.92%
src/realm/array_blobs_big.cpp	2	98.72%
src/realm/sync/noinst/server/server_history.cpp	2	67.94%
src/realm/util/serializer.cpp	2	90.03%
src/realm/uuid.cpp	2	97.01%
test/test_sync.cpp	2	94.14%
src/realm/sync/transform.cpp	3	63.07%
src/realm/util/file.cpp	3	81.47%
<!--	Total:	126

Totals
Change from base Build 2051:	-0.03%
Covered Lines:	235370
Relevant Lines:	256297

---

💛 - Coveralls

[jbreams]

@tgoyne , AutoOpen vs AsyncOpen vs whatever other kinds of open we have are getting a bit muddled for me. Can you point to where in the swift SDK (I assume that's where AutoOpen) is implemented so we can write a test that verifies this functionality? My read is that updating your location via other App calls (like trying to log in a user) will still fail semi-synchronously, and this change just makes actually starting a sync::Session asynchronously retry getting a location if it doesn't have one. Maybe I've missed something though.

[tgoyne]

The AutoOpen implementation is quite complicated and may not be particularly information. There are no other App calls involved; in the relevant use-case we already have a cached logged-in user from a previous run of the application. We call Realm::get_synchronized_realm() with cancel_waits_on_nonfatal_error=true to make the async open fail on any error rather than just non-transient errors, and fall back to a synchronous open if any errors occur. For this to work all transient errors when opening a session have to be reported to the error handler.

[jbreams]

So to adapt these changes to fit this case we'd need to add some handling here https://github.com/realm/realm-core/pull/7365/files#diff-8a4439bf8b1d6f5ce56b98f9d0409beb874ce6de9aee5ed795688f5d4378787eR750 and maybe have SyncSession::handle_error() drive the location update backoff?

[tgoyne]

A higher level thought on design: I think that SyncSession shouldn't really be aware of location fetching at all. It could have a single WaitingForDependencies state that it enters when it tries to activate and one of the things it needs is missing and a way to ask its SyncUser to try again to do whatever things it needs to do to supply the dependencies (and then pass the resulting error back to any waiters if applicable).

I think we had a pre-existing problem where an expired cached access token would result in AutoOpen failing to fall back to the local realm, and it seems like we have to solve all of the exact same problems for the waiting for access token state and waiting for location state.

[michael-wb]

This PR is superceded by the fix in #7469.