From 0ed5c5825d4c3f9932cdee6c1c9858ab651edf82 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 20 Jan 2023 15:24:14 +1100 Subject: [PATCH] Cleaned up some of the error logging --- .../securesms/ApplicationContext.java | 5 ++++ .../messaging/file_server/FileServerApi.kt | 6 ++++- .../messaging/jobs/BatchMessageReceiveJob.kt | 24 ++++++++++++----- .../messaging/jobs/MessageSendJob.kt | 26 ++++++++++++++----- .../messaging/open_groups/OpenGroupApi.kt | 6 ++++- .../libsession/snode/OnionRequestAPI.kt | 4 +-- .../libsession/utilities/DownloadUtilities.kt | 7 ++++- .../org/session/libsignal/utilities/HTTP.kt | 14 +++++++--- 8 files changed, 71 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java b/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java index 4f1270acc5..ef4f5c46a3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java +++ b/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java @@ -47,6 +47,7 @@ import org.session.libsession.utilities.Util; import org.session.libsession.utilities.WindowDebouncer; import org.session.libsession.utilities.dynamiclanguage.DynamicLanguageContextWrapper; import org.session.libsession.utilities.dynamiclanguage.LocaleParser; +import org.session.libsignal.utilities.HTTP; import org.session.libsignal.utilities.JsonUtil; import org.session.libsignal.utilities.Log; import org.session.libsignal.utilities.ThreadUtils; @@ -67,6 +68,7 @@ import org.thoughtcrime.securesms.groups.OpenGroupMigrator; import org.thoughtcrime.securesms.home.HomeActivity; import org.thoughtcrime.securesms.jobmanager.JobManager; import org.thoughtcrime.securesms.jobmanager.impl.JsonDataSerializer; +import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint; import org.thoughtcrime.securesms.jobs.FastJobStorage; import org.thoughtcrime.securesms.jobs.JobManagerFactories; import org.thoughtcrime.securesms.logging.AndroidLogger; @@ -237,6 +239,9 @@ public class ApplicationContext extends Application implements DefaultLifecycleO resubmitProfilePictureIfNeeded(); loadEmojiSearchIndexIfNeeded(); EmojiSource.refresh(); + + NetworkConstraint networkConstraint = new NetworkConstraint.Factory(this).create(); + HTTP.INSTANCE.setConnectedToNetwork(networkConstraint::isMet); } @Override diff --git a/libsession/src/main/java/org/session/libsession/messaging/file_server/FileServerApi.kt b/libsession/src/main/java/org/session/libsession/messaging/file_server/FileServerApi.kt index f4972080be..01fae1f503 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/file_server/FileServerApi.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/file_server/FileServerApi.kt @@ -77,7 +77,11 @@ object FileServerApi { OnionRequestAPI.sendOnionRequest(requestBuilder.build(), server, serverPublicKey).map { it.body ?: throw Error.ParsingFailed }.fail { e -> - Log.e("Loki", "File server request failed.", e) + when (e) { + // No need for the stack trace for HTTP errors + is HTTP.HTTPRequestFailedException -> Log.e("Loki", "File server request failed due to error: ${e.message}") + else -> Log.e("Loki", "File server request failed", e) + } } } else { Promise.ofFail(IllegalStateException("It's currently not allowed to send non onion routed requests.")) diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt index 07c104cfda..7bf330fe3b 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt @@ -94,12 +94,24 @@ class BatchMessageReceiveJob( threadMap[threadID]!! += parsedParams } } catch (e: Exception) { - Log.e(TAG, "Couldn't receive message.", e) - if (e is MessageReceiver.Error && !e.isRetryable) { - Log.e(TAG, "Message failed permanently",e) - } else { - Log.e(TAG, "Message failed",e) - failures += messageParameters + when (e) { + is MessageReceiver.Error.DuplicateMessage, MessageReceiver.Error.SelfSend -> { + Log.i(TAG, "Couldn't receive message, failed with error: ${e.message}") + failures += messageParameters + } + is MessageReceiver.Error -> { + if (!e.isRetryable) { + Log.e(TAG, "Couldn't receive message, failed permanently", e) + } + else { + Log.e(TAG, "Couldn't receive message, failed", e) + failures += messageParameters + } + } + else -> { + Log.e(TAG, "Couldn't receive message, failed", e) + failures += messageParameters + } } } } diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/MessageSendJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/MessageSendJob.kt index cdd9e0a3ac..8ce1adf481 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/MessageSendJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/MessageSendJob.kt @@ -11,6 +11,7 @@ import org.session.libsession.messaging.messages.visible.VisibleMessage import org.session.libsession.messaging.sending_receiving.MessageSender import org.session.libsession.messaging.utilities.Data import org.session.libsession.snode.OnionRequestAPI +import org.session.libsignal.utilities.HTTP import org.session.libsignal.utilities.Log class MessageSendJob(val message: Message, val destination: Destination) : Job { @@ -67,14 +68,25 @@ class MessageSendJob(val message: Message, val destination: Destination) : Job { val promise = MessageSender.send(this.message, this.destination).success { this.handleSuccess() }.fail { exception -> - Log.e(TAG, "Couldn't send message due to error: $exception.") - if (exception is MessageSender.Error) { - if (!exception.isRetryable) { this.handlePermanentFailure(exception) } - } - if (exception is OnionRequestAPI.HTTPRequestFailedAtDestinationException && exception.statusCode == 429) { - this.handlePermanentFailure(exception) + var logStacktrace = true + + when (exception) { + // No need for the stack trace for HTTP errors + is HTTP.HTTPRequestFailedException -> { + logStacktrace = false + + if (exception.statusCode == 429) { this.handlePermanentFailure(exception) } + else { this.handleFailure(exception) } + } + is MessageSender.Error -> { + if (!exception.isRetryable) { this.handlePermanentFailure(exception) } + else { this.handleFailure(exception) } + } + else -> this.handleFailure(exception) } - this.handleFailure(exception) + + if (logStacktrace) { Log.e(TAG, "Couldn't send message due to error", exception) } + else { Log.e(TAG, "Couldn't send message due to error: ${exception.message}") } } try { promise.get() diff --git a/libsession/src/main/java/org/session/libsession/messaging/open_groups/OpenGroupApi.kt b/libsession/src/main/java/org/session/libsession/messaging/open_groups/OpenGroupApi.kt index 51f7108f5a..46eff4b03b 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/open_groups/OpenGroupApi.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/open_groups/OpenGroupApi.kt @@ -383,7 +383,11 @@ object OpenGroupApi { } return if (request.useOnionRouting) { OnionRequestAPI.sendOnionRequest(requestBuilder.build(), request.server, publicKey).fail { e -> - Log.e("SOGS", "Failed onion request", e) + when (e) { + // No need for the stack trace for HTTP errors + is HTTP.HTTPRequestFailedException -> Log.e("SOGS", "Failed onion request: ${e.message}") + else -> Log.e("SOGS", "Failed onion request", e) + } } } else { Promise.ofFail(IllegalStateException("It's currently not allowed to send non onion routed requests.")) diff --git a/libsession/src/main/java/org/session/libsession/snode/OnionRequestAPI.kt b/libsession/src/main/java/org/session/libsession/snode/OnionRequestAPI.kt index f93a7b243e..bcce887a5a 100644 --- a/libsession/src/main/java/org/session/libsession/snode/OnionRequestAPI.kt +++ b/libsession/src/main/java/org/session/libsession/snode/OnionRequestAPI.kt @@ -78,8 +78,8 @@ object OnionRequestAPI { // endregion class HTTPRequestFailedBlindingRequiredException(statusCode: Int, json: Map<*, *>, destination: String): HTTPRequestFailedAtDestinationException(statusCode, json, destination) - open class HTTPRequestFailedAtDestinationException(val statusCode: Int, val json: Map<*, *>, val destination: String) - : Exception("HTTP request failed at destination ($destination) with status code $statusCode.") + open class HTTPRequestFailedAtDestinationException(statusCode: Int, json: Map<*, *>, val destination: String) + : HTTP.HTTPRequestFailedException(statusCode, json, "HTTP request failed at destination ($destination) with status code $statusCode.") class InsufficientSnodesException : Exception("Couldn't find enough snodes to build a path.") private data class OnionBuildingResult( diff --git a/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt b/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt index 0a61d1ede0..b850baa253 100644 --- a/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt +++ b/libsession/src/main/java/org/session/libsession/utilities/DownloadUtilities.kt @@ -2,6 +2,7 @@ package org.session.libsession.utilities import okhttp3.HttpUrl import org.session.libsession.messaging.file_server.FileServerApi +import org.session.libsignal.utilities.HTTP import org.session.libsignal.utilities.Log import java.io.* @@ -40,7 +41,11 @@ object DownloadUtilities { outputStream.write(it) } } catch (e: Exception) { - Log.e("Loki", "Couldn't download attachment.", e) + when (e) { + // No need for the stack trace for HTTP errors + is HTTP.HTTPRequestFailedException -> Log.e("Loki", "Couldn't download attachment due to error: ${e.message}") + else -> Log.e("Loki", "Couldn't download attachment", e) + } throw e } } diff --git a/libsignal/src/main/java/org/session/libsignal/utilities/HTTP.kt b/libsignal/src/main/java/org/session/libsignal/utilities/HTTP.kt index aea1fce2d9..5eac7cecd4 100644 --- a/libsignal/src/main/java/org/session/libsignal/utilities/HTTP.kt +++ b/libsignal/src/main/java/org/session/libsignal/utilities/HTTP.kt @@ -12,6 +12,7 @@ import javax.net.ssl.SSLContext import javax.net.ssl.X509TrustManager object HTTP { + var isConnectedToNetwork: (() -> Boolean) = { false } private val seedNodeConnection by lazy { OkHttpClient().newBuilder() @@ -64,8 +65,12 @@ object HTTP { private const val timeout: Long = 120 - class HTTPRequestFailedException(val statusCode: Int, val json: Map<*, *>?) - : kotlin.Exception("HTTP request failed with status code $statusCode.") + open class HTTPRequestFailedException( + val statusCode: Int, + val json: Map<*, *>?, + message: String = "HTTP request failed with status code $statusCode" + ) : kotlin.Exception(message) + class HTTPNoNetworkException : HTTPRequestFailedException(0, null, "No network connection") enum class Verb(val rawValue: String) { GET("GET"), PUT("PUT"), POST("POST"), DELETE("DELETE") @@ -120,8 +125,11 @@ object HTTP { response = connection.newCall(request.build()).execute() } catch (exception: Exception) { Log.d("Loki", "${verb.rawValue} request to $url failed due to error: ${exception.localizedMessage}.") + + if (!isConnectedToNetwork()) { throw HTTPNoNetworkException() } + // Override the actual error so that we can correctly catch failed requests in OnionRequestAPI - throw HTTPRequestFailedException(0, null) + throw HTTPRequestFailedException(0, null, "HTTP request failed due to: ${exception.message}") } return when (val statusCode = response.code()) { 200 -> {