Commit 84bbde3f authored by Grisha Kruglov's avatar Grisha Kruglov Committed by Grisha Kruglov
Browse files

Make sure to re-throw places panics during sync

parent e6df84c3
......@@ -12,7 +12,6 @@ import mozilla.appservices.places.BookmarkSeparator
import mozilla.appservices.places.BookmarkTreeNode
import mozilla.appservices.places.BookmarkUpdateInfo
import mozilla.appservices.places.PlacesApi
import mozilla.appservices.places.PlacesException
import mozilla.components.concept.storage.BookmarkInfo
import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType
......@@ -179,13 +178,10 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo
* @return Sync status of OK or Error
*/
override suspend fun sync(authInfo: AuthInfo): SyncStatus {
return try {
withContext(scope.coroutineContext) {
return withContext(scope.coroutineContext) {
syncAndHandleExceptions {
places.syncBookmarks(authInfo.into())
SyncStatus.Ok
}
} catch (e: PlacesException) {
SyncStatus.Error(e)
}
}
}
......@@ -6,7 +6,6 @@ package mozilla.components.browser.storage.sync
import android.content.Context
import kotlinx.coroutines.withContext
import mozilla.appservices.places.PlacesException
import mozilla.appservices.places.VisitObservation
import mozilla.components.concept.storage.HistoryAutocompleteResult
import mozilla.components.concept.storage.HistoryStorage
......@@ -174,13 +173,10 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist
* @return Sync status of OK or Error
*/
override suspend fun sync(authInfo: AuthInfo): SyncStatus {
return try {
withContext(scope.coroutineContext) {
return withContext(scope.coroutineContext) {
syncAndHandleExceptions {
places.syncHistory(authInfo.into())
SyncStatus.Ok
}
} catch (e: PlacesException) {
SyncStatus.Error(e)
}
}
}
......@@ -10,10 +10,13 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.cancelChildren
import kotlinx.coroutines.withContext
import mozilla.appservices.places.InternalPanic
import mozilla.appservices.places.PlacesException
import mozilla.appservices.places.PlacesReaderConnection
import mozilla.appservices.places.PlacesWriterConnection
import mozilla.appservices.places.UrlParseFailed
import mozilla.components.concept.storage.Storage
import mozilla.components.concept.sync.SyncStatus
import mozilla.components.concept.sync.SyncableStore
import mozilla.components.support.base.log.logger.Logger
......@@ -63,4 +66,25 @@ abstract class PlacesStorage(context: Context) : Storage, SyncableStore {
logger.warn("Ignoring url exception while running $operation", e)
}
}
/**
* Runs a [syncBlock], re-throwing any panics that may be encountered.
* @return [SyncStatus.Ok] on success, or [SyncStatus.Error] on non-panic [PlacesException].
*/
protected inline fun syncAndHandleExceptions(syncBlock: () -> Unit): SyncStatus {
return try {
logger.debug("Syncing...")
syncBlock()
logger.debug("Successfully synced.")
SyncStatus.Ok
// Order of these catches matters: InternalPanic extends PlacesException.
} catch (e: InternalPanic) {
logger.error("Places panic while syncing", e)
throw e
} catch (e: PlacesException) {
logger.error("Places exception while syncing", e)
SyncStatus.Error(e)
}
}
}
......@@ -6,6 +6,7 @@ package mozilla.components.browser.storage.sync
import androidx.test.core.app.ApplicationProvider
import kotlinx.coroutines.runBlocking
import mozilla.appservices.places.InternalPanic
import mozilla.appservices.places.PlacesException
import mozilla.appservices.places.PlacesReaderConnection
import mozilla.appservices.places.PlacesWriterConnection
......@@ -577,4 +578,35 @@ class PlacesHistoryStorageTest {
val error = result as SyncStatus.Error
assertEquals("test error", error.exception.message)
}
@Test(expected = InternalPanic::class)
fun `storage re-throws sync panics`() = runBlocking {
val exception = InternalPanic("test panic")
val conn = object : Connection {
override fun reader(): PlacesReaderConnection {
fail()
return mock()
}
override fun writer(): PlacesWriterConnection {
fail()
return mock()
}
override fun syncHistory(syncInfo: SyncAuthInfo) {
throw exception
}
override fun syncBookmarks(syncInfo: SyncAuthInfo) {
fail()
}
override fun close() {
fail()
}
}
val storage = MockingPlacesHistoryStorage(conn)
storage.sync(AuthInfo("kid", "token", "key", "serverUrl"))
fail()
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment