From ca64a613c9ee961dbd99565d3917bfad9972033f Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 12 Aug 2026 22:46:49 +0000 Subject: [PATCH] Stop crashing on transient network failures during archive media streams. --- .../network/RequestResultClassification.kt | 31 ++++++++++ .../RequestResultClassificationTest.kt | 60 +++++++++++++++++++ .../org/signal/network/api/ArchiveApiV2.kt | 2 +- 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 core/network/src/main/java/org/signal/network/RequestResultClassification.kt create mode 100644 core/network/src/test/java/org/signal/network/RequestResultClassificationTest.kt diff --git a/core/network/src/main/java/org/signal/network/RequestResultClassification.kt b/core/network/src/main/java/org/signal/network/RequestResultClassification.kt new file mode 100644 index 0000000000..c359e2333b --- /dev/null +++ b/core/network/src/main/java/org/signal/network/RequestResultClassification.kt @@ -0,0 +1,31 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.signal.network + +import org.signal.libsignal.net.BadRequestError +import org.signal.libsignal.net.RequestResult +import java.io.IOException +import org.signal.libsignal.net.toRequestResult as libsignalToRequestResult + +/** + * Classifies [this] as a [RequestResult]. Prefer this over libsignal's `Throwable.toRequestResult()`. + * + * Libsignal's classifier misses handling things like IOException and forwards them as application errors. + */ +inline fun Throwable.toRequestResult(): RequestResult { + if (this is E) { + return RequestResult.NonSuccess(this) + } + + val result = libsignalToRequestResult() + val unclassified = (result as? RequestResult.ApplicationError)?.cause + + return if (unclassified is IOException) { + RequestResult.RetryableNetworkError(unclassified) + } else { + result + } +} diff --git a/core/network/src/test/java/org/signal/network/RequestResultClassificationTest.kt b/core/network/src/test/java/org/signal/network/RequestResultClassificationTest.kt new file mode 100644 index 0000000000..8542686d55 --- /dev/null +++ b/core/network/src/test/java/org/signal/network/RequestResultClassificationTest.kt @@ -0,0 +1,60 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.signal.network + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import assertk.assertions.isSameInstanceAs +import org.junit.Test +import org.signal.libsignal.net.RequestResult +import org.signal.libsignal.net.RequestUnauthorizedException +import org.signal.libsignal.net.RetryLaterException +import java.net.SocketException +import java.time.Duration + +class RequestResultClassificationTest { + + @Test + fun `plain IOException is retryable`() { + val exception = SocketException("no connection") + + val result = exception.toRequestResult() + + assertThat(result).isInstanceOf(RequestResult.RetryableNetworkError::class) + assertThat((result as RequestResult.RetryableNetworkError).networkError).isSameInstanceAs(exception) + } + + @Test + fun `non-IOException is an application error`() { + val exception = IllegalStateException("bug") + + val result = exception.toRequestResult() + + assertThat(result).isInstanceOf(RequestResult.ApplicationError::class) + assertThat((result as RequestResult.ApplicationError).cause).isSameInstanceAs(exception) + } + + @Test + fun `libsignal classification still wins over the IOException fallback`() { + val exception = RetryLaterException(Duration.ofSeconds(30)) + + val result = exception.toRequestResult() + + assertThat(result).isInstanceOf(RequestResult.RetryableNetworkError::class) + assertThat((result as RequestResult.RetryableNetworkError).retryAfter).isEqualTo(Duration.ofSeconds(30)) + } + + @Test + fun `expected error is a non-success even though it is an IOException`() { + val exception = RequestUnauthorizedException("nope") + + val result = exception.toRequestResult() + + assertThat(result).isInstanceOf(RequestResult.NonSuccess::class) + assertThat((result as RequestResult.NonSuccess).error).isSameInstanceAs(exception) + } +} diff --git a/lib/network/src/main/java/org/signal/network/api/ArchiveApiV2.kt b/lib/network/src/main/java/org/signal/network/api/ArchiveApiV2.kt index 8dd9444e12..ac3aa15a9f 100644 --- a/lib/network/src/main/java/org/signal/network/api/ArchiveApiV2.kt +++ b/lib/network/src/main/java/org/signal/network/api/ArchiveApiV2.kt @@ -30,7 +30,6 @@ import org.signal.libsignal.net.ServerSideErrorException import org.signal.libsignal.net.UnauthBackupsService import org.signal.libsignal.net.UnauthenticatedChatConnection import org.signal.libsignal.net.UploadForm -import org.signal.libsignal.net.toRequestResult import org.signal.libsignal.protocol.ecc.ECPrivateKey import org.signal.libsignal.zkgroup.GenericServerPublicParams import org.signal.libsignal.zkgroup.InvalidInputException @@ -38,6 +37,7 @@ import org.signal.libsignal.zkgroup.VerificationFailedException import org.signal.libsignal.zkgroup.backups.BackupAuthCredential import org.signal.libsignal.zkgroup.backups.BackupAuthCredentialRequestContext import org.signal.libsignal.zkgroup.backups.BackupAuthCredentialResponse +import org.signal.network.toRequestResult import org.signal.network.websocket.WebSocketRequestMessage import org.signal.network.websocket.WebsocketResponse import org.signal.network.websocket.get