RCORE-2060 Enabling 'cancel_waits_on_nonfatal_error' does not cancel waits during location update while offline (#7528)

* added tests using cancel_waits_on_nonfatal_error and fix operation during location update
* Updated changelog and updated comments/debug statements
* fix swift build and test and tsan errors
* Added test to replicate swift autoopen feature
* Fixed swift build issue
* removed an unused function
* Updates from review

Author: Michael Wilkerson-Barker

CHANGELOG.md

@@ -20,6 +20,7 @@
 * Non-streaming download sync progress notification is fixed for flexible sync Realms where before it was sometimes stopping to emit values right after the registration of the callback (PR [#7561](https://github.com/realm/realm-core/issues/7561)).
 * Schema initialization could hit an assertion failure if the sync client applied a downloaded changeset while the Realm file was in the process of being opened ([#7041](https://github.com/realm/realm-core/issues/7041), since v11.4.0).
 * Queries using query paths on Mixed values returns inconsistent results ([#7587](https://github.com/realm/realm-core/issues/7587), since v14.0.0)
+* Enabling 'cancel_waits_on_nonfatal_error' does not cancel waits during location update while offline ([#7527](https://github.com/realm/realm-core/issues/7527), since v13.26.0)
 
 ### Breaking changes
 * The following things have been renamed or moved as part of moving all of the App Services functionality to the app namespace:

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

@@ -842,6 +842,7 @@ void App::log_out(const std::shared_ptr<User>& user, SyncUser::State new_state,
         return;
     }
 
+    log_debug("App: log_out(%1)", user->user_id());
     auto request =
         make_request(HttpMethod::del, url_for_path("/auth/session"), user, RequestTokenType::RefreshToken, "");
 
@@ -1284,14 +1285,17 @@ void App::refresh_access_token(const std::shared_ptr<User>& user, bool update_lo
         return;
     }
 
-    log_debug("App: refresh_access_token: email: %1 %2", user->user_profile().email(),
-              update_location ? "(updating location)" : "");
+    log_debug("App: refresh_access_token: user_id: %1%2", user->user_id(),
+              update_location ? " (updating location)" : "");
 
     // If update_location is set, force the location info to be updated before sending the request
     do_request(
         make_request(HttpMethod::post, url_for_path("/auth/session"), user, RequestTokenType::RefreshToken, ""),
         [completion = std::move(completion), self = shared_from_this(), user](auto&&, const Response& response) {
             if (auto error = AppUtils::check_for_errors(response)) {
+                self->log_error("App: refresh_access_token: %1 -> %2 ERROR: %3", user->user_id(),
+                                response.http_status_code, error->what());
+
                 return completion(std::move(error));
             }
 

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

@@ -340,9 +340,15 @@ SyncSession::handle_refresh(const std::shared_ptr<SyncSession>& session, bool re
                 // internal backoff timer which will happen automatically so nothing needs to
                 // happen here.
                 util::CheckedUniqueLock lock(session->m_state_mutex);
+                // If updating access token while opening realm, just become active at this point
+                // and try to use the current access token.
                 if (session->m_state == State::WaitingForAccessToken) {
                     session->become_active();
                 }
+                // If `cancel_waits_on_nonfatal_error` is true, then cancel the waiters and pass along the error
+                else if (session->config(&SyncConfig::cancel_waits_on_nonfatal_error)) {
+                    session->cancel_pending_waits(std::move(lock), error->to_status()); // unlocks the mutex
+                }
             }
         }
         else {

test/object-store/realm.cpp

@@ -45,14 +45,19 @@
 
 #if REALM_ENABLE_SYNC
 #include <util/sync/flx_sync_harness.hpp>
+#include <util/sync/sync_test_utils.hpp>
+#include <util/test_file.hpp>
+#ifdef REALM_ENABLE_AUTH_TESTS
+#include <util/sync/baas_admin_api.hpp>
+#endif // REALM_ENABLE_AUTH_TESTS
 
 #include <realm/object-store/sync/async_open_task.hpp>
 #include <realm/object-store/sync/impl/app_metadata.hpp>
 #include <realm/object-store/sync/sync_session.hpp>
 
 #include <realm/sync/noinst/client_history_impl.hpp>
 #include <realm/sync/subscriptions.hpp>
-#endif
+#endif // REALM_ENABLE_SYNC
 
 #include <catch2/catch_all.hpp>
 #include <catch2/matchers/catch_matchers_string.hpp>
@@ -62,7 +67,7 @@
 #include <array>
 #if REALM_HAVE_UV
 #include <uv.h>
-#endif
+#endif // REALM_HAVE_UV
 
 namespace realm {
 class TestHelper {
@@ -1215,6 +1220,148 @@ TEST_CASE("Get Realm using Async Open", "[sync][pbs][async open]") {
         REQUIRE(got_error);
     }
 
+#if REALM_APP_SERVICES
+
+    SECTION("waiters are cancelled if cancel_waits_on_nonfatal_error") {
+        auto logger = util::Logger::get_default_logger();
+        auto transport = std::make_shared<HookedTransport<UnitTestTransport>>();
+        auto socket_provider = std::make_shared<HookedSocketProvider>(logger, "some user agent");
+        enum TestMode { expired_at_start, expired_by_websocket, websocket_fails };
+        enum FailureMode { location_fails, token_fails, token_not_authorized };
+        auto txt_test_mode = [](TestMode mode) {
+            switch (mode) {
+                case TestMode::expired_at_start:
+                    return "access token expired when realm is opened";
+                case TestMode::expired_by_websocket:
+                    return "access token expired by websocket";
+                case TestMode::websocket_fails:
+                    return "websocket returns connection failed";
+                default:
+                    return "Unknown TestMode";
+            }
+        };
+        auto txt_failure_mode = [](FailureMode mode) {
+            switch (mode) {
+                case FailureMode::location_fails:
+                    return "location update fails";
+                case FailureMode::token_fails:
+                    return "access token refresh fails";
+                case FailureMode::token_not_authorized:
+                    return "websocket connect not authorized";
+                default:
+                    return "Unknown FailureMode";
+            }
+        };
+
+        app::AppConfig app_config;
+        set_app_config_defaults(app_config, transport);
+        app_config.sync_client_config.socket_provider = socket_provider;
+        app_config.base_file_path = util::make_temp_dir();
+        app_config.metadata_mode = app::AppConfig::MetadataMode::NoEncryption;
+
+        auto the_app = app::App::get_app(app::App::CacheMode::Disabled, app_config);
+        create_user_and_log_in(the_app);
+        auto user = the_app->current_user();
+        // User should be logged in at this point
+        REQUIRE(user->is_logged_in());
+
+        bool not_authorized = false;
+        bool token_refresh_called = false;
+        bool location_refresh_called = false;
+
+        TestMode test_mode = GENERATE(expired_at_start, expired_by_websocket, websocket_fails);
+        FailureMode failure = GENERATE(location_fails, token_fails, token_not_authorized);
+
+        DYNAMIC_SECTION(txt_test_mode(test_mode) << " - " << txt_failure_mode(failure)) {
+            logger->info("TEST: %1 - %2", txt_test_mode(test_mode), txt_failure_mode(failure));
+            if (test_mode == TestMode::expired_at_start) {
+                // invalidate the user's cached access token
+                auto app_user = the_app->current_user();
+                app_user->update_data_for_testing([&](app::UserData& data) {
+                    data.access_token = RealmJWT(expired_token);
+                });
+            }
+            else if (test_mode == TestMode::expired_by_websocket) {
+                // tell websocket to return not authorized to refresh access token
+                not_authorized = true;
+            }
+        }
+
+        the_app.reset();
+
+        auto err_handler = [](std::shared_ptr<SyncSession> session, SyncError error) {
+            auto logger = util::Logger::get_default_logger();
+            logger->debug("The sync error handler caught an error: '%1' for '%2'", error.status, session->path());
+            // Ignore connection failed non-fatal errors and check for access token refresh unauthorized fatal errors
+            if (error.status.code() == ErrorCodes::SyncConnectFailed) {
+                REQUIRE_FALSE(error.is_fatal);
+                return;
+            }
+            // If it's not SyncConnectFailed, then it should be AuthError
+            REQUIRE(error.status.code() == ErrorCodes::AuthError);
+            REQUIRE(error.is_fatal);
+        };
+
+        transport->request_hook = [&](const app::Request& req) -> std::optional<app::Response> {
+            static constexpr int CURLE_OPERATION_TIMEDOUT = 28;
+            std::lock_guard<std::mutex> lock(mutex);
+            if (req.url.find("/auth/session") != std::string::npos) {
+                token_refresh_called = true;
+                if (failure == FailureMode::token_not_authorized) {
+                    return app::Response{403, 0, {}, "403 not authorized"};
+                }
+                if (failure == FailureMode::token_fails) {
+                    return app::Response{0, CURLE_OPERATION_TIMEDOUT, {}, "Operation timed out"};
+                }
+            }
+            else if (req.url.find("/location") != std::string::npos) {
+                location_refresh_called = true;
+                if (failure == FailureMode::location_fails) {
+                    // Fake "offline/request timed out" custom error response
+                    return app::Response{0, CURLE_OPERATION_TIMEDOUT, {}, "Operation timed out"};
+                }
+            }
+            return std::nullopt;
+        };
+
+        socket_provider->websocket_connect_func = [&]() -> std::optional<SocketProviderError> {
+            if (not_authorized) {
+                not_authorized = false; // one shot
+                return SocketProviderError(sync::websocket::WebSocketError::websocket_unauthorized,
+                                           "403 not authorized");
+            }
+            return SocketProviderError(sync::websocket::WebSocketError::websocket_connection_failed,
+                                       "Operation timed out");
+        };
+
+        the_app = app::App::get_app(app::App::CacheMode::Disabled, app_config);
+        SyncTestFile config(the_app->current_user(), "realm");
+        config.sync_config->cancel_waits_on_nonfatal_error = true;
+        config.sync_config->error_handler = err_handler;
+
+        // User should be logged in at this point
+        REQUIRE(config.sync_config->user->is_logged_in());
+
+        auto task = Realm::get_synchronized_realm(config);
+        auto pf = util::make_promise_future<std::exception_ptr>();
+        task->start([&pf](auto ref, auto error) mutable {
+            REQUIRE(!ref);
+            REQUIRE(error);
+            pf.promise.emplace_value(error);
+        });
+
+        auto result = pf.future.get_no_throw();
+        REQUIRE(result.is_ok());
+        REQUIRE(result.get_value());
+        std::lock_guard<std::mutex> lock(mutex);
+        REQUIRE(location_refresh_called);
+        if (failure != FailureMode::location_fails) {
+            REQUIRE(token_refresh_called);
+        }
+    }
+
+#endif // REALM_APP_SERVICES
+
     SECTION("read-only mode sets the schema version") {
         {
             SharedRealm realm = Realm::get_shared_realm(config);
@@ -1348,6 +1495,109 @@ TEST_CASE("Get Realm using Async Open", "[sync][pbs][async open]") {
     }
 }
 
+#if REALM_ENABLE_AUTH_TESTS
+
+TEST_CASE("Syhcnronized realm: AutoOpen", "[sync][baas][pbs][async open]") {
+    const auto partition = random_string(100);
+    auto schema = get_default_schema();
+    enum TestMode { expired_at_start, expired_by_websocket, websocket_fails };
+    enum FailureMode { location_fails, token_fails, token_not_authorized };
+
+    auto logger = util::Logger::get_default_logger();
+    auto transport = std::make_shared<HookedTransport<>>();
+    auto socket_provider = std::make_shared<HookedSocketProvider>(logger, "some user agent");
+    std::mutex mutex;
+
+    // Create the app session and get the logged in user identity
+    auto server_app_config = minimal_app_config("autoopen-realm", schema);
+    TestAppSession session(create_app(server_app_config), transport, DeleteApp{true}, realm::ReconnectMode::normal,
+                           socket_provider);
+    auto user = session.app()->current_user();
+    std::string identity = user->user_id();
+    REQUIRE(user->is_logged_in());
+    REQUIRE(!identity.empty());
+    // Reopen the App instance and retrieve the cached user
+    session.reopen(false);
+    user = session.app()->get_existing_logged_in_user(identity);
+
+    SyncTestFile config(user, partition, schema);
+    config.sync_config->cancel_waits_on_nonfatal_error = true;
+    config.sync_config->error_handler = [&logger](std::shared_ptr<SyncSession> session, SyncError error) {
+        logger->debug("The sync error handler caught an error: '%1' for '%2'", error.status, session->path());
+        // Ignore connection failed non-fatal errors and check for access token refresh unauthorized fatal errors
+        if (error.status.code() == ErrorCodes::SyncConnectFailed) {
+            REQUIRE_FALSE(error.is_fatal);
+            return;
+        }
+        // If it's not SyncConnectFailed, then it should be AuthError
+        REQUIRE(error.status.code() == ErrorCodes::AuthError);
+        REQUIRE(error.is_fatal);
+    };
+
+    bool not_authorized = false;
+    bool token_refresh_called = false;
+    bool location_refresh_called = false;
+
+    FailureMode failure = FailureMode::location_fails;
+
+    transport->request_hook = [&](const app::Request& req) -> std::optional<app::Response> {
+        static constexpr int CURLE_OPERATION_TIMEDOUT = 28;
+        std::lock_guard<std::mutex> lock(mutex);
+        if (req.url.find("/auth/session") != std::string::npos) {
+            token_refresh_called = true;
+            if (failure == FailureMode::token_not_authorized) {
+                return app::Response{403, 0, {}, "403 not authorized"};
+            }
+            if (failure == FailureMode::token_fails) {
+                return app::Response{0, CURLE_OPERATION_TIMEDOUT, {}, "Operation timed out"};
+            }
+        }
+        else if (req.url.find("/location") != std::string::npos) {
+            location_refresh_called = true;
+            if (failure == FailureMode::location_fails) {
+                // Fake "offline/request timed out" custom error response
+                return app::Response{0, CURLE_OPERATION_TIMEDOUT, {}, "Operation timed out"};
+            }
+        }
+        return std::nullopt;
+    };
+
+    socket_provider->websocket_connect_func = [&]() -> std::optional<SocketProviderError> {
+        if (not_authorized) {
+            not_authorized = false; // one shot
+            return SocketProviderError(sync::websocket::WebSocketError::websocket_unauthorized, "403 not authorized");
+        }
+        return SocketProviderError(sync::websocket::WebSocketError::websocket_connection_failed,
+                                   "Operation timed out");
+    };
+
+    auto task = Realm::get_synchronized_realm(config);
+    auto pf = util::make_promise_future<std::exception_ptr>();
+    task->start([&pf](auto ref, auto error) mutable {
+        REQUIRE(!ref);
+        REQUIRE(error);
+        pf.promise.emplace_value(error);
+    });
+
+    auto result = pf.future.get_no_throw();
+    REQUIRE(result.is_ok());
+    REQUIRE(result.get_value());
+    {
+        std::lock_guard<std::mutex> lock(mutex);
+        REQUIRE(location_refresh_called);
+        if (failure != FailureMode::location_fails) {
+            REQUIRE(token_refresh_called);
+        }
+    }
+
+    transport->request_hook = nullptr;
+    socket_provider->websocket_connect_func = nullptr;
+    auto r = Realm::get_shared_realm(config);
+    wait_for_download(*r);
+}
+
+#endif // REALM_ENABLE_AUTH_TESTS
+
 TEST_CASE("SharedRealm: convert", "[sync][pbs][convert]") {
     TestSyncManager tsm;
     ObjectSchema object_schema = {"object",

test/object-store/util/sync/baas_admin_api.cpp

@@ -346,8 +346,11 @@ app::Response do_http_request(const app::Request& request)
 
     auto logger = util::Logger::get_default_logger();
     if (response_code != CURLE_OK) {
+        std::string message = curl_easy_strerror(response_code);
         logger->error("curl_easy_perform() failed when sending request to '%1' with body '%2': %3", request.url,
-                      request.body, curl_easy_strerror(response_code));
+                      request.body, message);
+        // Return a failing response with the CURL error as the custom code
+        return {0, response_code, {}, message};
     }
     if (logger->would_log(util::Logger::Level::trace)) {
         std::string coid = [&] {