diff --git a/libs/logging/src/main/kotlin/com/getcode/utils/ErrorUtils.kt b/libs/logging/src/main/kotlin/com/getcode/utils/ErrorUtils.kt index 828748fdc6..25b93af613 100644 --- a/libs/logging/src/main/kotlin/com/getcode/utils/ErrorUtils.kt +++ b/libs/logging/src/main/kotlin/com/getcode/utils/ErrorUtils.kt @@ -61,11 +61,7 @@ object ErrorUtils { ) } - if ( - BuildConfig.NOTIFY_ERRORS && - ignoredErrors.none { it.isInstance(throwable) } && - ignoredErrors.none { it.isInstance(throwableCause) } - ) { + if (BuildConfig.NOTIFY_ERRORS && shouldReport(throwable, throwableCause)) { val isNotifiable = when { throwable is ConditionallyNotifiable -> throwable.isNotifiable throwableCause is ConditionallyNotifiable -> throwableCause.isNotifiable @@ -76,6 +72,17 @@ object ErrorUtils { } } + /** + * Whether [throwable] is worth handing to the reporters at all. [UnreportedError] marks an + * expected server result the caller already handles, so it stays in the log without opening a + * Bugsnag error group. + */ + internal fun shouldReport(throwable: Throwable, cause: Throwable): Boolean = + throwable !is UnreportedError && + cause !is UnreportedError && + ignoredErrors.none { it.isInstance(throwable) } && + ignoredErrors.none { it.isInstance(cause) } + private fun isNetworkError(throwable: Throwable): Boolean = throwable is TimeoutException || throwable.cause is TimeoutException || diff --git a/libs/logging/src/main/kotlin/com/getcode/utils/UnreportedError.kt b/libs/logging/src/main/kotlin/com/getcode/utils/UnreportedError.kt new file mode 100644 index 0000000000..06606637d3 --- /dev/null +++ b/libs/logging/src/main/kotlin/com/getcode/utils/UnreportedError.kt @@ -0,0 +1,12 @@ +package com.getcode.utils + +/** + * Marker interface for errors that are an expected outcome rather than a fault — a server result + * the caller already handles and renders. [ErrorUtils] still logs these, so they stay in the trace + * attached to a real report, but it does not hand them to the reporters and they never open a + * Bugsnag error group. + * + * This is a third tier below [NotifiableError] (reported, WARNING, drives the Slack filter) and a + * plain [CodeServerError] (reported, INFO, recorded for reference). + */ +interface UnreportedError diff --git a/libs/logging/src/test/kotlin/com/getcode/utils/ErrorUtilsTest.kt b/libs/logging/src/test/kotlin/com/getcode/utils/ErrorUtilsTest.kt new file mode 100644 index 0000000000..84cc366dcd --- /dev/null +++ b/libs/logging/src/test/kotlin/com/getcode/utils/ErrorUtilsTest.kt @@ -0,0 +1,37 @@ +package com.getcode.utils + +import java.net.UnknownHostException +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class ErrorUtilsTest { + + private class Benign : Throwable("benign"), UnreportedError + + private class Fault : Throwable("fault") + + @Test + fun `a plain throwable is reported`() { + val error = Fault() + assertTrue(ErrorUtils.shouldReport(error, error)) + } + + @Test + fun `an UnreportedError is not reported`() { + val error = Benign() + assertFalse(ErrorUtils.shouldReport(error, error)) + } + + @Test + fun `an UnreportedError in the cause position is not reported`() { + assertFalse(ErrorUtils.shouldReport(Fault(), Benign())) + } + + @Test + fun `ignored error types stay ignored`() { + val error = UnknownHostException("no dns") + assertFalse(ErrorUtils.shouldReport(error, error)) + assertFalse(ErrorUtils.shouldReport(Fault(), error)) + } +} diff --git a/services/flipcash/src/main/kotlin/com/flipcash/services/models/Errors.kt b/services/flipcash/src/main/kotlin/com/flipcash/services/models/Errors.kt index e26f041035..a88786471e 100644 --- a/services/flipcash/src/main/kotlin/com/flipcash/services/models/Errors.kt +++ b/services/flipcash/src/main/kotlin/com/flipcash/services/models/Errors.kt @@ -4,6 +4,7 @@ import com.flipcash.services.models.chat.BlobRejection import com.getcode.solana.keys.Checksum import com.getcode.utils.CodeServerError import com.getcode.utils.NotifiableError +import com.getcode.utils.UnreportedError sealed class LoginError( override val message: String? = null, @@ -374,7 +375,12 @@ sealed class GetMessagesError( override val cause: Throwable? = null ): CodeServerError(message, cause) { class Denied : GetMessagesError("Denied") - class NotFound : GetMessagesError("Not found") + + /** + * The chat has no messages to return. Opening a conversation nobody has written in yet is a + * normal outcome, not a fault, so callers render an empty transcript and it is not reported. + */ + class NotFound : GetMessagesError("Not found"), UnreportedError class Unrecognized : GetMessagesError("Unrecognized"), NotifiableError data class Other(override val cause: Throwable? = null) : GetMessagesError(message = cause?.message, cause = cause), NotifiableError } diff --git a/services/flipcash/src/test/kotlin/com/flipcash/services/models/ErrorsTest.kt b/services/flipcash/src/test/kotlin/com/flipcash/services/models/ErrorsTest.kt index d7dc58b994..6cefb0a344 100644 --- a/services/flipcash/src/test/kotlin/com/flipcash/services/models/ErrorsTest.kt +++ b/services/flipcash/src/test/kotlin/com/flipcash/services/models/ErrorsTest.kt @@ -1,6 +1,7 @@ package com.flipcash.services.models import com.getcode.utils.CodeServerError +import com.getcode.utils.UnreportedError import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertIs @@ -147,6 +148,11 @@ class ErrorsTest { assertIs(GetMessagesError.Other()) } + @Test + fun `GetMessagesError NotFound is not reported`() { + assertIs(GetMessagesError.NotFound()) + } + // -- SendMessageError -- @Test