[RKOTLIN-1096] Add SyncException.isFatal to signal unrecoverable sync exceptions (#1800)

Author: rorbech

.github/workflows/include-static-analysis.yml

@@ -33,7 +33,6 @@ jobs:
 
       - name: Run Ktlint
         run: ./gradlew ktlintCheck
-        continue-on-error: true
 
       - name: Stash Ktlint results
         run: |
@@ -85,7 +84,6 @@ jobs:
 
       - name: Run Detekt
         run: ./gradlew detekt
-        continue-on-error: true
 
       - name: Stash Detekt results    
         run: |

.github/workflows/pr.yml

@@ -1782,4 +1782,4 @@ jobs:
     secrets: inherit
     with:
       version-label: ${{ needs.check-cache.outputs.version-label }}
-      packages-sha-label: ${{ needs.check-cache.outputs.packages-sha }}
+      packages-sha-label: ${{ needs.check-cache.outputs.packages-sha }}

CHANGELOG.md

@@ -6,6 +6,7 @@
 ### Enhancements
 - Avoid exporting Core's symbols so we can statically build the Kotlin SDK with other SDKs like Swift in the same project. (Issue [JIRA](https://jira.mongodb.org/browse/RKOTLIN-877)).
 - Improved mechanism for unpacking of JVM native libs suitable for local development. (Issue [#1715](https://github.com/realm/realm-kotlin/issues/1715) [JIRA](https://jira.mongodb.org/browse/RKOTLIN-1065)).
+* [Sync] Add `SyncException.isFatal` to signal fatal unrecoverable exceptions. (Issue [#1767](https://github.com/realm/realm-kotlin/issues/1767) [RKOTLIN-1096](https://jira.mongodb.org/browse/RKOTLIN-1096)).
 
 ### Fixed
 - None.

packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/exceptions/SyncExceptions.kt

@@ -19,6 +19,7 @@ package io.realm.kotlin.mongodb.exceptions
 
 import io.realm.kotlin.internal.asPrimitiveRealmAnyOrElse
 import io.realm.kotlin.internal.interop.sync.CoreCompensatingWriteInfo
+import io.realm.kotlin.mongodb.sync.SyncSession
 import io.realm.kotlin.types.RealmAny
 
 /**
@@ -31,7 +32,19 @@ import io.realm.kotlin.types.RealmAny
  *
  * @see io.realm.kotlin.mongodb.sync.SyncConfiguration.Builder.errorHandler
  */
-public open class SyncException internal constructor(message: String?) : AppException(message)
+public open class SyncException internal constructor(message: String?, isFatal: Boolean) : AppException(message) {
+    /**
+     * Flag to indicate that something has gone wrong with Device Sync in a way that is not
+     * recoverable and [SyncSession] will be [SyncSession.State.INACTIVE] until this error is
+     * resolved.
+     *
+     * It is still possible to use the Realm locally after receiving an error where this flag is
+     * true. However, this must be done with caution as data written to the realm after this point
+     * risk getting lost as many errors of this category will result in a Client Reset once the
+     * client re-connects to the server.
+     */
+    public val isFatal: Boolean = isFatal
+}
 
 /**
  * Thrown when something has gone wrong with Device Sync in a way that is not recoverable.
@@ -47,30 +60,33 @@ public open class SyncException internal constructor(message: String?) : AppExce
  *
  * @see io.realm.kotlin.mongodb.sync.SyncConfiguration.Builder.errorHandler
  */
-public class UnrecoverableSyncException internal constructor(message: String) :
-    SyncException(message)
+@Deprecated("This will be removed in the future. Test for SyncException.isFatal instead.")
+public open class UnrecoverableSyncException internal constructor(message: String) :
+    SyncException(message, true)
 
 /**
  * Thrown when the type of sync used by the server does not match the one used by the client, i.e.
  * the server and client disagrees whether to use Partition-based or Flexible Sync.
  */
-public class WrongSyncTypeException internal constructor(message: String) : SyncException(message)
+public class WrongSyncTypeException internal constructor(message: String) :
+    UnrecoverableSyncException(message)
 
 /**
  * Thrown when the server does not support one or more of the queries defined in the
  * [io.realm.kotlin.mongodb.sync.SubscriptionSet].
  */
-public class BadFlexibleSyncQueryException internal constructor(message: String?) :
-    SyncException(message)
+public class BadFlexibleSyncQueryException internal constructor(message: String?, isFatal: Boolean) :
+    SyncException(message, isFatal)
 
 /**
  * Thrown when the server undoes one or more client writes. Details on undone writes can be found in
  * [writes].
  */
 public class CompensatingWriteException internal constructor(
     message: String,
-    compensatingWrites: Array<CoreCompensatingWriteInfo>
-) : SyncException(message) {
+    compensatingWrites: Array<CoreCompensatingWriteInfo>,
+    isFatal: Boolean
+) : SyncException(message, isFatal) {
     /**
      * List of all the objects created that has been reversed as part of triggering this exception.
      */

packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/internal/RealmQueryExtImpl.kt

@@ -69,7 +69,7 @@ internal suspend fun <T : RealmObject> createSubscriptionFromQuery(
                 realm.syncSession.downloadAllServerChanges()
                 subscriptions.refresh()
                 subscriptions.errorMessage?.let { errorMessage: String ->
-                    throw BadFlexibleSyncQueryException(errorMessage)
+                    throw BadFlexibleSyncQueryException(errorMessage, isFatal = false)
                 }
             }
             // Rerun the query on the latest Realm version.

packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/internal/RealmSyncUtils.kt

@@ -78,33 +78,32 @@ internal fun <T, R> channelResultCallback(
 internal fun convertSyncError(syncError: SyncError): SyncException {
     val errorCode = syncError.errorCode
     val message = createMessageFromSyncError(errorCode)
-    return if (syncError.isFatal) {
-        // An unrecoverable exception happened
-        UnrecoverableSyncException(message)
-    } else {
-        when (errorCode.errorCode) {
-            ErrorCode.RLM_ERR_WRONG_SYNC_TYPE -> WrongSyncTypeException(message)
+    return when (errorCode.errorCode) {
+        ErrorCode.RLM_ERR_WRONG_SYNC_TYPE -> WrongSyncTypeException(message)
 
-            ErrorCode.RLM_ERR_INVALID_SUBSCRIPTION_QUERY -> {
-                // Flexible Sync Query was rejected by the server
-                BadFlexibleSyncQueryException(message)
-            }
+        ErrorCode.RLM_ERR_INVALID_SUBSCRIPTION_QUERY -> {
+            // Flexible Sync Query was rejected by the server
+            BadFlexibleSyncQueryException(message, syncError.isFatal)
+        }
 
-            ErrorCode.RLM_ERR_SYNC_COMPENSATING_WRITE -> CompensatingWriteException(
-                message,
-                syncError.compensatingWrites
-            )
-            ErrorCode.RLM_ERR_SYNC_PROTOCOL_INVARIANT_FAILED,
-            ErrorCode.RLM_ERR_SYNC_PROTOCOL_NEGOTIATION_FAILED,
-            ErrorCode.RLM_ERR_SYNC_PERMISSION_DENIED -> {
-                // Permission denied errors should be unrecoverable according to Core, i.e. the
-                // client will disconnect sync and transition to the "inactive" state
-                UnrecoverableSyncException(message)
-            }
-            else -> {
-                // An error happened we are not sure how to handle. Just report as a generic
-                // SyncException.
-                SyncException(message)
+        ErrorCode.RLM_ERR_SYNC_COMPENSATING_WRITE -> CompensatingWriteException(
+            message,
+            syncError.compensatingWrites,
+            syncError.isFatal
+        )
+        ErrorCode.RLM_ERR_SYNC_PROTOCOL_INVARIANT_FAILED,
+        ErrorCode.RLM_ERR_SYNC_PROTOCOL_NEGOTIATION_FAILED,
+        ErrorCode.RLM_ERR_SYNC_PERMISSION_DENIED -> {
+            // Permission denied errors should be unrecoverable according to Core, i.e. the
+            // client will disconnect sync and transition to the "inactive" state
+            UnrecoverableSyncException(message)
+        }
+        else -> {
+            // An error happened we are not sure how to handle. Just report as a generic
+            // SyncException.
+            when (syncError.isFatal) {
+                false -> SyncException(message, syncError.isFatal)
+                true -> UnrecoverableSyncException(message)
             }
         }
     }

packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/internal/SubscriptionSetImpl.kt

@@ -127,7 +127,7 @@ internal class SubscriptionSetImpl<T : BaseRealm>(
                     if (result) {
                         return true
                     } else {
-                        throw BadFlexibleSyncQueryException(errorMessage)
+                        throw BadFlexibleSyncQueryException(errorMessage, isFatal = false)
                     }
                 }
                 else -> throw IllegalStateException("Unexpected value: $result")

packages/test-sync/src/commonTest/kotlin/io/realm/kotlin/test/mongodb/common/SyncedRealmTests.kt

@@ -45,6 +45,7 @@ import io.realm.kotlin.mongodb.User
 import io.realm.kotlin.mongodb.exceptions.DownloadingRealmTimeOutException
 import io.realm.kotlin.mongodb.exceptions.SyncException
 import io.realm.kotlin.mongodb.exceptions.UnrecoverableSyncException
+import io.realm.kotlin.mongodb.exceptions.WrongSyncTypeException
 import io.realm.kotlin.mongodb.internal.SyncSessionImpl
 import io.realm.kotlin.mongodb.subscriptions
 import io.realm.kotlin.mongodb.sync.InitialSubscriptionsCallback
@@ -370,7 +371,10 @@ class SyncedRealmTests {
             // Second
             channel.receiveOrFail().let { error ->
                 assertNotNull(error.message)
+                // Deprecated
                 assertIs<UnrecoverableSyncException>(error)
+                assertIs<SyncException>(error)
+                assertTrue(error.isFatal)
             }
 
             deferred.cancel()
@@ -417,6 +421,49 @@ class SyncedRealmTests {
         }
     }
 
+    @Test
+    fun errorHandler_wrongSyncTypeException() {
+        val channel = TestChannel<Throwable>()
+        // Remove permissions to generate a sync error containing ONLY the original path
+        // This way we assert we don't read wrong data from the user_info field
+        val (email, password) = "test_nowrite_noread_${randomEmail()}" to "password1234"
+        val user = runBlocking {
+            app.createUserAndLogIn(email, password)
+        }
+
+        // Opens FLX synced realm against a PBS app
+        val config = SyncConfiguration.Builder(
+            schema = setOf(ParentPk::class, ChildPk::class),
+            user = user,
+        ).errorHandler { _, error ->
+            channel.trySendOrFail(error)
+        }.build()
+
+        runBlocking {
+            val deferred = async {
+                Realm.open(config).use {
+                    // Make sure that the test eventually fail. Coroutines can cancel a delay
+                    // so this doesn't always block the test for 10 seconds.
+                    delay(10_000)
+                    channel.send(AssertionError("Realm was successfully opened"))
+                }
+            }
+
+            val error = channel.receiveOrFail()
+            val message = error.message
+            assertNotNull(message)
+            assertIs<WrongSyncTypeException>(error)
+            assertTrue(error.isFatal)
+            // Deprecated
+            assertIs<UnrecoverableSyncException>(error)
+            assertTrue(
+                message.contains("Client connected using flexible sync when app is using partition-based sync"),
+                "Was: $message"
+            )
+            deferred.cancel()
+        }
+    }
+
     @Test
     fun testErrorHandler() {
         // Open a realm with a schema. Close it without doing anything else