RCORE-1982 Opening realm with cached user while offline results in fa…

…tal error and session does not retry connection (#7469)

* Moved to using a pre-initialized sync-route instead of leaving empty and querying on first connect attempt

* Added base url update logic when session connection fails - added verified flag to prevent updating location too much

* Updated changelog; added default_base_url for C_API; added restart_sessions param to set_sync_route()

* Fixed c_api compile error

* Silly uninitialized variable

* Updates from review; fixed deadlock in test

* Updated test

* Changed default base url to function in CAPI

* Updates from review

* Removed old test and updated translationg comments in create_ws_host_url

* Updates from review

* Updated changelog after release buidl

* Updates from review

* Moved HookedSocketProvider and HookedTransport to sync_test_utils for common use

* Added test for updating the an invalid sync route using local server

* Expanded test to use multiple realms with and without multiplexing

* Delete directory _after_ stopping app...

* Updates from review

* Updated comment and changelog from review

CHANGELOG.md

@@ -9,6 +9,7 @@
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Modifying nested collections left the accessor used to make the modification in a stale state, resulting in some unneccesary work being done when making multiple modifications via one accessor ([PR #7470](https://github.com/realm/realm-core/pull/7470), since v14.0.0).
 * Fix depth level for nested collection in debug mode, set it to the same level as release ([#7484](https://github.com/realm/realm-core/issues/7484), since v14.0.0).
+* Fix opening realm with cached user while offline results in fatal error and session does not retry connection. ([#7349](https://github.com/realm/realm-core/issues/7349), since v13.26.0)
 
 ### Breaking changes
 * Update C-API log callback signature to include the log category, and `realm_set_log_callback` to not take a `realm_log_level_e`. ([PR #7494](https://github.com/realm/realm-core/pull/7494)

src/realm.h

@@ -88,6 +88,7 @@ typedef bool (*realm_migration_func_t)(realm_userdata_t userdata, realm_t* old_r
 typedef bool (*realm_data_initialization_func_t)(realm_userdata_t userdata, realm_t* realm);
 typedef bool (*realm_should_compact_on_launch_func_t)(realm_userdata_t userdata, uint64_t total_bytes,
                                                       uint64_t used_bytes);
+
 typedef enum realm_schema_mode {
     RLM_SCHEMA_MODE_AUTOMATIC,
     RLM_SCHEMA_MODE_IMMUTABLE,
@@ -2977,6 +2978,7 @@ RLM_API realm_auth_provider_e realm_auth_credentials_get_provider(realm_app_cred
 RLM_API realm_app_config_t* realm_app_config_new(const char* app_id,
                                                  const realm_http_transport_t* http_transport) RLM_API_NOEXCEPT;
 
+RLM_API const char* realm_app_get_default_base_url(void) RLM_API_NOEXCEPT;
 RLM_API void realm_app_config_set_base_url(realm_app_config_t*, const char*) RLM_API_NOEXCEPT;
 RLM_API void realm_app_config_set_default_request_timeout(realm_app_config_t*, uint64_t ms) RLM_API_NOEXCEPT;
 RLM_API void realm_app_config_set_platform_version(realm_app_config_t*, const char*) RLM_API_NOEXCEPT;

src/realm/object-store/c_api/app.cpp

@@ -125,6 +125,11 @@ static inline bson::BsonArray parse_ejson_array(const char* serialized)
     }
 }
 
+RLM_API const char* realm_app_get_default_base_url(void) noexcept
+{
+    return app::App::default_base_url.data();
+}
+
 RLM_API realm_app_credentials_t* realm_app_credentials_new_anonymous(bool reuse_credentials) noexcept
 {
     return new realm_app_credentials_t(AppCredentials::anonymous(reuse_credentials));

src/realm/object-store/sync/app.cpp

@@ -167,7 +167,6 @@ UniqueFunction<void(const Response&)> handle_default_response(UniqueFunction<voi
     };
 }
 
-constexpr static std::string_view s_default_base_url = "https://realm.mongodb.com";
 constexpr static std::string_view s_base_path = "/api/client/v2.0";
 constexpr static std::string_view s_app_path = "/app";
 constexpr static std::string_view s_auth_path = "/auth";
@@ -184,6 +183,8 @@ std::mutex s_apps_mutex;
 namespace realm {
 namespace app {
 
+std::string_view App::default_base_url = "https://realm.mongodb.com";
+
 App::Config::DeviceInfo::DeviceInfo()
     : platform(util::get_library_platform())
     , cpu_arch(util::get_library_cpu_arch())
@@ -215,7 +216,7 @@ SharedApp App::get_app(CacheMode mode, const Config& config,
 {
     if (mode == CacheMode::Enabled) {
         std::lock_guard<std::mutex> lock(s_apps_mutex);
-        auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(s_default_base_url))];
+        auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(default_base_url))];
         if (!app) {
             app = std::make_shared<App>(Private(), config);
             app->configure(sync_client_config);
@@ -261,7 +262,7 @@ void App::close_all_sync_sessions()
 
 App::App(Private, const Config& config)
     : m_config(config)
-    , m_base_url(m_config.base_url.value_or(std::string(s_default_base_url)))
+    , m_base_url(m_config.base_url.value_or(std::string(default_base_url)))
     , m_location_updated(false)
     , m_request_timeout_ms(m_config.default_request_timeout_ms.value_or(s_default_timeout_ms))
 {
@@ -301,16 +302,21 @@ App::~App() {}
 
 void App::configure(const SyncClientConfig& sync_client_config)
 {
+    std::string ws_route;
     {
         util::CheckedLockGuard guard(m_route_mutex);
         // Make sure to request the location when the app is configured
         m_location_updated = false;
+        // Create a tentative sync route using the generated ws_host_url
+        REALM_ASSERT(!m_ws_host_url.empty());
+        ws_route = make_sync_route();
     }
 
-    // Start with an empty sync route in the sync manager. It will ensure the
-    // location has been updated at least once when the first sync session is
-    // started by requesting a new access token.
-    m_sync_manager = SyncManager::create(shared_from_this(), {}, sync_client_config, config().app_id);
+    // When App starts, the ws_host_url will be populated with the generated value based on
+    // the provided host_url value and the sync route will be created using this. If this is
+    // is incorrect, the websocket connection will fail and the SyncSession will request a
+    // new access token, which will update the location if it has not already.
+    m_sync_manager = SyncManager::create(shared_from_this(), ws_route, sync_client_config, config().app_id);
 }
 
 bool App::init_logger()
@@ -384,23 +390,58 @@ void App::configure_route(const std::string& host_url, const std::optional<std::
     if (ws_host_url && ws_host_url->length() > 0) {
         m_ws_host_url = *ws_host_url;
     }
-    // Otherwise, convert the host url to a websocket host url (http[s]:// -> ws[s]://)
+    // Otherwise, convert the host url to a websocket host url
     else {
-        m_ws_host_url = m_host_url;
-        if (m_ws_host_url.find("http") == 0) {
-            m_ws_host_url.replace(0, 4, "ws");
-        }
+        m_ws_host_url = App::create_ws_host_url(m_host_url);
     }
 
     // host_url is the url to the server: e.g., https://realm.mongodb.com or https://localhost:9090
-    // base_route is the baseline client api path: e.g. https://realm.mongodb.com/api/client/v2.0
+    // base_route is the baseline client api path: e.g. <host_url>/api/client/v2.0
     m_base_route = util::format("%1%2", m_host_url, s_base_path);
-    // app_route is the cloud app URL: https://realm.mongodb.com/api/client/v2.0/app/<app_id>
+    // app_route is the cloud app URL: <host_url>/api/client/v2.0/app/<app_id>
     m_app_route = util::format("%1%2/%3", m_base_route, s_app_path, m_config.app_id);
-    // auth_route is cloud app auth URL: https://realm.mongodb.com/api/client/v2.0/app/<app_id>/auth
+    // auth_route is cloud app auth URL: <host_url>/api/client/v2.0/app/<app_id>/auth
     m_auth_route = util::format("%1%2", m_app_route, s_auth_path);
 }
 
+// Create a temporary websocket URL domain using the given host URL
+// This updates the URL based on the following assumptions:
+// If the URL doesn't start with 'http' => <host-url>
+// http[s]://[region-prefix]realm.mongodb.com => ws[s]://ws.[region-prefix]realm.mongodb.com
+// http[s]://[region-prefix]services.cloud.mongodb.com => ws[s]://[region-prefix].ws.services.cloud.mongodb.com
+// All others => http[s]://<host-url> => ws[s]://<host-url>
+std::string App::create_ws_host_url(const std::string_view host_url)
+{
+    constexpr static std::string_view orig_base_domain = "realm.mongodb.com";
+    constexpr static std::string_view new_base_domain = "services.cloud.mongodb.com";
+    const size_t base_len = std::char_traits<char>::length("http://");
+
+    // Doesn't contain 7 or more characters (length of 'http://') or start with http,
+    // just return provided string
+    if (host_url.length() < base_len || host_url.substr(0, 4) != "http") {
+        return std::string(host_url);
+    }
+    // If it starts with 'https' then ws url will start with 'wss'
+    bool https = host_url[4] == 's';
+    size_t prefix_len = base_len + (https ? 1 : 0);
+    std::string_view prefix = https ? "wss://" : "ws://";
+
+    // http[s]://[<region-prefix>]realm.mongodb.com[/<path>] =>
+    //     ws[s]://ws.[<region-prefix>]realm.mongodb.com[/<path>]
+    if (host_url.find(orig_base_domain) != std::string_view::npos) {
+        return util::format("%1ws.%2", prefix, host_url.substr(prefix_len));
+    }
+    // http[s]://[<region-prefix>]services.cloud.mongodb.com[/<path>] =>
+    //     ws[s]://[<region-prefix>].ws.services.cloud.mongodb.com[/<path>]
+    if (auto start = host_url.find(new_base_domain); start != std::string_view::npos) {
+        return util::format("%1%2ws.%3", prefix, host_url.substr(prefix_len, start - prefix_len),
+                            host_url.substr(start));
+    }
+
+    // All others => http[s]://<host-url>[/<path>] => ws[s]://<host-url>[/<path>]
+    return util::format("ws%1", host_url.substr(4));
+}
+
 void App::update_hostname(const std::string& host_url, const std::optional<std::string>& ws_host_url,
                           const std::optional<std::string>& new_base_url)
 {
@@ -612,11 +653,11 @@ std::string App::get_base_url() const
 
 void App::update_base_url(std::optional<std::string> base_url, UniqueFunction<void(Optional<AppError>)>&& completion)
 {
-    std::string new_base_url = base_url.value_or(std::string(s_default_base_url));
+    std::string new_base_url = base_url.value_or(std::string(default_base_url));
 
     if (new_base_url.empty()) {
         // Treat an empty string the same as requesting the default base url
-        new_base_url = s_default_base_url;
+        new_base_url = default_base_url;
         log_debug("App::update_base_url: empty => %1", new_base_url);
     }
     else {
@@ -1023,8 +1064,6 @@ std::optional<AppError> App::update_location(const Response& response, const std
         return error;
     }
 
-    REALM_ASSERT(m_sync_manager); // Need a valid sync manager
-
     // Update the location info with the data from the response
     try {
         auto json = parse<BsonDocument>(response.body);
@@ -1038,8 +1077,10 @@ std::optional<AppError> App::update_location(const Response& response, const std
             // Update the local hostname and path information
             update_hostname(hostname, ws_hostname, base_url);
             m_location_updated = true;
-            // Provide the Device Sync websocket route to the SyncManager
-            m_sync_manager->set_sync_route(make_sync_route());
+            if (m_sync_manager) {
+                // Provide the Device Sync websocket route to the SyncManager
+                m_sync_manager->set_sync_route(make_sync_route());
+            }
         }
     }
     catch (const AppError& ex) {

src/realm/object-store/sync/app.hpp

@@ -90,6 +90,8 @@ class App : public std::enable_shared_from_this<App>,
         DeviceInfo device_info;
     };
 
+    static std::string_view default_base_url;
+
     // `enable_shared_from_this` is unsafe with public constructors;
     // use `App::get_app()` instead
     explicit App(Private, const Config& config);
@@ -432,6 +434,8 @@ class App : public std::enable_shared_from_this<App>,
     // Return the base url path used for Sync Session Websocket requests
     std::string get_ws_host_url() REQUIRES(!m_route_mutex);
 
+    static std::string create_ws_host_url(const std::string_view host_url);
+
 private:
     const Config m_config;
 

src/realm/object-store/sync/sync_manager.cpp

@@ -43,13 +43,13 @@ SyncClientTimeouts::SyncClientTimeouts()
 {
 }
 
-std::shared_ptr<SyncManager> SyncManager::create(std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+std::shared_ptr<SyncManager> SyncManager::create(std::shared_ptr<app::App> app, std::string sync_route,
                                                  const SyncClientConfig& config, const std::string& app_id)
 {
-    return std::make_shared<SyncManager>(Private(), std::move(app), std::move(sync_route), config, app_id);
+    return std::make_shared<SyncManager>(Private(), std::move(app), sync_route, config, app_id);
 }
 
-SyncManager::SyncManager(Private, std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+SyncManager::SyncManager(Private, std::shared_ptr<app::App> app, std::string sync_route,
                          const SyncClientConfig& config, const std::string& app_id)
     : m_config(config)
     , m_file_manager(std::make_unique<SyncFileManager>(m_config.base_file_path, app_id))
@@ -150,7 +150,6 @@ void SyncManager::tear_down_for_testing()
         // Destroy the client now that we have no remaining sessions.
         m_sync_client = nullptr;
         m_logger_ptr.reset();
-        m_sync_route.reset();
     }
 
     {
@@ -666,6 +665,25 @@ void SyncManager::close_all_sessions()
     get_sync_client().wait_for_session_terminations();
 }
 
+void SyncManager::set_sync_route(std::string sync_route, bool verified)
+{
+    REALM_ASSERT(!sync_route.empty()); // Cannot be set to empty string
+    {
+        util::CheckedLockGuard lock(m_mutex);
+        m_sync_route = sync_route;
+        m_sync_route_verified = verified;
+    }
+}
+
+void SyncManager::restart_all_sessions()
+{
+    // Restart the sessions that are currently active
+    auto sessions = get_all_sessions();
+    for (auto& session : sessions) {
+        session->restart_session();
+    }
+}
+
 void SyncManager::OnlyForTesting::voluntary_disconnect_all_connections(SyncManager& mgr)
 {
     mgr.get_sync_client().voluntary_disconnect_all_connections();

src/realm/object-store/sync/sync_manager.hpp

@@ -225,23 +225,20 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
     // Immediately closes any open sync sessions for this sync manager
     void close_all_sessions() REQUIRES(!m_mutex, !m_session_mutex);
 
+    // Force all the active sessions to restart
+    void restart_all_sessions() REQUIRES(!m_mutex, !m_session_mutex);
+
     // Used by App to update the sync route any time the location info has been refreshed.
-    // m_sync_route starts out as unset when the SyncManager is created or configured.
-    // It will be updated to a valid value upon the first App AppServices HTTP request or
-    // the access token will be refreshed (forcing a location update) when a SyncSession
-    // is activated and it is still unset. This value is not allowed to be reset to
-    // nullopt once it has a valid value.
-    void set_sync_route(std::string sync_route) REQUIRES(!m_mutex)
-    {
-        REALM_ASSERT(!sync_route.empty());
-        util::CheckedLockGuard lock(m_mutex);
-        m_sync_route = std::move(sync_route);
-    }
+    // m_sync_route starts out as a generated value based on the configured base_url when
+    // the SyncManager is created by App. If this is incorrect, the websocket connection
+    // will fail, resulting in an update to the access token (and the location, if it hasn't
+    // been updated yet).
+    void set_sync_route(std::string sync_route, bool verified = true) REQUIRES(!m_mutex);
 
-    const std::optional<std::string> sync_route() const REQUIRES(!m_mutex)
+    std::pair<const std::string, bool> sync_route() REQUIRES(!m_mutex)
     {
         util::CheckedLockGuard lock(m_mutex);
-        return m_sync_route;
+        return std::make_pair(m_sync_route, m_sync_route_verified);
     }
 
     std::weak_ptr<app::App> app() const REQUIRES(!m_mutex)
@@ -270,10 +267,10 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
         static void voluntary_disconnect_all_connections(SyncManager&);
     };
 
-    static std::shared_ptr<SyncManager> create(std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
+    static std::shared_ptr<SyncManager> create(std::shared_ptr<app::App> app, std::string sync_route,
                                                const SyncClientConfig& config, const std::string& app_id);
-    SyncManager(Private, std::shared_ptr<app::App> app, std::optional<std::string> sync_route,
-                const SyncClientConfig& config, const std::string& app_id);
+    SyncManager(Private, std::shared_ptr<app::App> app, std::string sync_route, const SyncClientConfig& config,
+                const std::string& app_id);
 
 private:
     friend class app::App;
@@ -331,9 +328,11 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
     // Callers of this method should hold the `m_session_mutex` themselves.
     bool do_has_existing_sessions() REQUIRES(m_session_mutex);
 
-    // The sync route URL to connect to the server. This can be initially empty, but should not
-    // be cleared once it has been set to a value, except by `tear_down_for_testing()`.
-    std::optional<std::string> m_sync_route GUARDED_BY(m_mutex);
+    // The sync route URL for the sync connection to the server.
+    std::string m_sync_route GUARDED_BY(m_mutex);
+    // If true, then the sync route has been verified by querying the location info or successfully
+    // connecting to the server.
+    bool m_sync_route_verified GUARDED_BY(m_mutex) = false;
 
     std::weak_ptr<app::App> m_app GUARDED_BY(m_mutex);
     const std::string m_app_id;