From 6201fe36886af1f93a4ff3b18047aa6b092d2611 Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 3 Jul 2024 14:12:18 +0100 Subject: [PATCH 01/23] chore(publish): correctly copy `build.gradle.kts` instead of the old `build.gradle` during publishing --- dockerfiles/Dockerfile.android-publisher | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockerfiles/Dockerfile.android-publisher b/dockerfiles/Dockerfile.android-publisher index f3bc0fabff..161b41da65 100644 --- a/dockerfiles/Dockerfile.android-publisher +++ b/dockerfiles/Dockerfile.android-publisher @@ -5,7 +5,7 @@ WORKDIR /app # Copy gradle files COPY gradlew gradle.properties /app/ COPY gradle/ /app/gradle/ -COPY build.gradle settings.gradle.kts /app/ +COPY build.gradle.kts settings.gradle.kts /app/ COPY buildSrc/ buildSrc/ # Copy sdk source files From 2cb636093e8b89e5e6dd048083e23af0eaf7b205 Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 5 Jul 2024 14:33:18 +0100 Subject: [PATCH 02/23] feat(breadcrumbs): include useful Activity.intent info in onCreate breadcrumbs --- CHANGELOG.md | 7 ++++ .../android/ActivityBreadcrumbCollector.kt | 40 +++++++++++++++---- features/smoke_tests/03_sessions.feature | 5 +++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83e0cf2fd5..6a6382c73b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Enhancements + +* Include additional Intent information for Activity.onCreate breadcrumbs (action, categories, type, flags, id, extra keys) + [#2057](https://github.com/bugsnag/bugsnag-android/pull/2057) + ## 6.6.1 (2024-07-03) ### Bug fixes diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt index 35330356d4..b97a2eb86a 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt @@ -2,6 +2,8 @@ package com.bugsnag.android import android.app.Activity import android.app.Application +import android.content.Intent +import android.os.Build import android.os.Bundle import java.util.WeakHashMap @@ -11,8 +13,16 @@ internal class ActivityBreadcrumbCollector( private val prevState = WeakHashMap() - override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) = - leaveBreadcrumb(activity, "onCreate()", savedInstanceState != null) + override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) { + leaveBreadcrumb( + activity, + "onCreate()", + mutableMapOf().apply { + set("hasBundle", savedInstanceState != null) + setActivityIntentMetadata(activity.intent) + } + ) + } override fun onActivityStarted(activity: Activity) = leaveBreadcrumb(activity, "onStart()") @@ -39,12 +49,8 @@ internal class ActivityBreadcrumbCollector( private fun leaveBreadcrumb( activity: Activity, lifecycleCallback: String, - hasBundle: Boolean? = null + metadata: MutableMap = mutableMapOf() ) { - val metadata = mutableMapOf() - if (hasBundle != null) { - metadata["hasBundle"] = hasBundle - } val previousVal = prevState[activity] if (previousVal != null) { @@ -55,4 +61,24 @@ internal class ActivityBreadcrumbCollector( cb("$activityName#$lifecycleCallback", metadata) prevState[activity] = lifecycleCallback } + + private fun MutableMap.setActivityIntentMetadata(intent: Intent?) { + if (intent == null) return + + intent.action?.let { set("action", it) } + intent.categories?.let { set("categories", it.joinToString(", ")) } + intent.type?.let { set("type", it) } + + if (intent.flags != 0) { + @Suppress("MagicNumber") // hex radix + set("flags", "0x${intent.flags.toString(16)}") + } + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + intent.identifier?.let { set("id", it) } + } + + set("hasData", intent.data != null) + set("hasExtras", intent.extras?.keySet()?.joinToString(", ") ?: false) + } } diff --git a/features/smoke_tests/03_sessions.feature b/features/smoke_tests/03_sessions.feature index 3033fed8bf..7d98cf16bd 100644 --- a/features/smoke_tests/03_sessions.feature +++ b/features/smoke_tests/03_sessions.feature @@ -57,6 +57,11 @@ Feature: Session functionality smoke tests And the event "session.events.unhandled" equals 0 And the event "severityReason.unhandledOverridden" is false + And the event has a "state" breadcrumb named "SecondActivity#onCreate()" + And the breadcrumb named "SecondActivity#onCreate()" has "metaData.action" equal to "com.bugsnag.android.mazerunner.UPDATE_CONTEXT" + And the breadcrumb named "SecondActivity#onCreate()" has "metaData.hasBundle" is false + And the breadcrumb named "SecondActivity#onCreate()" has "metaData.hasExtras" is false + @debug-safe Scenario: Manual session control works When I run "ManualSessionSmokeScenario" and relaunch the crashed app From db9e665369f53f7ff7ba5197d5a66960a936f31d Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 9 Jul 2024 11:57:28 +0100 Subject: [PATCH 03/23] fix(json): handle edge cases when deserializing threads with no state property --- CHANGELOG.md | 5 ++ .../com/bugsnag/android/BugsnagEventMapper.kt | 2 +- .../java/com/bugsnag/android/JsonUtils.kt | 20 ++++-- .../android/ThreadDeserializationTest.kt | 68 +++++++++++++++++++ .../resources/thread_deserialization_0.json | 5 ++ .../resources/thread_deserialization_1.json | 5 ++ .../resources/thread_deserialization_2.json | 6 ++ 7 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt create mode 100644 bugsnag-android-core/src/test/resources/thread_deserialization_0.json create mode 100644 bugsnag-android-core/src/test/resources/thread_deserialization_1.json create mode 100644 bugsnag-android-core/src/test/resources/thread_deserialization_2.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a6382c73b..5a52b4e907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ * Include additional Intent information for Activity.onCreate breadcrumbs (action, categories, type, flags, id, extra keys) [#2057](https://github.com/bugsnag/bugsnag-android/pull/2057) +### Bug fixes + +* Handle rare cases where we need to deserialize threads that don't have a valid `state` property + [#2058](https://github.com/bugsnag/bugsnag-android/pull/2058) + ## 6.6.1 (2024-07-03) ### Bug fixes diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt index e44902d423..b6113c8b58 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt @@ -195,7 +195,7 @@ internal class BugsnagEventMapper( thread.readEntry("name"), ErrorType.fromDescriptor(thread.readEntry("type")) ?: ErrorType.ANDROID, thread["errorReportingThread"] == true, - thread.readEntry("state"), + thread["state"] as? String ?: "", (thread["stacktrace"] as? List>)?.let { convertStacktrace(it) } ?: Stacktrace(emptyList()) ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/JsonUtils.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/JsonUtils.kt index 2de6290f24..420bb8d5cd 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/JsonUtils.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/JsonUtils.kt @@ -3,7 +3,6 @@ package com.bugsnag.android import com.bugsnag.android.internal.JsonHelper import org.junit.Assert import java.io.StringWriter -import java.lang.NullPointerException /** * Serializes a [JsonStream.Streamable] object into JSON and compares its equality against a JSON @@ -86,15 +85,26 @@ internal fun verifyJsonParser( } /** - * Generates parameterised test cases from a variable number of [JsonStream.Streamable] elements. + * Generates parameterised test cases from a variable number of elements. * The expected JSON file for each element should match the naming format * '$filename_serialization_$index.json' */ -internal fun generateSerializationTestCases( +internal fun generateSerializationTestCases(filename: String, vararg elements: T) = + generateJsonTestCases(elements, "${filename}_serialization_") + +/** + * Generates parameterised test cases from a variable number of elements. + * The expected JSON file for each element should match the naming format + * '$filename_serialization_$index.json' + */ +internal fun generateDeserializationTestCases(filename: String, vararg elements: T) = + generateJsonTestCases(elements, "${filename}_deserialization_") + +private fun generateJsonTestCases( + elements: Array, filename: String, - vararg elements: T ): Collection> { return elements.mapIndexed { index, obj -> - Pair(obj, "${filename}_serialization_$index.json") + Pair(obj, "${filename}$index.json") } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt new file mode 100644 index 0000000000..fb92014ef3 --- /dev/null +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt @@ -0,0 +1,68 @@ +package com.bugsnag.android + +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.junit.runners.Parameterized.Parameter +import org.junit.runners.Parameterized.Parameters + +@RunWith(Parameterized::class) +class ThreadDeserializationTest { + companion object { + @JvmStatic + @Parameters + fun testCases(): Collection> { + return generateDeserializationTestCases( + "thread", + Thread( + ThreadInternal( + "riker", + "will.riker", + ErrorType.C, + false, + "", + Stacktrace(emptyList()) + ), + NoopLogger + ), + Thread( + ThreadInternal( + "321", + "mayne", + ErrorType.ANDROID, + false, + "", + Stacktrace(emptyList()) + ), + NoopLogger + ), + Thread( + ThreadInternal( + "1415926535897932384626433832795028841971693993751058209749445923078" + + "164062862089986280348253421170679821480865132823066470938446095" + + "505822317253594081284811174502841027019385211055596446229489549" + + "303819644288109756659334461284756482337867831652712019091456485" + + "669234603486104543266482", + "smoke signal handler", + ErrorType.ANDROID, + false, + "happy", + Stacktrace(emptyList()) + ), + NoopLogger + ) + ) + } + } + + @Parameter + lateinit var testCase: Pair + + private val eventMapper = BugsnagEventMapper(NoopLogger) + + @Test + fun testJsonDeserialization() = + verifyJsonParser(testCase.first, testCase.second) { + eventMapper.convertThread(it) + } +} diff --git a/bugsnag-android-core/src/test/resources/thread_deserialization_0.json b/bugsnag-android-core/src/test/resources/thread_deserialization_0.json new file mode 100644 index 0000000000..6f4a9a5a6f --- /dev/null +++ b/bugsnag-android-core/src/test/resources/thread_deserialization_0.json @@ -0,0 +1,5 @@ +{ + "id": "riker", + "name": "will.riker", + "type": "c" +} \ No newline at end of file diff --git a/bugsnag-android-core/src/test/resources/thread_deserialization_1.json b/bugsnag-android-core/src/test/resources/thread_deserialization_1.json new file mode 100644 index 0000000000..85c029460c --- /dev/null +++ b/bugsnag-android-core/src/test/resources/thread_deserialization_1.json @@ -0,0 +1,5 @@ +{ + "id": 321, + "name": "mayne", + "type": "android" +} \ No newline at end of file diff --git a/bugsnag-android-core/src/test/resources/thread_deserialization_2.json b/bugsnag-android-core/src/test/resources/thread_deserialization_2.json new file mode 100644 index 0000000000..63a8d3342a --- /dev/null +++ b/bugsnag-android-core/src/test/resources/thread_deserialization_2.json @@ -0,0 +1,6 @@ +{ + "id": "1415926535897932384626433832795028841971693993751058209749445923078164062862089986280348253421170679821480865132823066470938446095505822317253594081284811174502841027019385211055596446229489549303819644288109756659334461284756482337867831652712019091456485669234603486104543266482", + "name": "smoke signal handler", + "type": "android", + "state": "happy" +} \ No newline at end of file From 6567ddfcd25e9fe268b40df03a397d837bb24c33 Mon Sep 17 00:00:00 2001 From: YingYing Chen <40571804+YYChen01988@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:17:13 +0100 Subject: [PATCH 04/23] chore(mazerunner): bump the version of AGP that the test fixture is built with (#2059) Co-authored-by: jason --- features/fixtures/mazerunner/app/build.gradle | 6 +++--- features/fixtures/mazerunner/app/detekt-baseline.xml | 2 -- .../fixtures/mazerunner/app/src/main/AndroidManifest.xml | 3 --- .../java/com/bugsnag/android/mazerunner/MainActivity.kt | 1 - .../java/com/bugsnag/android/mazerunner/NetworkStatus.kt | 8 ++++++-- features/fixtures/mazerunner/build.gradle | 6 +++--- .../mazerunner/gradle/wrapper/gradle-wrapper.properties | 2 +- .../jvm-scenarios/src/main/AndroidManifest.xml | 9 ++++++++- .../android/mazerunner/scenarios/JvmCrashLoopScenario.kt | 1 + .../com/bugsnag/android/mazerunner/scenarios/Scenario.kt | 8 ++++---- .../mazerunner/scenarios/SessionStoppingScenario.kt | 1 + .../bugsnag/android/multiprocess/MultiProcessService.kt | 2 +- .../com/bugsnag/android/multiprocess/ProcessCompat.kt | 2 +- features/smoke_tests/02_handled.feature | 2 +- features/smoke_tests/04_unhandled.feature | 2 +- 15 files changed, 31 insertions(+), 24 deletions(-) diff --git a/features/fixtures/mazerunner/app/build.gradle b/features/fixtures/mazerunner/app/build.gradle index 2534707394..49dea80520 100644 --- a/features/fixtures/mazerunner/app/build.gradle +++ b/features/fixtures/mazerunner/app/build.gradle @@ -4,12 +4,12 @@ apply plugin: "io.gitlab.arturbosch.detekt" apply plugin: "org.jlleitschuh.gradle.ktlint" android { - compileSdkVersion 31 + compileSdk 34 ndkVersion parent.ext.ndkVersion defaultConfig { minSdkVersion 17 - targetSdkVersion 33 + targetSdkVersion 34 versionCode 34 versionName "1.1.14" manifestPlaceholders = [ @@ -47,7 +47,7 @@ android { buildTypes { release { minifyEnabled true // obfuscation disabled to simplify maze - proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro" + proguardFiles getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro" signingConfig signingConfigs.release } } diff --git a/features/fixtures/mazerunner/app/detekt-baseline.xml b/features/fixtures/mazerunner/app/detekt-baseline.xml index c54e54cfd0..1699b5db65 100644 --- a/features/fixtures/mazerunner/app/detekt-baseline.xml +++ b/features/fixtures/mazerunner/app/detekt-baseline.xml @@ -2,9 +2,7 @@ - ComplexMethod:MainActivity.kt$MainActivity$// Starts a thread to poll for Maze Runner actions to perform private fun startCommandRunner() MagicNumber:MainActivity.kt$MainActivity$1000 MagicNumber:MainActivity.kt$MainActivity$250 - SwallowedException:NetworkStatus.kt$catch (e: Exception) { NetworkStatus.NO_INTERNET } diff --git a/features/fixtures/mazerunner/app/src/main/AndroidManifest.xml b/features/fixtures/mazerunner/app/src/main/AndroidManifest.xml index a152aba337..7cd4624af1 100644 --- a/features/fixtures/mazerunner/app/src/main/AndroidManifest.xml +++ b/features/fixtures/mazerunner/app/src/main/AndroidManifest.xml @@ -29,9 +29,6 @@ - diff --git a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt index 7b9e12d133..f3589239c0 100644 --- a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt +++ b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt @@ -298,7 +298,6 @@ class MainActivity : Activity() { sessionsUrl: String, notifyUrl: String ): Scenario { - val apiKeyField = findViewById(R.id.manualApiKey) val manualMode = apiKeyField.text.isNotEmpty() diff --git a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/NetworkStatus.kt b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/NetworkStatus.kt index 14c6290bf9..d221290003 100644 --- a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/NetworkStatus.kt +++ b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/NetworkStatus.kt @@ -23,13 +23,17 @@ val Context.networkStatus: NetworkStatus val capabilities = connectivityManager.getNetworkCapabilities(network) ?: return NetworkStatus.UNKNOWN_CAPABILITIES - return if (capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) NetworkStatus.CONNECTED - else NetworkStatus.NO_INTERNET + return if (capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) { + NetworkStatus.CONNECTED + } else { + NetworkStatus.NO_INTERNET + } } else { val networkInfo = connectivityManager.activeNetworkInfo ?: return NetworkStatus.NO_NETWORK if (networkInfo.isAvailable && networkInfo.isConnected) { + @Suppress("SwallowedException") return try { URL("https://www.google.com").readText() NetworkStatus.CONNECTED diff --git a/features/fixtures/mazerunner/build.gradle b/features/fixtures/mazerunner/build.gradle index 0076b5b5f7..dca8c57f85 100644 --- a/features/fixtures/mazerunner/build.gradle +++ b/features/fixtures/mazerunner/build.gradle @@ -23,14 +23,14 @@ buildscript { dependencies { def agpVersion = project.hasProperty("USE_AGP_VERSION") ? project.property("USE_AGP_VERSION") - : "8.3.2" + : "8.5.0" project.logger.lifecycle("Using AGP $agpVersion") classpath "com.android.tools.build:gradle:$agpVersion" classpath "com.bugsnag:bugsnag-android-gradle-plugin:8.1.0" classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" - classpath "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.18.1" - classpath "org.jlleitschuh.gradle:ktlint-gradle:10.2.0" + classpath "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.23.5" + classpath "org.jlleitschuh.gradle:ktlint-gradle:11.3.1" } } diff --git a/features/fixtures/mazerunner/gradle/wrapper/gradle-wrapper.properties b/features/fixtures/mazerunner/gradle/wrapper/gradle-wrapper.properties index e411586a54..48c0a02ca4 100644 --- a/features/fixtures/mazerunner/gradle/wrapper/gradle-wrapper.properties +++ b/features/fixtures/mazerunner/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/AndroidManifest.xml b/features/fixtures/mazerunner/jvm-scenarios/src/main/AndroidManifest.xml index 8072ee00db..5b2e718ef4 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/AndroidManifest.xml +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/AndroidManifest.xml @@ -1,2 +1,9 @@ - + + + + + + diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/JvmCrashLoopScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/JvmCrashLoopScenario.kt index c1a7e0d9ff..9c221a3530 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/JvmCrashLoopScenario.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/JvmCrashLoopScenario.kt @@ -31,6 +31,7 @@ internal class JvmCrashLoopScenario( ) } + @Suppress("UseCheckOrError") override fun startScenario() { super.startScenario() val lastRunInfo = Bugsnag.getLastRunInfo() diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt index eddb4e51f8..597183b196 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt @@ -19,8 +19,8 @@ import com.bugsnag.android.Configuration import com.bugsnag.android.mazerunner.BugsnagIntentParams import com.bugsnag.android.mazerunner.MazerunnerHttpClient import com.bugsnag.android.mazerunner.log -import com.bugsnag.android.mazerunner.multiprocess.MultiProcessService -import com.bugsnag.android.mazerunner.multiprocess.findCurrentProcessName +import com.bugsnag.android.multiprocess.MultiProcessService +import com.bugsnag.android.multiprocess.findCurrentProcessName import com.bugsnag.android.performance.measureSpan import java.io.File import kotlin.system.measureNanoTime @@ -119,7 +119,6 @@ abstract class Scenario( context.registerReceiver( object : BroadcastReceiver() { override fun onReceive(context: Context?, intent: Intent?) { - // explicitly post on the main thread to avoid // the broadcast receiver wrapping exceptions Handler(Looper.getMainLooper()).post { @@ -186,7 +185,8 @@ abstract class Scenario( Unit override fun onLowMemory() = Unit - }) + } + ) } override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/SessionStoppingScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/SessionStoppingScenario.kt index 7b57d61bb6..2d0b4c9767 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/SessionStoppingScenario.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/SessionStoppingScenario.kt @@ -87,6 +87,7 @@ internal class SessionStoppingScenario( } }, END { + @Suppress("UseCheckOrError") override fun performAction(): ScenarioState = throw IllegalStateException("One too many delivery attempts") }; diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt index 2aed67d90a..f75377ab91 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt @@ -1,4 +1,4 @@ -package com.bugsnag.android.mazerunner.multiprocess +package com.bugsnag.android.multiprocess import android.app.Service import android.content.Intent diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/ProcessCompat.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/ProcessCompat.kt index 564a650070..83d1f5b334 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/ProcessCompat.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/ProcessCompat.kt @@ -1,4 +1,4 @@ -package com.bugsnag.android.mazerunner.multiprocess +package com.bugsnag.android.multiprocess import android.annotation.SuppressLint import android.app.Application diff --git a/features/smoke_tests/02_handled.feature b/features/smoke_tests/02_handled.feature index 5f2d5d7a51..417f1139f4 100644 --- a/features/smoke_tests/02_handled.feature +++ b/features/smoke_tests/02_handled.feature @@ -24,7 +24,7 @@ Feature: Handled smoke tests And the event "exceptions.0.stacktrace.0.method" ends with "HandledJavaSmokeScenario.startScenario" And the exception "stacktrace.0.file" equals "SourceFile" # R8 minification alters the lineNumber, see the mapping file/source code for the original value - And the event "exceptions.0.stacktrace.0.lineNumber" equals 56 + And the event "exceptions.0.stacktrace.0.lineNumber" equals 86 And the event "exceptions.0.stacktrace.0.inProject" is true And the error payload field "events.0.projectPackages" is a non-empty array And the event "projectPackages.0" equals "com.bugsnag.android.mazerunner" diff --git a/features/smoke_tests/04_unhandled.feature b/features/smoke_tests/04_unhandled.feature index d25c411172..6eb54cffb8 100644 --- a/features/smoke_tests/04_unhandled.feature +++ b/features/smoke_tests/04_unhandled.feature @@ -27,7 +27,7 @@ Feature: Unhandled smoke tests And the event "exceptions.0.stacktrace.0.method" ends with "UnhandledJavaLoadedConfigScenario.startScenario" And the exception "stacktrace.0.file" equals "SourceFile" # R8 minification alters the lineNumber, see the mapping file/source code for the original value - And the event "exceptions.0.stacktrace.0.lineNumber" equals 41 + And the event "exceptions.0.stacktrace.0.lineNumber" equals 42 And the event "exceptions.0.stacktrace.0.inProject" is true And the thread with name "main" contains the error reporting flag From 486e8f33fbd3f7ce66fad68f7b8fca5cf3ed020c Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 2 Aug 2024 12:42:30 +0100 Subject: [PATCH 05/23] fix(e2e tests): fixed the multi process scenario by using the correct `registerReceiver` methods --- .../android/mazerunner/scenarios/Scenario.kt | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt index 597183b196..3e2f13b88a 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt @@ -1,5 +1,6 @@ package com.bugsnag.android.mazerunner.scenarios +import android.annotation.SuppressLint import android.app.Activity import android.app.Application import android.content.BroadcastReceiver @@ -25,6 +26,8 @@ import com.bugsnag.android.performance.measureSpan import java.io.File import kotlin.system.measureNanoTime +private const val RECEIVER_EXPORTED = 2 + abstract class Scenario( protected val config: Configuration, protected val context: Context, @@ -116,21 +119,27 @@ abstract class Scenario( callback: () -> Unit = {} ) { val filter = IntentFilter(MultiProcessService.ACTION_LAUNCHED_MULTI_PROCESS) - context.registerReceiver( - object : BroadcastReceiver() { - override fun onReceive(context: Context?, intent: Intent?) { - // explicitly post on the main thread to avoid - // the broadcast receiver wrapping exceptions - Handler(Looper.getMainLooper()).post { - callback() - } + val receiver = object : BroadcastReceiver() { + override fun onReceive(context: Context?, intent: Intent?) { + log("Received '${MultiProcessService.ACTION_LAUNCHED_MULTI_PROCESS}' broadcast") + // explicitly post on the main thread to avoid + // the broadcast receiver wrapping exceptions + Handler(Looper.getMainLooper()).post { + callback() } - }, - filter - ) + } + } + + @SuppressLint("WrongConstant") + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + context.registerReceiver(receiver, filter, RECEIVER_EXPORTED) + } else { + context.registerReceiver(receiver, filter) + } val intent = Intent(context, MultiProcessService::class.java) params.encode(intent) + log("Starting MultiProcessService") context.startService(intent) } From 9247e9934f738fd808f0a5571621df01fbff55bd Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 2 Aug 2024 10:21:35 +0100 Subject: [PATCH 06/23] fix(internal_metrics): limit strcmp/strlen calls when updating callback counts --- .../src/main/jni/internal_metrics.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c b/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c index d30ab63100..ec7294b659 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c @@ -140,7 +140,8 @@ static void bsg_modify_callback_count(bugsnag_event *event, const char *api, for (; i < total_callbacks && event->set_callback_counts[i].name[0] != 0; i++) { set_callback_count *callback_counter = &event->set_callback_counts[i]; - if (strcmp(callback_counter->name, api) == 0) { + if (strncmp(callback_counter->name, api, sizeof(callback_counter->name)) == + 0) { callback_counter->count += delta; if (callback_counter->count < 0) { callback_counter->count = 0; @@ -157,13 +158,14 @@ static void bsg_modify_callback_count(bugsnag_event *event, const char *api, void bsg_set_callback_count(bugsnag_event *event, const char *api, int32_t count) { - if (!internal_metrics_enabled || event == NULL) { + if (!internal_metrics_enabled || event == NULL || !api) { return; } static const int total_callbacks = sizeof(event->set_callback_counts) / sizeof(*event->set_callback_counts); - if (strlen(api) >= sizeof(event->set_callback_counts[0].name)) { + if (strnlen(api, sizeof(event->set_callback_counts[0].name)) >= + sizeof(event->set_callback_counts[0].name)) { // API name is too big to store. return; } From 2ea2bd100628f1a4778338ec548c333c0d98d9b9 Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 2 Aug 2024 10:22:30 +0100 Subject: [PATCH 07/23] fix(ndk): check the bsg_environment strictly after locking (for write functions) and before using to avoid de-ref failures later on --- CHANGELOG.md | 2 + .../src/main/jni/bugsnag_ndk.c | 314 ++++++++++-------- 2 files changed, 180 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a52b4e907..cd0e4e18c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ * Handle rare cases where we need to deserialize threads that don't have a valid `state` property [#2058](https://github.com/bugsnag/bugsnag-android/pull/2058) +* Avoid racing ourselves in the `bugsnag-plugin-android-ndk` during multi-threaded startups + [#2064](https://github.com/bugsnag/bugsnag-android/pull/2064) ## 6.6.1 (2024-07-03) diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c index 13278dfceb..bce44c7dfd 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c @@ -27,10 +27,19 @@ static pthread_mutex_t bsg_global_env_write_mutex = PTHREAD_MUTEX_INITIALIZER; /** * All functions which will edit the environment (unless they are handling a - * crash) must first request the lock + * crash) must first request the lock. Returns the bsg_environment that should + * be used, may return NULL if there is no valid bsg_environment (no lock + * will be held if this returns NULL) */ -static void request_env_write_lock(void) { +static bsg_environment *request_env_write_lock(void) { pthread_mutex_lock(&bsg_global_env_write_mutex); + bsg_environment *local_env = bsg_global_env; + if (local_env != NULL) { + return local_env; + } else { + pthread_mutex_unlock(&bsg_global_env_write_mutex); + return NULL; + } } /** @@ -244,11 +253,11 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addHandledEvent(JNIEnv *env, jobject _this) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + bugsnag_event *event = &bsg_env->next_event; if (bsg_event_has_session(event)) { event->handled_events++; @@ -259,11 +268,11 @@ Java_com_bugsnag_android_ndk_NativeBridge_addHandledEvent(JNIEnv *env, JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addUnhandledEvent(JNIEnv *env, jobject _this) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + bugsnag_event *event = &bsg_env->next_event; if (bsg_event_has_session(event)) { event->unhandled_events++; @@ -274,14 +283,17 @@ Java_com_bugsnag_android_ndk_NativeBridge_addUnhandledEvent(JNIEnv *env, JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_startedSession( JNIEnv *env, jobject _this, jstring session_id_, jstring start_date_, jint handled_count, jint unhandled_count) { - if (bsg_global_env == NULL || session_id_ == NULL) { + if (session_id_ == NULL) { + return; + } + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } char *session_id = (char *)bsg_safe_get_string_utf_chars(env, session_id_); char *started_at = (char *)bsg_safe_get_string_utf_chars(env, start_date_); if (session_id != NULL && started_at != NULL) { - request_env_write_lock(); - bsg_event_start_session(&bsg_global_env->next_event, session_id, started_at, + bsg_event_start_session(&bsg_env->next_event, session_id, started_at, handled_count, unhandled_count); release_env_write_lock(); } @@ -291,11 +303,11 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_startedSession( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_pausedSession( JNIEnv *env, jobject _this) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + bugsnag_event *event = &bsg_env->next_event; memset(event->session_id, 0, bsg_strlen(event->session_id)); memset(event->session_start, 0, bsg_strlen(event->session_start)); event->handled_events = 0; @@ -351,10 +363,14 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb( } bsg_populate_crumb_metadata(env, crumb, metadata); - request_env_write_lock(); - bsg_event_add_breadcrumb(&bsg_global_env->next_event, crumb); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_event_add_breadcrumb(&bsg_env->next_event, crumb); release_env_write_lock(); + end: free(crumb); } bsg_safe_release_string_utf_chars(env, name_, name); @@ -365,16 +381,17 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateAppVersion(JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_app_set_version(&bsg_global_env->next_event, value); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_app_set_version(&bsg_env->next_event, value); release_env_write_lock(); +end: bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -382,30 +399,31 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateBuildUUID(JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_app_set_build_uuid(&bsg_global_env->next_event, value); - release_env_write_lock(); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_app_set_build_uuid(&bsg_env->next_event, value); +end: bsg_safe_release_string_utf_chars(env, new_value, value); } JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateContext( JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_event_set_context(&bsg_global_env->next_event, value); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_set_context(&bsg_env->next_event, value); +end: release_env_write_lock(); if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); @@ -415,22 +433,22 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateContext( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateInForeground( JNIEnv *env, jobject _this, jboolean new_value, jstring activity_) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } char *activity = (char *)bsg_safe_get_string_utf_chars(env, activity_); - request_env_write_lock(); - bool was_in_foreground = bsg_global_env->next_event.app.in_foreground; - bsg_global_env->next_event.app.in_foreground = (bool)new_value; - bsg_strncpy(bsg_global_env->next_event.app.active_screen, activity, - sizeof(bsg_global_env->next_event.app.active_screen)); + bool was_in_foreground = bsg_env->next_event.app.in_foreground; + bsg_env->next_event.app.in_foreground = (bool)new_value; + bsg_strncpy(bsg_env->next_event.app.active_screen, activity, + sizeof(bsg_env->next_event.app.active_screen)); if ((bool)new_value) { if (!was_in_foreground) { - time(&bsg_global_env->foreground_start_time); + time(&bsg_env->foreground_start_time); } } else { - bsg_global_env->foreground_start_time = 0; - bsg_global_env->next_event.app.duration_in_foreground_ms_offset = 0; + bsg_env->foreground_start_time = 0; + bsg_env->next_event.app.duration_in_foreground_ms_offset = 0; } release_env_write_lock(); if (activity_ != NULL) { @@ -441,12 +459,12 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateInForeground( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateIsLaunching( JNIEnv *env, jobject _this, jboolean new_value) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - request_env_write_lock(); - bugsnag_app_set_is_launching(&bsg_global_env->next_event, new_value); - bsg_update_next_run_info(bsg_global_env); + bugsnag_app_set_is_launching(&bsg_env->next_event, new_value); + bsg_update_next_run_info(bsg_env); release_env_write_lock(); } @@ -454,9 +472,6 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateLowMemory( JNIEnv *env, jobject _this, jboolean low_memory, jstring memory_trim_level_description) { - if (bsg_global_env == NULL) { - return; - } char *memory_trim_level = (char *)bsg_safe_get_string_utf_chars(env, memory_trim_level_description); @@ -465,12 +480,16 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateLowMemory( return; } - request_env_write_lock(); - bugsnag_event_add_metadata_bool(&bsg_global_env->next_event, "app", - "lowMemory", (bool)low_memory); - bugsnag_event_add_metadata_string(&bsg_global_env->next_event, "app", + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_add_metadata_bool(&bsg_env->next_event, "app", "lowMemory", + (bool)low_memory); + bugsnag_event_add_metadata_string(&bsg_env->next_event, "app", "memoryTrimLevel", memory_trim_level); release_env_write_lock(); +end: if (memory_trim_level_description != NULL) { bsg_safe_release_string_utf_chars(env, memory_trim_level_description, memory_trim_level); @@ -481,17 +500,18 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateOrientation(JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } - char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_device_set_orientation(&bsg_global_env->next_event, value); + + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_device_set_orientation(&bsg_env->next_event, value); release_env_write_lock(); +end: if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -500,16 +520,17 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateOrientation(JNIEnv *env, JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateReleaseStage( JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_app_set_release_stage(&bsg_global_env->next_event, value); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_app_set_release_stage(&bsg_env->next_event, value); release_env_write_lock(); +end: if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -517,18 +538,20 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateReleaseStage( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateUserId( JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event *event = &bsg_env->next_event; bugsnag_user user = bugsnag_event_get_user(event); bugsnag_event_set_user(event, value, user.email, user.name); release_env_write_lock(); +end: if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -536,18 +559,20 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateUserId( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateUserName( JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event *event = &bsg_env->next_event; bugsnag_user user = bugsnag_event_get_user(event); bugsnag_event_set_user(event, user.id, user.email, value); release_env_write_lock(); +end: if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -557,18 +582,20 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateUserEmail(JNIEnv *env, jobject _this, jstring new_value) { - if (bsg_global_env == NULL) { - return; - } char *value = (char *)bsg_safe_get_string_utf_chars(env, new_value); if (value == NULL) { return; } - request_env_write_lock(); - bugsnag_event *event = &bsg_global_env->next_event; + + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event *event = &bsg_env->next_event; bugsnag_user user = bugsnag_event_get_user(event); bugsnag_event_set_user(event, user.id, value, user.name); release_env_write_lock(); +end: if (new_value != NULL) { bsg_safe_release_string_utf_chars(env, new_value, value); } @@ -577,19 +604,19 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateUserEmail(JNIEnv *env, JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addMetadataString( JNIEnv *env, jobject _this, jstring tab_, jstring key_, jstring value_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); char *key = (char *)bsg_safe_get_string_utf_chars(env, key_); char *value = (char *)bsg_safe_get_string_utf_chars(env, value_); if (tab != NULL && key != NULL && value != NULL) { - request_env_write_lock(); - bugsnag_event_add_metadata_string(&bsg_global_env->next_event, tab, key, - value); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_add_metadata_string(&bsg_env->next_event, tab, key, value); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, tab_, tab); bsg_safe_release_string_utf_chars(env, key_, key); bsg_safe_release_string_utf_chars(env, value_, value); @@ -598,17 +625,18 @@ Java_com_bugsnag_android_ndk_NativeBridge_addMetadataString( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addMetadataDouble( JNIEnv *env, jobject _this, jstring tab_, jstring key_, jdouble value_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); char *key = (char *)bsg_safe_get_string_utf_chars(env, key_); if (tab != NULL && key != NULL) { - request_env_write_lock(); - bugsnag_event_add_metadata_double(&bsg_global_env->next_event, tab, key, + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_add_metadata_double(&bsg_env->next_event, tab, key, (double)value_); } release_env_write_lock(); +end: bsg_safe_release_string_utf_chars(env, tab_, tab); bsg_safe_release_string_utf_chars(env, key_, key); } @@ -616,17 +644,18 @@ Java_com_bugsnag_android_ndk_NativeBridge_addMetadataDouble( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addMetadataBoolean( JNIEnv *env, jobject _this, jstring tab_, jstring key_, jboolean value_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); char *key = (char *)bsg_safe_get_string_utf_chars(env, key_); if (tab != NULL && key != NULL) { - request_env_write_lock(); - bugsnag_event_add_metadata_bool(&bsg_global_env->next_event, tab, key, + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_add_metadata_bool(&bsg_env->next_event, tab, key, (bool)value_); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, tab_, tab); bsg_safe_release_string_utf_chars(env, key_, key); } @@ -634,18 +663,19 @@ Java_com_bugsnag_android_ndk_NativeBridge_addMetadataBoolean( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addMetadataOpaque( JNIEnv *env, jobject _this, jstring tab_, jstring key_, jstring value_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); char *key = (char *)bsg_safe_get_string_utf_chars(env, key_); char *value = (char *)bsg_safe_get_string_utf_chars(env, value_); if (tab != NULL && key != NULL) { - request_env_write_lock(); - bsg_add_metadata_value_opaque(&bsg_global_env->next_event.metadata, tab, - key, value); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_add_metadata_value_opaque(&bsg_env->next_event.metadata, tab, key, + value); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, tab_, tab); bsg_safe_release_string_utf_chars(env, key_, key); bsg_safe_release_string_utf_chars(env, value_, value); @@ -655,49 +685,51 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_clearMetadataTab(JNIEnv *env, jobject _this, jstring tab_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); if (tab == NULL) { return; } - request_env_write_lock(); - bugsnag_event_clear_metadata_section(&bsg_global_env->next_event, tab); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_clear_metadata_section(&bsg_env->next_event, tab); release_env_write_lock(); +end: bsg_safe_release_string_utf_chars(env, tab_, tab); } JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_removeMetadata( JNIEnv *env, jobject _this, jstring tab_, jstring key_) { - if (bsg_global_env == NULL) { - return; - } char *tab = (char *)bsg_safe_get_string_utf_chars(env, tab_); char *key = (char *)bsg_safe_get_string_utf_chars(env, key_); if (tab != NULL && key != NULL) { - request_env_write_lock(); - bugsnag_event_clear_metadata(&bsg_global_env->next_event, tab, key); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bugsnag_event_clear_metadata(&bsg_env->next_event, tab, key); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, tab_, tab); bsg_safe_release_string_utf_chars(env, key_, key); } JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateMetadata( JNIEnv *env, jobject _this, jobject metadata) { - if (bsg_global_env == NULL) { + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("updateMetadata failed: JNI cache not initialized."); return; } - if (!bsg_jni_cache->initialized) { - BUGSNAG_LOG("updateMetadata failed: JNI cache not initialized."); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - request_env_write_lock(); - bsg_populate_metadata(env, &bsg_global_env->next_event.metadata, metadata); + bsg_populate_metadata(env, &bsg_env->next_event.metadata, metadata); release_env_write_lock(); } @@ -710,19 +742,19 @@ Java_com_bugsnag_android_ndk_NativeBridge_getSignalUnwindStackFunction( JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addFeatureFlag( JNIEnv *env, jobject thiz, jstring name_, jstring variant_) { - if (bsg_global_env == NULL) { - return; - } - char *name = (char *)bsg_safe_get_string_utf_chars(env, name_); char *variant = (char *)bsg_safe_get_string_utf_chars(env, variant_); if (name != NULL) { - request_env_write_lock(); - bsg_set_feature_flag(&bsg_global_env->next_event, name, variant); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_set_feature_flag(&bsg_env->next_event, name, variant); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, name_, name); bsg_safe_release_string_utf_chars(env, variant_, variant); } @@ -732,30 +764,29 @@ Java_com_bugsnag_android_ndk_NativeBridge_clearFeatureFlag(JNIEnv *env, jobject thiz, jstring name_) { - if (bsg_global_env == NULL) { - return; - } - char *name = (char *)bsg_safe_get_string_utf_chars(env, name_); if (name != NULL) { - request_env_write_lock(); - bsg_clear_feature_flag(&bsg_global_env->next_event, name); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_clear_feature_flag(&bsg_env->next_event, name); release_env_write_lock(); } +end: bsg_safe_release_string_utf_chars(env, name_, name); } JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_clearFeatureFlags(JNIEnv *env, jobject thiz) { - if (bsg_global_env == NULL) { + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { return; } - - request_env_write_lock(); - bsg_free_feature_flags(&bsg_global_env->next_event); + bsg_free_feature_flags(&bsg_env->next_event); release_env_write_lock(); } @@ -769,10 +800,9 @@ JNIEXPORT jobject JNICALL Java_com_bugsnag_android_ndk_NativeBridge_getCurrentCallbackSetCounts( JNIEnv *env, jobject thiz) { - if (bsg_global_env == NULL) { + if (bsg_global_env == NULL || bsg_jni_cache == NULL) { return NULL; } - static const int total_callbacks = sizeof(bsg_global_env->next_event.set_callback_counts) / sizeof(*bsg_global_env->next_event.set_callback_counts); @@ -803,7 +833,7 @@ Java_com_bugsnag_android_ndk_NativeBridge_getCurrentCallbackSetCounts( JNIEXPORT jobject JNICALL Java_com_bugsnag_android_ndk_NativeBridge_getCurrentNativeApiCallUsage( JNIEnv *env, jobject thiz) { - if (bsg_global_env == NULL) { + if (bsg_global_env == NULL || bsg_jni_cache == NULL) { return NULL; } @@ -895,7 +925,13 @@ Java_com_bugsnag_android_ndk_NativeBridge_notifyAddCallback(JNIEnv *env, return; } - bsg_notify_add_callback(&bsg_global_env->next_event, callback); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_notify_add_callback(&bsg_env->next_event, callback); + release_env_write_lock(); +end: bsg_safe_release_string_utf_chars(env, callback_, callback); } @@ -907,7 +943,13 @@ Java_com_bugsnag_android_ndk_NativeBridge_notifyRemoveCallback( return; } - bsg_notify_remove_callback(&bsg_global_env->next_event, callback); + bsg_environment *bsg_env = request_env_write_lock(); + if (bsg_env == NULL) { + goto end; + } + bsg_notify_remove_callback(&bsg_env->next_event, callback); + release_env_write_lock(); +end: bsg_safe_release_string_utf_chars(env, callback_, callback); } From 4ab60366ec5840bae1622a76772cda6d74327499 Mon Sep 17 00:00:00 2001 From: YYChen01988 Date: Tue, 16 Jul 2024 09:31:48 +0100 Subject: [PATCH 08/23] feat(Event): open event add error --- .../main/java/com/bugsnag/android/Error.java | 5 +++ .../java/com/bugsnag/android/ErrorInternal.kt | 13 ++++++- .../main/java/com/bugsnag/android/Event.java | 36 +++++++++++++++---- .../java/com/bugsnag/android/EventInternal.kt | 20 +++++++++++ 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java index 8af63d17da..9447a89f90 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java @@ -98,4 +98,9 @@ static List createError(@NonNull Throwable exc, @NonNull Logger logger) { return ErrorInternal.Companion.createError(exc, projectPackages, logger); } + + + public void addStackframe(@NonNull List element) { + impl.addStackTrace(element); + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt index 6b247dd9ca..c6b76e8b48 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt @@ -10,7 +10,11 @@ internal class ErrorInternal @JvmOverloads internal constructor( val stacktrace: List = stacktrace.trace internal companion object { - fun createError(exc: Throwable, projectPackages: Collection, logger: Logger): MutableList { + fun createError( + exc: Throwable, + projectPackages: Collection, + logger: Logger + ): MutableList { return exc.safeUnrollCauses() .mapTo(mutableListOf()) { currentEx -> // Somehow it's possible for stackTrace to be null in rare cases @@ -32,4 +36,11 @@ internal class ErrorInternal @JvmOverloads internal constructor( writer.name("stacktrace").value(stacktrace) writer.endObject() } + + fun addStackTrace(element: List) { + val stackFrame = element.flatMap { + listOf(Stackframe(it.methodName, it.fileName, it.lineNumber, null)) + } + stacktrace.toMutableList().addAll(stackFrame) + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index 4dc9b31e4f..79a178c7ec 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -52,7 +52,7 @@ private void logNull(String property) { /** * The Throwable object that caused the event in your application. - * + *

* Manipulating this field does not affect the error information reported to the * Bugsnag dashboard. Use {@link Event#getErrors()} to access and amend the representation of * the error that will be sent. @@ -66,7 +66,7 @@ public Throwable getOriginalError() { * Information extracted from the {@link Throwable} that caused the event can be found in this * field. The list contains at least one {@link Error} that represents the thrown object * with subsequent elements in the list populated from {@link Throwable#getCause()}. - * + *

* A reference to the actual {@link Throwable} object that caused the event is available * through {@link Event#getOriginalError()} ()}. */ @@ -167,7 +167,7 @@ public Severity getSeverity() { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - * + *

* As the name implies, this option accepts a hash of sorts. */ public void setGroupingHash(@Nullable String groupingHash) { @@ -179,7 +179,7 @@ public void setGroupingHash(@Nullable String groupingHash) { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - * + *

* As the name implies, this option accepts a hash of sorts. */ @Nullable @@ -362,7 +362,7 @@ public void toStream(@NonNull JsonStream stream) throws IOException { /** * Whether the event was a crash (i.e. unhandled) or handled error in which the system * continued running. - * + *

* Unhandled errors count towards your stability score. If you don't want certain errors * to count towards your stability score, you can alter this property through an * {@link OnErrorCallback}. @@ -374,7 +374,7 @@ public boolean isUnhandled() { /** * Whether the event was a crash (i.e. unhandled) or handled error in which the system * continued running. - * + *

* Unhandled errors count towards your stability score. If you don't want certain errors * to count towards your stability score, you can alter this property through an * {@link OnErrorCallback}. @@ -388,7 +388,7 @@ public void setUnhandled(boolean unhandled) { * using bugsnag-android-performance, but can also be set manually if required. * * @param traceId the ID of the trace the event occurred within - * @param spanId the ID of the span that the event occurred within + * @param spanId the ID of the span that the event occurred within */ public void setTraceCorrelation(@NonNull UUID traceId, long spanId) { if (traceId != null) { @@ -442,4 +442,26 @@ void setRedactedKeys(Collection redactedKeys) { void setInternalMetrics(InternalMetrics metrics) { impl.setInternalMetrics(metrics); } + + public Error addError(Throwable error) { + if (error == null) { + return null; + } + return impl.addError(error); + } + + public Error addError(String errorClass, String errorMessage) { + return impl.addError(errorClass, errorMessage, ErrorType.ANDROID); + } + + + public Error addError(String errorClass, String errorMessage, ErrorType errorType) { + return impl.addError(errorClass, errorMessage, errorType); + } + + public Thread addThread(Thread thread) { + return impl.addThread(thread); + } + + } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt index 88b46df334..6842b738b3 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt @@ -324,4 +324,24 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata override fun clearFeatureFlag(name: String) = featureFlags.clearFeatureFlag(name) override fun clearFeatureFlags() = featureFlags.clearFeatureFlags() + + fun addError(throwError: Throwable): Error? { + val error = Error.createError(throwError, projectPackages, logger) + errors.addAll(error) + return error.firstOrNull() + } + + fun addError(errorClass: String, errorMessage: String, errorType: ErrorType): Error { + val error = Error( + ErrorInternal(errorClass, errorMessage, Stacktrace(ArrayList()), errorType), + logger + ) + errors.add(error) + return error + } + + fun addThread(thread: Thread): Thread { + threads.add(thread) + return thread + } } From 5318a3af66f3057d3b11e9aaa9d240c6b4735d38 Mon Sep 17 00:00:00 2001 From: YYChen01988 Date: Tue, 16 Jul 2024 14:10:33 +0100 Subject: [PATCH 09/23] feat(Event): open event add thread and breadCrumb --- .../api/bugsnag-android-core.api | 8 +++ .../main/java/com/bugsnag/android/Error.java | 23 +++++++- .../java/com/bugsnag/android/ErrorInternal.kt | 21 +++++-- .../main/java/com/bugsnag/android/Event.java | 44 ++++++++++---- .../java/com/bugsnag/android/EventInternal.kt | 47 +++++++++++++-- .../java/com/bugsnag/android/Stacktrace.kt | 58 +++++++++---------- 6 files changed, 147 insertions(+), 54 deletions(-) diff --git a/bugsnag-android-core/api/bugsnag-android-core.api b/bugsnag-android-core/api/bugsnag-android-core.api index 04f73e66b5..296e5bb112 100644 --- a/bugsnag-android-core/api/bugsnag-android-core.api +++ b/bugsnag-android-core/api/bugsnag-android-core.api @@ -312,6 +312,8 @@ public final class com/bugsnag/android/EndpointConfiguration { } public class com/bugsnag/android/Error : com/bugsnag/android/JsonStream$Streamable { + public fun addStackframe (Ljava/lang/StackTraceElement;Ljava/util/Collection;)Lcom/bugsnag/android/Stackframe; + public fun addStackframe (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Number;)Lcom/bugsnag/android/Stackframe; public fun getErrorClass ()Ljava/lang/String; public fun getErrorMessage ()Ljava/lang/String; public fun getStacktrace ()Ljava/util/List; @@ -350,11 +352,15 @@ public final class com/bugsnag/android/ErrorTypes { } public class com/bugsnag/android/Event : com/bugsnag/android/FeatureFlagAware, com/bugsnag/android/JsonStream$Streamable, com/bugsnag/android/MetadataAware, com/bugsnag/android/UserAware { + public fun addError (Ljava/lang/String;Ljava/lang/String;)Lcom/bugsnag/android/Error; + public fun addError (Ljava/lang/String;Ljava/lang/String;Lcom/bugsnag/android/ErrorType;)Lcom/bugsnag/android/Error; + public fun addError (Ljava/lang/Throwable;)Lcom/bugsnag/android/Error; public fun addFeatureFlag (Ljava/lang/String;)V public fun addFeatureFlag (Ljava/lang/String;Ljava/lang/String;)V public fun addFeatureFlags (Ljava/lang/Iterable;)V public fun addMetadata (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;)V public fun addMetadata (Ljava/lang/String;Ljava/util/Map;)V + public fun addThread (Ljava/lang/String;Ljava/lang/String;)Lcom/bugsnag/android/Thread; public fun clearFeatureFlag (Ljava/lang/String;)V public fun clearFeatureFlags ()V public fun clearMetadata (Ljava/lang/String;)V @@ -374,6 +380,8 @@ public class com/bugsnag/android/Event : com/bugsnag/android/FeatureFlagAware, c public fun getThreads ()Ljava/util/List; public fun getUser ()Lcom/bugsnag/android/User; public fun isUnhandled ()Z + public fun leaveBreadcrumb (Ljava/lang/String;)Lcom/bugsnag/android/Breadcrumb; + public fun leaveBreadcrumb (Ljava/lang/String;Lcom/bugsnag/android/BreadcrumbType;Ljava/util/Map;)Lcom/bugsnag/android/Breadcrumb; public fun setApiKey (Ljava/lang/String;)V public fun setContext (Ljava/lang/String;)V public fun setGroupingHash (Ljava/lang/String;)V diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java index 9447a89f90..24a6412974 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java @@ -99,8 +99,25 @@ static List createError(@NonNull Throwable exc, return ErrorInternal.Companion.createError(exc, projectPackages, logger); } + /** + * A stackframe can be added to an error in an event + */ + + @Nullable + public Stackframe addStackframe(@Nullable StackTraceElement element, + @NonNull Collection projectPackages) { + if (element != null) { + return impl.addStackframe(element, projectPackages, logger); + } else { + logNull("stackframe"); + } + return null; + } - public void addStackframe(@NonNull List element) { - impl.addStackTrace(element); + @NonNull + public Stackframe addStackframe(@Nullable String method, + @Nullable String file, + @Nullable Number lineNumber) { + return impl.addStackframe(method, file, lineNumber); } -} +} \ No newline at end of file diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt index c6b76e8b48..22f50b2a58 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt @@ -7,7 +7,7 @@ internal class ErrorInternal @JvmOverloads internal constructor( var type: ErrorType = ErrorType.ANDROID ) : JsonStream.Streamable { - val stacktrace: List = stacktrace.trace + val stacktrace: MutableList = stacktrace.trace internal companion object { fun createError( @@ -37,10 +37,19 @@ internal class ErrorInternal @JvmOverloads internal constructor( writer.endObject() } - fun addStackTrace(element: List) { - val stackFrame = element.flatMap { - listOf(Stackframe(it.methodName, it.fileName, it.lineNumber, null)) - } - stacktrace.toMutableList().addAll(stackFrame) + fun addStackframe( + element: StackTraceElement, + projectPackages: Collection, + logger: Logger + ): Stackframe? { + val frame = Stacktrace.serializeStackframe(element, projectPackages, logger) + frame?.let { stacktrace.add(it) } + return frame + } + + fun addStackframe(method: String?, file: String?, lineNumber: Number?): Stackframe { + val frame = Stackframe(method, file, lineNumber, null) + stacktrace.add(frame) + return frame } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index 79a178c7ec..1f4fd23dc2 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.util.Collection; +import java.util.Date; import java.util.List; import java.util.Map; import java.util.UUID; @@ -66,7 +67,7 @@ public Throwable getOriginalError() { * Information extracted from the {@link Throwable} that caused the event can be found in this * field. The list contains at least one {@link Error} that represents the thrown object * with subsequent elements in the list populated from {@link Throwable#getCause()}. - *

+ * * A reference to the actual {@link Throwable} object that caused the event is available * through {@link Event#getOriginalError()} ()}. */ @@ -167,7 +168,7 @@ public Severity getSeverity() { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - *

+ * * As the name implies, this option accepts a hash of sorts. */ public void setGroupingHash(@Nullable String groupingHash) { @@ -179,7 +180,7 @@ public void setGroupingHash(@Nullable String groupingHash) { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - *

+ * * As the name implies, this option accepts a hash of sorts. */ @Nullable @@ -362,7 +363,7 @@ public void toStream(@NonNull JsonStream stream) throws IOException { /** * Whether the event was a crash (i.e. unhandled) or handled error in which the system * continued running. - *

+ * * Unhandled errors count towards your stability score. If you don't want certain errors * to count towards your stability score, you can alter this property through an * {@link OnErrorCallback}. @@ -374,7 +375,7 @@ public boolean isUnhandled() { /** * Whether the event was a crash (i.e. unhandled) or handled error in which the system * continued running. - *

+ * * Unhandled errors count towards your stability score. If you don't want certain errors * to count towards your stability score, you can alter this property through an * {@link OnErrorCallback}. @@ -443,25 +444,46 @@ void setInternalMetrics(InternalMetrics metrics) { impl.setInternalMetrics(metrics); } - public Error addError(Throwable error) { + /** + * Open API for adding errors, threads and breadcrumbs to the event. + */ + + @Nullable + public Error addError(@NonNull Throwable error) { if (error == null) { return null; } return impl.addError(error); } - public Error addError(String errorClass, String errorMessage) { + @Nullable + public Error addError(@NonNull String errorClass, @NonNull String errorMessage) { return impl.addError(errorClass, errorMessage, ErrorType.ANDROID); } - - public Error addError(String errorClass, String errorMessage, ErrorType errorType) { + @Nullable + public Error addError(@NonNull String errorClass, + @NonNull String errorMessage, + @NonNull ErrorType errorType) { return impl.addError(errorClass, errorMessage, errorType); } - public Thread addThread(Thread thread) { - return impl.addThread(thread); + @Nullable + public Thread addThread(@NonNull String id, + @NonNull String name) { + return impl.addThread(id, name, ErrorType.ANDROID, false, + Thread.State.RUNNABLE.getDescriptor()); } + @NonNull + public Breadcrumb leaveBreadcrumb(@NonNull String message, + @NonNull BreadcrumbType type, + @Nullable Map metadata) { + return impl.leaveBreadcrumb(message, type, metadata, new Date()); + } + @NonNull + public Breadcrumb leaveBreadcrumb(@NonNull String message) { + return impl.leaveBreadcrumb(message); + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt index 6842b738b3..ed50d06cb1 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt @@ -6,6 +6,7 @@ import com.bugsnag.android.internal.InternalMetricsNoop import com.bugsnag.android.internal.JsonHelper import com.bugsnag.android.internal.TrimMetrics import java.io.IOException +import java.util.Date import java.util.regex.Pattern internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, MetadataAware, UserAware { @@ -326,12 +327,12 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata override fun clearFeatureFlags() = featureFlags.clearFeatureFlags() fun addError(throwError: Throwable): Error? { - val error = Error.createError(throwError, projectPackages, logger) - errors.addAll(error) - return error.firstOrNull() + val newErrors = Error.createError(throwError, projectPackages, logger) + errors.addAll(newErrors) + return newErrors.firstOrNull() } - fun addError(errorClass: String, errorMessage: String, errorType: ErrorType): Error { + fun addError(errorClass: String, errorMessage: String?, errorType: ErrorType): Error { val error = Error( ErrorInternal(errorClass, errorMessage, Stacktrace(ArrayList()), errorType), logger @@ -340,8 +341,44 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata return error } - fun addThread(thread: Thread): Thread { + fun addThread( + id: String, + name: String, + errorType: ErrorType, + isErrorReportingThread: Boolean, + state: String + ): Thread { + val thread = Thread( + ThreadInternal( + id, + name, + errorType, + isErrorReportingThread, + state, + Stacktrace(ArrayList()) + ), + logger + ) threads.add(thread) return thread } + + fun leaveBreadcrumb( + message: String, + type: BreadcrumbType, + metadata: MutableMap?, + timestamp: Date + ): Breadcrumb { + val breadcrumb = Breadcrumb(message, type, metadata, timestamp, logger) + breadcrumbs.add(breadcrumb) + return breadcrumb + } + + fun leaveBreadcrumb( + message: String + ): Breadcrumb { + val breadcrumb = Breadcrumb(message, logger) + breadcrumbs.add(breadcrumb) + return breadcrumb + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt index d7ea3f6b49..37710fc32c 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt @@ -25,9 +25,33 @@ internal class Stacktrace : JsonStream.Streamable { else -> null } } + + public fun serializeStackframe( + el: StackTraceElement, + projectPackages: Collection, + logger: Logger + ): Stackframe? { + try { + val className = el.className + val methodName = when { + className.isNotEmpty() -> className + "." + el.methodName + else -> el.methodName + } + + return Stackframe( + methodName, + el.fileName ?: "Unknown", + el.lineNumber, + inProject(className, projectPackages) + ) + } catch (lineEx: Exception) { + logger.w("Failed to serialize stacktrace", lineEx) + return null + } + } } - val trace: List + val trace: MutableList constructor(frames: List) { trace = limitTraceLength(frames) @@ -39,7 +63,7 @@ internal class Stacktrace : JsonStream.Streamable { logger: Logger ) { val frames = limitTraceLength(stacktrace) - trace = frames.mapNotNull { serializeStackframe(it, projectPackages, logger) } + trace = frames.mapNotNullTo(ArrayList()) { Companion.serializeStackframe(it, projectPackages, logger) } } private fun limitTraceLength(frames: Array): Array { @@ -49,34 +73,10 @@ internal class Stacktrace : JsonStream.Streamable { } } - private fun limitTraceLength(frames: List): List { + private fun limitTraceLength(frames: List): MutableList { return when { - frames.size >= STACKTRACE_TRIM_LENGTH -> frames.subList(0, STACKTRACE_TRIM_LENGTH) - else -> frames - } - } - - private fun serializeStackframe( - el: StackTraceElement, - projectPackages: Collection, - logger: Logger - ): Stackframe? { - try { - val className = el.className - val methodName = when { - className.isNotEmpty() -> className + "." + el.methodName - else -> el.methodName - } - - return Stackframe( - methodName, - el.fileName ?: "Unknown", - el.lineNumber, - inProject(className, projectPackages) - ) - } catch (lineEx: Exception) { - logger.w("Failed to serialize stacktrace", lineEx) - return null + frames.size >= STACKTRACE_TRIM_LENGTH -> frames.subList(0, STACKTRACE_TRIM_LENGTH).toMutableList() + else -> frames.toMutableList() } } From 8a076370bfff4f813cfcf8a77dee4c40a007f5df Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 09:42:49 +0100 Subject: [PATCH 10/23] refactor(event): reworked Event.addError/leaveBreadcrumb/addThread to handle unlikely nulls, and not need to be marked @Nullable --- .../main/java/com/bugsnag/android/Event.java | 141 ++++++++++++------ .../java/com/bugsnag/android/EventInternal.kt | 37 ++--- 2 files changed, 113 insertions(+), 65 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index 1f4fd23dc2..3d70a6a6c2 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -8,7 +8,6 @@ import java.io.IOException; import java.util.Collection; -import java.util.Date; import java.util.List; import java.util.Map; import java.util.UUID; @@ -52,7 +51,7 @@ private void logNull(String property) { } /** - * The Throwable object that caused the event in your application. + * The {@link Throwable} object that caused the event in your application. *

* Manipulating this field does not affect the error information reported to the * Bugsnag dashboard. Use {@link Event#getErrors()} to access and amend the representation of @@ -67,7 +66,7 @@ public Throwable getOriginalError() { * Information extracted from the {@link Throwable} that caused the event can be found in this * field. The list contains at least one {@link Error} that represents the thrown object * with subsequent elements in the list populated from {@link Throwable#getCause()}. - * + *

* A reference to the actual {@link Throwable} object that caused the event is available * through {@link Event#getOriginalError()} ()}. */ @@ -76,6 +75,35 @@ public List getErrors() { return impl.getErrors(); } + /** + * Add a new error to this report and return its Error data. The new Error will appear at the + * end of the {@link #getErrors() errors list}. + */ + @NonNull + public Error addError(@NonNull Throwable error) { + return impl.addError(error != null ? error : new Throwable()); + } + + /** + * Add a new empty error to this report and return its Error data. The new Error will appear + * at the end of the {@link #getErrors() errors list}. + */ + @NonNull + public Error addError(@NonNull String errorClass, @NonNull String errorMessage) { + return impl.addError(errorClass, errorMessage, ErrorType.ANDROID); + } + + /** + * Add a new empty error to this report and return its Error data. The new Error will appear + * at the end of the {@link #getErrors() errors list}. + */ + @NonNull + public Error addError(@NonNull String errorClass, + @NonNull String errorMessage, + @NonNull ErrorType errorType) { + return impl.addError(errorClass, errorMessage, errorType); + } + /** * If thread state is being captured along with the event, this field will contain a * list of {@link Thread} objects. @@ -85,6 +113,46 @@ public List getThreads() { return impl.getThreads(); } + /** + * Create, add and return a new empty {@link Thread} object to this event with a given id + * and name. This can be used to augment the report with thread data that would not be picked + * up as part of a normal report being generated (for example: native threads managed + * by cross-platform toolkits). + * + * @return a new Thread object of type {@link ErrorType#ANDROID} with no stacktrace + */ + @NonNull + public Thread addThread(@NonNull String id, + @NonNull String name) { + return impl.addThread( + id, + name, + ErrorType.ANDROID, + false, + Thread.State.RUNNABLE.getDescriptor() + ); + } + + /** + * Create, add and return a new empty {@link Thread} object to this event with a given id + * and name. This can be used to augment the report with thread data that would not be picked + * up as part of a normal report being generated (for example: native threads managed + * by cross-platform toolkits). + * + * @return a new Thread object of type {@link ErrorType#ANDROID} with no stacktrace + */ + @NonNull + public Thread addThread(long id, + @NonNull String name) { + return impl.addThread( + Long.toString(id), + name, + ErrorType.ANDROID, + false, + Thread.State.RUNNABLE.getDescriptor() + ); + } + /** * A list of breadcrumbs leading up to the event. These values can be accessed and amended * if necessary. See {@link Breadcrumb} for details of the data available. @@ -94,6 +162,26 @@ public List getBreadcrumbs() { return impl.getBreadcrumbs(); } + /** + * Add a new breadcrumb to this event and return its Breadcrumb object. The new breadcrumb + * will be added to the end of the {@link #getBreadcrumbs() breadcrumbs list} by this method. + */ + @NonNull + public Breadcrumb leaveBreadcrumb(@NonNull String message, + @NonNull BreadcrumbType type, + @Nullable Map metadata) { + return impl.leaveBreadcrumb(message, type, metadata); + } + + /** + * Add a new breadcrumb to this event and return its Breadcrumb object. The new breadcrumb + * will be added to the end of the {@link #getBreadcrumbs() breadcrumbs list} by this# method. + */ + @NonNull + public Breadcrumb leaveBreadcrumb(@NonNull String message) { + return impl.leaveBreadcrumb(message, BreadcrumbType.MANUAL, null); + } + /** * A list of feature flags active at the time of the event. * See {@link FeatureFlag} for details of the data available. @@ -168,7 +256,7 @@ public Severity getSeverity() { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - * + *

* As the name implies, this option accepts a hash of sorts. */ public void setGroupingHash(@Nullable String groupingHash) { @@ -180,7 +268,7 @@ public void setGroupingHash(@Nullable String groupingHash) { * All events with the same grouping hash will be grouped together into one error. This is an * advanced usage of the library and mis-using it will cause your events not to group properly * in your dashboard. - * + *

* As the name implies, this option accepts a hash of sorts. */ @Nullable @@ -443,47 +531,4 @@ void setRedactedKeys(Collection redactedKeys) { void setInternalMetrics(InternalMetrics metrics) { impl.setInternalMetrics(metrics); } - - /** - * Open API for adding errors, threads and breadcrumbs to the event. - */ - - @Nullable - public Error addError(@NonNull Throwable error) { - if (error == null) { - return null; - } - return impl.addError(error); - } - - @Nullable - public Error addError(@NonNull String errorClass, @NonNull String errorMessage) { - return impl.addError(errorClass, errorMessage, ErrorType.ANDROID); - } - - @Nullable - public Error addError(@NonNull String errorClass, - @NonNull String errorMessage, - @NonNull ErrorType errorType) { - return impl.addError(errorClass, errorMessage, errorType); - } - - @Nullable - public Thread addThread(@NonNull String id, - @NonNull String name) { - return impl.addThread(id, name, ErrorType.ANDROID, false, - Thread.State.RUNNABLE.getDescriptor()); - } - - @NonNull - public Breadcrumb leaveBreadcrumb(@NonNull String message, - @NonNull BreadcrumbType type, - @Nullable Map metadata) { - return impl.leaveBreadcrumb(message, type, metadata, new Date()); - } - - @NonNull - public Breadcrumb leaveBreadcrumb(@NonNull String message) { - return impl.leaveBreadcrumb(message); - } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt index ed50d06cb1..cd35772578 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt @@ -329,12 +329,17 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata fun addError(throwError: Throwable): Error? { val newErrors = Error.createError(throwError, projectPackages, logger) errors.addAll(newErrors) - return newErrors.firstOrNull() + return newErrors.first() } - fun addError(errorClass: String, errorMessage: String?, errorType: ErrorType): Error { + fun addError(errorClass: String?, errorMessage: String?, errorType: ErrorType?): Error { val error = Error( - ErrorInternal(errorClass, errorMessage, Stacktrace(ArrayList()), errorType), + ErrorInternal( + errorClass.toString(), + errorMessage, + Stacktrace(ArrayList()), + errorType ?: ErrorType.ANDROID + ), logger ) errors.add(error) @@ -343,7 +348,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata fun addThread( id: String, - name: String, + name: String?, errorType: ErrorType, isErrorReportingThread: Boolean, state: String @@ -351,7 +356,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata val thread = Thread( ThreadInternal( id, - name, + name.toString(), errorType, isErrorReportingThread, state, @@ -364,20 +369,18 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata } fun leaveBreadcrumb( - message: String, - type: BreadcrumbType, - metadata: MutableMap?, - timestamp: Date + message: String?, + type: BreadcrumbType?, + metadata: MutableMap? ): Breadcrumb { - val breadcrumb = Breadcrumb(message, type, metadata, timestamp, logger) - breadcrumbs.add(breadcrumb) - return breadcrumb - } + val breadcrumb = Breadcrumb( + message.toString(), + type ?: BreadcrumbType.MANUAL, + metadata, + Date(), + logger + ) - fun leaveBreadcrumb( - message: String - ): Breadcrumb { - val breadcrumb = Breadcrumb(message, logger) breadcrumbs.add(breadcrumb) return breadcrumb } From ec529a80ad13b6b9c42f67e5d76532344763c394 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 11:51:13 +0100 Subject: [PATCH 11/23] refactor(event): removed `Error.addStackframe(StackTraceElement)` since it is unlikely to be used, and dramatically complicates the API --- .../main/java/com/bugsnag/android/Error.java | 32 ++++++------------- .../java/com/bugsnag/android/ErrorInternal.kt | 29 +++++++---------- .../java/com/bugsnag/android/Stacktrace.kt | 4 +-- .../com/bugsnag/android/ErrorFacadeTest.java | 16 ++++++++++ 4 files changed, 39 insertions(+), 42 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java index 24a6412974..d711e4d609 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Error.java @@ -88,6 +88,16 @@ public List getStacktrace() { return impl.getStacktrace(); } + /** + * Add a new stackframe to the end of this Error returning the new Stackframe data object. + */ + @NonNull + public Stackframe addStackframe(@Nullable String method, + @Nullable String file, + long lineNumber) { + return impl.addStackframe(method, file, lineNumber); + } + @Override public void toStream(@NonNull JsonStream stream) throws IOException { impl.toStream(stream); @@ -98,26 +108,4 @@ static List createError(@NonNull Throwable exc, @NonNull Logger logger) { return ErrorInternal.Companion.createError(exc, projectPackages, logger); } - - /** - * A stackframe can be added to an error in an event - */ - - @Nullable - public Stackframe addStackframe(@Nullable StackTraceElement element, - @NonNull Collection projectPackages) { - if (element != null) { - return impl.addStackframe(element, projectPackages, logger); - } else { - logNull("stackframe"); - } - return null; - } - - @NonNull - public Stackframe addStackframe(@Nullable String method, - @Nullable String file, - @Nullable Number lineNumber) { - return impl.addStackframe(method, file, lineNumber); - } } \ No newline at end of file diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt index 22f50b2a58..9e716bbd62 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ErrorInternal.kt @@ -9,6 +9,12 @@ internal class ErrorInternal @JvmOverloads internal constructor( val stacktrace: MutableList = stacktrace.trace + fun addStackframe(method: String?, file: String?, lineNumber: Long): Stackframe { + val frame = Stackframe(method, file, lineNumber, null) + stacktrace.add(frame) + return frame + } + internal companion object { fun createError( exc: Throwable, @@ -20,8 +26,11 @@ internal class ErrorInternal @JvmOverloads internal constructor( // Somehow it's possible for stackTrace to be null in rare cases val stacktrace = currentEx.stackTrace ?: arrayOf() val trace = Stacktrace(stacktrace, projectPackages, logger) - val errorInternal = - ErrorInternal(currentEx.javaClass.name, currentEx.localizedMessage, trace) + val errorInternal = ErrorInternal( + currentEx.javaClass.name, + currentEx.localizedMessage, + trace + ) return@mapTo Error(errorInternal, logger) } @@ -36,20 +45,4 @@ internal class ErrorInternal @JvmOverloads internal constructor( writer.name("stacktrace").value(stacktrace) writer.endObject() } - - fun addStackframe( - element: StackTraceElement, - projectPackages: Collection, - logger: Logger - ): Stackframe? { - val frame = Stacktrace.serializeStackframe(element, projectPackages, logger) - frame?.let { stacktrace.add(it) } - return frame - } - - fun addStackframe(method: String?, file: String?, lineNumber: Number?): Stackframe { - val frame = Stackframe(method, file, lineNumber, null) - stacktrace.add(frame) - return frame - } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt index 37710fc32c..20fe2f7195 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt @@ -26,7 +26,7 @@ internal class Stacktrace : JsonStream.Streamable { } } - public fun serializeStackframe( + fun serializeStackframe( el: StackTraceElement, projectPackages: Collection, logger: Logger @@ -63,7 +63,7 @@ internal class Stacktrace : JsonStream.Streamable { logger: Logger ) { val frames = limitTraceLength(stacktrace) - trace = frames.mapNotNullTo(ArrayList()) { Companion.serializeStackframe(it, projectPackages, logger) } + trace = frames.mapNotNullTo(ArrayList()) { serializeStackframe(it, projectPackages, logger) } } private fun limitTraceLength(frames: Array): Array { diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java index dfe48a4d73..0cf594e971 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import org.junit.Before; import org.junit.Test; @@ -73,4 +74,19 @@ public void typeInvalid() { public void stacktraceValid() { assertEquals(trace, error.getStacktrace()); } + + @Test + public void addStackframe() { + Stackframe frame = error.addStackframe( + "SomeClass.fakeMethod", + "NoSuchFile.dat", + 1234L + ); + + // check the new frame is the last frame in the error stacktrace + assertSame(frame, error.getStacktrace().get(error.getStacktrace().size() - 1)); + assertEquals("SomeClass.fakeMethod", frame.getMethod()); + assertEquals("NoSuchFile.dat", frame.getFile()); + assertEquals(1234L, frame.getLineNumber()); + } } From 1785484ad5a879f4c18ae6b78539323429a9fa1a Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 11:57:00 +0100 Subject: [PATCH 12/23] refactor(thread): added `Thread.addStackframe` --- .../main/java/com/bugsnag/android/Thread.java | 10 ++++++++++ .../java/com/bugsnag/android/ThreadInternal.kt | 6 ++++++ .../com/bugsnag/android/ThreadFacadeTest.java | 16 ++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Thread.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Thread.java index adf7855e69..a6368ab275 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Thread.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Thread.java @@ -160,6 +160,16 @@ public List getStacktrace() { return impl.getStacktrace(); } + /** + * Add a new stackframe to the end of this thread returning the new Stackframe data object. + */ + @NonNull + public Stackframe addStackframe(@Nullable String method, + @Nullable String file, + long lineNumber) { + return impl.addStackframe(method, file, lineNumber); + } + @Override public void toStream(@NonNull JsonStream stream) throws IOException { impl.toStream(stream); diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadInternal.kt index df29a8263f..0671ad6ec9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadInternal.kt @@ -13,6 +13,12 @@ class ThreadInternal internal constructor( var stacktrace: MutableList = stacktrace.trace.toMutableList() + fun addStackframe(method: String?, file: String?, lineNumber: Long): Stackframe { + val frame = Stackframe(method, file, lineNumber, null) + stacktrace.add(frame) + return frame + } + @Throws(IOException::class) override fun toStream(writer: JsonStream) { writer.beginObject() diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java index 18c49f490f..ab6c7d3892 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import org.junit.Before; @@ -96,4 +97,19 @@ public void stacktraceInvalid() { assertEquals(stacktrace.getTrace(), thread.getStacktrace()); assertNotNull(logger.getMsg()); } + + @Test + public void addStackframe() { + Stackframe frame = thread.addStackframe( + "SomeClass.fakeMethod", + "NoSuchFile.dat", + 1234L + ); + + // check the new frame is the last frame in the thread stacktrace + assertSame(frame, thread.getStacktrace().get(thread.getStacktrace().size() - 1)); + assertEquals("SomeClass.fakeMethod", frame.getMethod()); + assertEquals("NoSuchFile.dat", frame.getFile()); + assertEquals(1234L, frame.getLineNumber()); + } } From ba232b2651832fedf05c016227d074f503b1a146 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 11:59:50 +0100 Subject: [PATCH 13/23] refactor(event): further null-safety --- .../src/main/java/com/bugsnag/android/EventInternal.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt index cd35772578..5db90f8d31 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt @@ -347,7 +347,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata } fun addThread( - id: String, + id: String?, name: String?, errorType: ErrorType, isErrorReportingThread: Boolean, @@ -355,7 +355,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata ): Thread { val thread = Thread( ThreadInternal( - id, + id.toString(), name.toString(), errorType, isErrorReportingThread, From 5117db0b3ef3a0d67b7560430714e55c73adb285 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 12:19:12 +0100 Subject: [PATCH 14/23] refactor(event): passing `null` to `Event.addError(Throwable)` will result in an empty `Error` instead of a spurious error based on the current stacktrace --- .../main/java/com/bugsnag/android/Event.java | 6 +++--- .../java/com/bugsnag/android/EventInternal.kt | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index 3d70a6a6c2..e758a13a8e 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -81,7 +81,7 @@ public List getErrors() { */ @NonNull public Error addError(@NonNull Throwable error) { - return impl.addError(error != null ? error : new Throwable()); + return impl.addError(error); } /** @@ -89,7 +89,7 @@ public Error addError(@NonNull Throwable error) { * at the end of the {@link #getErrors() errors list}. */ @NonNull - public Error addError(@NonNull String errorClass, @NonNull String errorMessage) { + public Error addError(@NonNull String errorClass, @Nullable String errorMessage) { return impl.addError(errorClass, errorMessage, ErrorType.ANDROID); } @@ -99,7 +99,7 @@ public Error addError(@NonNull String errorClass, @NonNull String errorMessage) */ @NonNull public Error addError(@NonNull String errorClass, - @NonNull String errorMessage, + @Nullable String errorMessage, @NonNull ErrorType errorType) { return impl.addError(errorClass, errorMessage, errorType); } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt index 5db90f8d31..8345929248 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventInternal.kt @@ -326,10 +326,19 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata override fun clearFeatureFlags() = featureFlags.clearFeatureFlags() - fun addError(throwError: Throwable): Error? { - val newErrors = Error.createError(throwError, projectPackages, logger) - errors.addAll(newErrors) - return newErrors.first() + fun addError(thrownError: Throwable?): Error { + if (thrownError == null) { + val newError = Error( + ErrorInternal("null", null, Stacktrace(ArrayList())), + logger + ) + errors.add(newError) + return newError + } else { + val newErrors = Error.createError(thrownError, projectPackages, logger) + errors.addAll(newErrors) + return newErrors.first() + } } fun addError(errorClass: String?, errorMessage: String?, errorType: ErrorType?): Error { From ba1c5fc11ac7121c524b8cd54cfd19bdddf3c080 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 12:21:31 +0100 Subject: [PATCH 15/23] docs(event): better JavaDocs for `Event.addError(errorClass, errorMessage)` --- .../src/main/java/com/bugsnag/android/Event.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index e758a13a8e..f2e82340e7 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -85,8 +85,8 @@ public Error addError(@NonNull Throwable error) { } /** - * Add a new empty error to this report and return its Error data. The new Error will appear - * at the end of the {@link #getErrors() errors list}. + * Add a new empty {@link ErrorType#ANDROID android} error to this report and return its Error + * data. The new Error will appear at the end of the {@link #getErrors() errors list}. */ @NonNull public Error addError(@NonNull String errorClass, @Nullable String errorMessage) { From 9f73257507ab9b2a94654d75dcb27908e89694a6 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 12:26:20 +0100 Subject: [PATCH 16/23] test(facades): new unit tests for the addError/addStackframe/addThread APIs --- .../com/bugsnag/android/ErrorFacadeTest.java | 11 ++++ .../com/bugsnag/android/EventFacadeTest.java | 63 +++++++++++++++++++ .../com/bugsnag/android/ThreadFacadeTest.java | 12 ++++ 3 files changed, 86 insertions(+) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java index 0cf594e971..f6c9db0c27 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java @@ -89,4 +89,15 @@ public void addStackframe() { assertEquals("NoSuchFile.dat", frame.getFile()); assertEquals(1234L, frame.getLineNumber()); } + + @Test + public void addStackframeWithNulls() { + Stackframe frame = error.addStackframe(null, null, -1L); + + // check the new frame is the last frame in the error stacktrace + assertSame(frame, error.getStacktrace().get(error.getStacktrace().size() - 1)); + assertNull(frame.getMethod()); + assertNull(frame.getFile()); + assertEquals(-1L, frame.getLineNumber()); + } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFacadeTest.java index cf8ee75f6c..c07113deb7 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFacadeTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import com.bugsnag.android.internal.ImmutableConfig; @@ -195,4 +196,66 @@ public void unhandledValid() { event.setUnhandled(false); assertFalse(event.isUnhandled()); } + + @Test + public void addThread() { + Thread newThread = event.addThread(123L, "Magic Thread"); + assertSame(newThread, event.getThreads().get(event.getThreads().size() - 1)); + assertEquals("123", newThread.getId()); + assertEquals("Magic Thread", newThread.getName()); + assertEquals(ErrorType.ANDROID, newThread.getType()); + assertEquals(Thread.State.RUNNABLE, newThread.getState()); + assertEquals(0, newThread.getStacktrace().size()); + } + + @Test + public void addError() { + Error newError = event.addError("ErrorClass", "No error message"); + assertSame(newError, event.getErrors().get(event.getErrors().size() - 1)); + assertEquals("ErrorClass", newError.getErrorClass()); + assertEquals("No error message", newError.getErrorMessage()); + assertEquals(ErrorType.ANDROID, newError.getType()); + assertEquals(0, newError.getStacktrace().size()); + } + + @Test + public void addErrorWithType() { + Error newError = event.addError("ErrorClass", "No error message", ErrorType.DART); + assertSame(newError, event.getErrors().get(event.getErrors().size() - 1)); + assertEquals("ErrorClass", newError.getErrorClass()); + assertEquals("No error message", newError.getErrorMessage()); + assertEquals(ErrorType.DART, newError.getType()); + assertEquals(0, newError.getStacktrace().size()); + } + + @Test + public void addErrorNullThrowable() { + Error newError = event.addError(null); + assertSame(newError, event.getErrors().get(event.getErrors().size() - 1)); + assertEquals("null", newError.getErrorClass()); + assertNull(newError.getErrorMessage()); + assertEquals(ErrorType.ANDROID, newError.getType()); + assertEquals(0, newError.getStacktrace().size()); + } + + @Test + public void addThreadWithNulls() { + Thread newThread = event.addThread(null, null); + assertSame(newThread, event.getThreads().get(event.getThreads().size() - 1)); + assertEquals("null", newThread.getId()); + assertEquals("null", newThread.getName()); + assertEquals(ErrorType.ANDROID, newThread.getType()); + assertEquals(Thread.State.RUNNABLE, newThread.getState()); + assertEquals(0, newThread.getStacktrace().size()); + } + + @Test + public void addBadError() { + Error newError = event.addError(null, null); + assertSame(newError, event.getErrors().get(event.getErrors().size() - 1)); + assertEquals("null", newError.getErrorClass()); + assertNull(newError.getErrorMessage()); + assertEquals(ErrorType.ANDROID, newError.getType()); + assertEquals(0, newError.getStacktrace().size()); + } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java index ab6c7d3892..3d0e65ad96 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadFacadeTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -112,4 +113,15 @@ public void addStackframe() { assertEquals("NoSuchFile.dat", frame.getFile()); assertEquals(1234L, frame.getLineNumber()); } + + @Test + public void addStackframeWithNulls() { + Stackframe frame = thread.addStackframe(null, null, -1L); + + // check the new frame is the last frame in the thread stacktrace + assertSame(frame, thread.getStacktrace().get(thread.getStacktrace().size() - 1)); + assertNull(frame.getMethod()); + assertNull(frame.getFile()); + assertEquals(-1L, frame.getLineNumber()); + } } From 3b874f9b4e0ceba187ef348f82aadcd118aa2f86 Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 12:26:48 +0100 Subject: [PATCH 17/23] chore(api): update api binary-compatibility check files --- CHANGELOG.md | 2 ++ bugsnag-android-core/api/bugsnag-android-core.api | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd0e4e18c1..25066874f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * Include additional Intent information for Activity.onCreate breadcrumbs (action, categories, type, flags, id, extra keys) [#2057](https://github.com/bugsnag/bugsnag-android/pull/2057) +* New APIs allowing new `Error`s, `Thread`s, and `Stackframe`s to be added to error reports (`Event`s) + [#2060](https://github.com/bugsnag/bugsnag-android/pull/2060) ### Bug fixes diff --git a/bugsnag-android-core/api/bugsnag-android-core.api b/bugsnag-android-core/api/bugsnag-android-core.api index 296e5bb112..2ffe8e2f0f 100644 --- a/bugsnag-android-core/api/bugsnag-android-core.api +++ b/bugsnag-android-core/api/bugsnag-android-core.api @@ -312,8 +312,7 @@ public final class com/bugsnag/android/EndpointConfiguration { } public class com/bugsnag/android/Error : com/bugsnag/android/JsonStream$Streamable { - public fun addStackframe (Ljava/lang/StackTraceElement;Ljava/util/Collection;)Lcom/bugsnag/android/Stackframe; - public fun addStackframe (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Number;)Lcom/bugsnag/android/Stackframe; + public fun addStackframe (Ljava/lang/String;Ljava/lang/String;J)Lcom/bugsnag/android/Stackframe; public fun getErrorClass ()Ljava/lang/String; public fun getErrorMessage ()Ljava/lang/String; public fun getStacktrace ()Ljava/util/List; @@ -360,6 +359,7 @@ public class com/bugsnag/android/Event : com/bugsnag/android/FeatureFlagAware, c public fun addFeatureFlags (Ljava/lang/Iterable;)V public fun addMetadata (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;)V public fun addMetadata (Ljava/lang/String;Ljava/util/Map;)V + public fun addThread (JLjava/lang/String;)Lcom/bugsnag/android/Thread; public fun addThread (Ljava/lang/String;Ljava/lang/String;)Lcom/bugsnag/android/Thread; public fun clearFeatureFlag (Ljava/lang/String;)V public fun clearFeatureFlags ()V @@ -770,6 +770,7 @@ public final class com/bugsnag/android/Telemetry : java/lang/Enum { } public class com/bugsnag/android/Thread : com/bugsnag/android/JsonStream$Streamable { + public fun addStackframe (Ljava/lang/String;Ljava/lang/String;J)Lcom/bugsnag/android/Stackframe; public fun getErrorReportingThread ()Z public fun getId ()Ljava/lang/String; public fun getName ()Ljava/lang/String; @@ -800,6 +801,7 @@ public final class com/bugsnag/android/Thread$State : java/lang/Enum { } public final class com/bugsnag/android/ThreadInternal : com/bugsnag/android/JsonStream$Streamable { + public final fun addStackframe (Ljava/lang/String;Ljava/lang/String;J)Lcom/bugsnag/android/Stackframe; public final fun getId ()Ljava/lang/String; public final fun getName ()Ljava/lang/String; public final fun getStacktrace ()Ljava/util/List; From 08526eab992dff271633829770e79977123352fe Mon Sep 17 00:00:00 2001 From: jason Date: Mon, 5 Aug 2024 14:50:41 +0100 Subject: [PATCH 18/23] refactor(stacktrace): reduce the number of copies required when creating report stacktraces --- CHANGELOG.md | 2 +- .../com/bugsnag/android/BugsnagEventMapper.kt | 4 +-- .../main/java/com/bugsnag/android/Event.java | 14 +++++----- .../java/com/bugsnag/android/Stacktrace.kt | 27 ++++++++++--------- .../com/bugsnag/android/ErrorFacadeTest.java | 3 ++- .../bugsnag/android/ErrorSerializationTest.kt | 6 ++--- .../android/StacktraceSerializationTest.kt | 4 +-- .../com/bugsnag/android/StacktraceTest.kt | 2 +- .../android/ThreadDeserializationTest.kt | 6 ++--- .../bugsnag/android/ErrorDeserializer.java | 2 +- .../android/NativeStackDeserializer.java | 2 +- .../bugsnag/android/ThreadDeserializer.java | 2 +- .../bugsnag/android/ThreadSerializerTest.java | 3 ++- 13 files changed, 41 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25066874f9..615872f324 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ * Include additional Intent information for Activity.onCreate breadcrumbs (action, categories, type, flags, id, extra keys) [#2057](https://github.com/bugsnag/bugsnag-android/pull/2057) -* New APIs allowing new `Error`s, `Thread`s, and `Stackframe`s to be added to error reports (`Event`s) +* New APIs allowing new `Error`s, `Thread`s, and `Stackframe`s to be added to `Event`s [#2060](https://github.com/bugsnag/bugsnag-android/pull/2060) ### Bug fixes diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt index b6113c8b58..b63606de39 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt @@ -197,12 +197,12 @@ internal class BugsnagEventMapper( thread["errorReportingThread"] == true, thread["state"] as? String ?: "", (thread["stacktrace"] as? List>)?.let { convertStacktrace(it) } - ?: Stacktrace(emptyList()) + ?: Stacktrace(mutableListOf()) ) } internal fun convertStacktrace(trace: List>): Stacktrace { - return Stacktrace(trace.map { Stackframe(it) }) + return Stacktrace(trace.mapTo(ArrayList(trace.size)) { Stackframe(it) }) } internal fun deserializeSeverityReason( diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java index f2e82340e7..0c560107e5 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Event.java @@ -76,7 +76,7 @@ public List getErrors() { } /** - * Add a new error to this report and return its Error data. The new Error will appear at the + * Add a new error to this event and return its Error data. The new Error will appear at the * end of the {@link #getErrors() errors list}. */ @NonNull @@ -85,7 +85,7 @@ public Error addError(@NonNull Throwable error) { } /** - * Add a new empty {@link ErrorType#ANDROID android} error to this report and return its Error + * Add a new empty {@link ErrorType#ANDROID android} error to this event and return its Error * data. The new Error will appear at the end of the {@link #getErrors() errors list}. */ @NonNull @@ -94,7 +94,7 @@ public Error addError(@NonNull String errorClass, @Nullable String errorMessage) } /** - * Add a new empty error to this report and return its Error data. The new Error will appear + * Add a new empty error to this event and return its Error data. The new Error will appear * at the end of the {@link #getErrors() errors list}. */ @NonNull @@ -115,8 +115,8 @@ public List getThreads() { /** * Create, add and return a new empty {@link Thread} object to this event with a given id - * and name. This can be used to augment the report with thread data that would not be picked - * up as part of a normal report being generated (for example: native threads managed + * and name. This can be used to augment the event with thread data that would not be picked + * up as part of a normal event being generated (for example: native threads managed * by cross-platform toolkits). * * @return a new Thread object of type {@link ErrorType#ANDROID} with no stacktrace @@ -135,8 +135,8 @@ public Thread addThread(@NonNull String id, /** * Create, add and return a new empty {@link Thread} object to this event with a given id - * and name. This can be used to augment the report with thread data that would not be picked - * up as part of a normal report being generated (for example: native threads managed + * and name. This can be used to augment the event with thread data that would not be picked + * up as part of a normal event being generated (for example: native threads managed * by cross-platform toolkits). * * @return a new Thread object of type {@link ErrorType#ANDROID} with no stacktrace diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt index 20fe2f7195..e846f7c6c9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Stacktrace.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import java.io.IOException +import kotlin.math.min /** * Serialize an exception stacktrace and mark frames as "in-project" @@ -53,7 +54,7 @@ internal class Stacktrace : JsonStream.Streamable { val trace: MutableList - constructor(frames: List) { + constructor(frames: MutableList) { trace = limitTraceLength(frames) } @@ -62,21 +63,23 @@ internal class Stacktrace : JsonStream.Streamable { projectPackages: Collection, logger: Logger ) { - val frames = limitTraceLength(stacktrace) - trace = frames.mapNotNullTo(ArrayList()) { serializeStackframe(it, projectPackages, logger) } - } - - private fun limitTraceLength(frames: Array): Array { - return when { - frames.size >= STACKTRACE_TRIM_LENGTH -> frames.sliceArray(0 until STACKTRACE_TRIM_LENGTH) - else -> frames + // avoid allocating new subLists or Arrays by only copying the required number of frames + // mapping them to our internal Stackframes as we go, roughly equivalent to + // stacktrace.take(STACKTRACE_TRIM_LENGTH).mapNotNullTo(ArrayList()) { ... } + val frameCount = min(STACKTRACE_TRIM_LENGTH, stacktrace.size) + trace = ArrayList(frameCount) + for (i in 0 until frameCount) { + val frame = serializeStackframe(stacktrace[i], projectPackages, logger) + if (frame != null) { + trace.add(frame) + } } } - private fun limitTraceLength(frames: List): MutableList { + private fun limitTraceLength(frames: MutableList): MutableList { return when { - frames.size >= STACKTRACE_TRIM_LENGTH -> frames.subList(0, STACKTRACE_TRIM_LENGTH).toMutableList() - else -> frames.toMutableList() + frames.size >= STACKTRACE_TRIM_LENGTH -> frames.subList(0, STACKTRACE_TRIM_LENGTH) + else -> frames } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java index f6c9db0c27..518907d8d1 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorFacadeTest.java @@ -8,6 +8,7 @@ import org.junit.Before; import org.junit.Test; +import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -24,7 +25,7 @@ public class ErrorFacadeTest { @Before public void setUp() { logger = new InterceptingLogger(); - trace = Collections.emptyList(); + trace = new ArrayList<>(); ErrorInternal impl = new ErrorInternal("com.bar.CrashyClass", "Whoops", new Stacktrace(trace), ErrorType.ANDROID); error = new Error(impl, logger); diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorSerializationTest.kt index 4d81aba1c3..a74c7896c1 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ErrorSerializationTest.kt @@ -14,13 +14,13 @@ internal class ErrorSerializationTest { @Parameters fun testCases() = generateSerializationTestCases( "error", - Error(ErrorInternal("foo", "bar", Stacktrace(listOf())), NoopLogger), + Error(ErrorInternal("foo", "bar", Stacktrace(mutableListOf())), NoopLogger), Error( ErrorInternal( "foo", "bar", Stacktrace( - listOf( + mutableListOf( Stackframe( method = "foo()", file = "Bar.kt", @@ -39,7 +39,7 @@ internal class ErrorSerializationTest { "com.bugsnag.android.StacktraceSerializationTest", "bar", Stacktrace( - listOf( + mutableListOf( Stackframe( method = "com.bugsnag.android.StacktraceSerializationTest\$Companion.inProject", file = "StacktraceSerializationTest.kt", diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceSerializationTest.kt index 7a8ef7416e..272de3e94e 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceSerializationTest.kt @@ -22,7 +22,7 @@ internal class StacktraceSerializationTest { Stacktrace(arrayOf(), emptySet(), NoopLogger), // empty custom frames ctor - Stacktrace(listOf(frame)), + Stacktrace(mutableListOf(frame)), // basic basic(), @@ -57,7 +57,7 @@ internal class StacktraceSerializationTest { } private fun trimStacktraceListCtor(): Stacktrace { - val elements = (0..999).map { count -> + val elements = (0..999).mapTo(ArrayList()) { count -> Stackframe("Foo", "Bar.kt", count, true).also { frame -> // set different type for each frame frame.type = when (count % 3) { diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceTest.kt index 53049a3d55..975a003b96 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/StacktraceTest.kt @@ -7,7 +7,7 @@ class StacktraceTest { @Test fun stackframeListTrimmed() { - val stackList = (1..300).map { index -> + val stackList = (1..300).mapTo(ArrayList()) { index -> Stackframe("A", "B", index, true) } val stacktrace = Stacktrace(stackList) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt index fb92014ef3..12a5ffd627 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadDeserializationTest.kt @@ -21,7 +21,7 @@ class ThreadDeserializationTest { ErrorType.C, false, "", - Stacktrace(emptyList()) + Stacktrace(mutableListOf()) ), NoopLogger ), @@ -32,7 +32,7 @@ class ThreadDeserializationTest { ErrorType.ANDROID, false, "", - Stacktrace(emptyList()) + Stacktrace(mutableListOf()) ), NoopLogger ), @@ -47,7 +47,7 @@ class ThreadDeserializationTest { ErrorType.ANDROID, false, "happy", - Stacktrace(emptyList()) + Stacktrace(mutableListOf()) ), NoopLogger ) diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ErrorDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ErrorDeserializer.java index e81133688c..1e643d82d8 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ErrorDeserializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ErrorDeserializer.java @@ -19,7 +19,7 @@ class ErrorDeserializer implements MapDeserializer { public Error deserialize(Map map) { String type = MapUtils.getOrThrow(map, "type"); List> stacktrace = MapUtils.getOrThrow(map, "stacktrace"); - List frames = new ArrayList<>(); + List frames = new ArrayList<>(stacktrace.size()); for (Map frame : stacktrace) { frames.add(stackframeDeserializer.deserialize(frame)); diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/NativeStackDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/NativeStackDeserializer.java index ba55bdab96..6024648fc6 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/NativeStackDeserializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/NativeStackDeserializer.java @@ -30,7 +30,7 @@ class NativeStackDeserializer implements MapDeserializer> { @Override public List deserialize(Map map) { List> nativeStack = MapUtils.getOrThrow(map, "nativeStack"); - List frames = new ArrayList<>(); + List frames = new ArrayList<>(nativeStack.size()); for (Map frame : nativeStack) { frames.add(deserializeStackframe(frame, projectPackages)); diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ThreadDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ThreadDeserializer.java index bd600fb194..b61bea08c3 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ThreadDeserializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ThreadDeserializer.java @@ -19,7 +19,7 @@ class ThreadDeserializer implements MapDeserializer { public Thread deserialize(Map map) { String type = MapUtils.getOrThrow(map, "type"); List> stacktrace = MapUtils.getOrThrow(map, "stacktrace"); - List frames = new ArrayList<>(); + List frames = new ArrayList<>(stacktrace.size()); for (Map frame : stacktrace) { frames.add(stackframeDeserializer.deserialize(frame)); diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/ThreadSerializerTest.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/ThreadSerializerTest.java index 34f2496afd..0ebe85a52d 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/ThreadSerializerTest.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/ThreadSerializerTest.java @@ -11,6 +11,7 @@ import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -34,7 +35,7 @@ public void setup() { frame.put("inProject", true); Stackframe stackframe = new Stackframe("foo()", "Bar.kt", 55, true); - List frames = Collections.singletonList(stackframe); + List frames = new ArrayList<>(Collections.singletonList(stackframe)); Stacktrace stacktrace = new Stacktrace(frames); thread = new Thread("1", "fake-thread", ErrorType.ANDROID, true, Thread.State.RUNNABLE, stacktrace, NoopLogger.INSTANCE); From ed7f122d4cacc31ddc953aaf4b9dd4cebb444dda Mon Sep 17 00:00:00 2001 From: Alex Moinet Date: Wed, 7 Aug 2024 07:57:25 +0100 Subject: [PATCH 19/23] Add other downstream repos for automatic updates (#2056) --- .github/workflows/downstream_updates.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/downstream_updates.yml b/.github/workflows/downstream_updates.yml index fd0d771bdd..20de33f516 100644 --- a/.github/workflows/downstream_updates.yml +++ b/.github/workflows/downstream_updates.yml @@ -17,7 +17,7 @@ jobs: RELEASE_VERSION: ${{ github.event_name == 'workflow_dispatch' && inputs.target_version || github.event.release.tag_name }} strategy: matrix: - downstream_repo: ['bugsnag/bugsnag-unity'] + downstream_repo: ['bugsnag/bugsnag-unity', 'bugsnag/bugsnag-js', 'bugsnag/bugsnag-flutter', 'bugsnag/bugsnag-unreal'] steps: - name: Install libcurl4-openssl-dev and net-tools run: | From d806fdcc2a64b16dff32fe352aa481ede1a7dbcf Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 7 Aug 2024 14:53:01 +0100 Subject: [PATCH 20/23] fix(breadcrumbs): restored the expected timestamp format to NDK breadcrumbs --- CHANGELOG.md | 2 ++ .../src/main/java/com/bugsnag/android/BreadcrumbState.kt | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 615872f324..8d5f51c581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ [#2058](https://github.com/bugsnag/bugsnag-android/pull/2058) * Avoid racing ourselves in the `bugsnag-plugin-android-ndk` during multi-threaded startups [#2064](https://github.com/bugsnag/bugsnag-android/pull/2064) +* Fixed a timestamp formatting issue that caused NDK crash breadcrumbs to be dropped + []() ## 6.6.1 (2024-07-03) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbState.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbState.kt index e2d392b6b8..20192ebea9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbState.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbState.kt @@ -1,5 +1,6 @@ package com.bugsnag.android +import com.bugsnag.android.internal.DateUtils import java.io.IOException import java.util.concurrent.atomic.AtomicInteger @@ -42,8 +43,7 @@ internal class BreadcrumbState( StateEvent.AddBreadcrumb( breadcrumb.impl.message, breadcrumb.impl.type, - // an encoding of milliseconds since the epoch - "t${breadcrumb.impl.timestamp.time}", + DateUtils.toIso8601(breadcrumb.impl.timestamp), breadcrumb.impl.metadata ?: mutableMapOf() ) } From e875868d9a4c3d1eaba6040151c3ce958b00a22e Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 7 Aug 2024 15:15:01 +0100 Subject: [PATCH 21/23] fix(ndk): corrected the ndk event filename timestamp (should be ms, not seconds) --- CHANGELOG.md | 2 +- .../src/main/jni/utils/serializer/event_writer.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5f51c581..b2338a3fb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ * Avoid racing ourselves in the `bugsnag-plugin-android-ndk` during multi-threaded startups [#2064](https://github.com/bugsnag/bugsnag-android/pull/2064) * Fixed a timestamp formatting issue that caused NDK crash breadcrumbs to be dropped - []() + [#2066](https://github.com/bugsnag/bugsnag-android/pull/2066) ## 6.6.1 (2024-07-03) diff --git a/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c b/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c index 451982d800..935890205d 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c @@ -74,8 +74,8 @@ static size_t build_filename(bsg_environment *env, char *out) { memcpy(out, env->event_path, length); out[length++] = '/'; - // the timestamp is encoded as unix time - length += bsg_uint64_to_string(now, &out[length]); + // the timestamp is encoded as unix time in millis + length += bsg_uint64_to_string(now * 1000uL, &out[length]); // append the api_key to the filename out[length++] = '_'; From 6bee81a920470db1051bfc2ff1d4d78ee6ebb4b0 Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 7 Aug 2024 15:27:45 +0100 Subject: [PATCH 22/23] test(breadcrumbs): assert the correct timestamp when leaving breadcrumbs --- .../java/com/bugsnag/android/ObserverInterfaceTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java index 329d68de4e..6b6fc09649 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.bugsnag.android.internal.DateUtils; import com.bugsnag.android.internal.StateObserver; import androidx.annotation.NonNull; @@ -176,6 +177,8 @@ public void testLeaveStringBreadcrumbSendsMessage() { assertEquals(BreadcrumbType.MANUAL, crumb.type); assertEquals("Drift 4 units left", crumb.message); assertTrue(crumb.metadata.isEmpty()); + // DateUtils.fromIso8601 throws an exception on failure, but we also check for nulls + assertNotNull(DateUtils.fromIso8601(crumb.timestamp)); } @Test From 2243aedef8b8de4b4180c472caafe06e1d257584 Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 7 Aug 2024 17:06:52 +0100 Subject: [PATCH 23/23] v6.7.0 --- CHANGELOG.md | 2 +- .../src/main/java/com/bugsnag/android/Notifier.kt | 2 +- examples/sdk-app-example/app/build.gradle | 4 ++-- gradle.properties | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2338a3fb6..eab7cbfb86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## TBD +## 6.7.0 (2024-08-08) ### Enhancements diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt index d2da04b9ed..56e4794d8b 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt @@ -7,7 +7,7 @@ import java.io.IOException */ class Notifier @JvmOverloads constructor( var name: String = "Android Bugsnag Notifier", - var version: String = "6.6.1", + var version: String = "6.7.0", var url: String = "https://bugsnag.com" ) : JsonStream.Streamable { diff --git a/examples/sdk-app-example/app/build.gradle b/examples/sdk-app-example/app/build.gradle index 8db9eba255..eded1defaf 100644 --- a/examples/sdk-app-example/app/build.gradle +++ b/examples/sdk-app-example/app/build.gradle @@ -42,8 +42,8 @@ android { } dependencies { - implementation "com.bugsnag:bugsnag-android:6.6.1" - implementation "com.bugsnag:bugsnag-plugin-android-okhttp:6.6.1" + implementation "com.bugsnag:bugsnag-android:6.7.0" + implementation "com.bugsnag:bugsnag-plugin-android-okhttp:6.7.0" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "androidx.appcompat:appcompat:1.6.1" implementation "com.google.android.material:material:1.11.0" diff --git a/gradle.properties b/gradle.properties index 4581a3adf9..871db7c760 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ org.gradle.jvmargs=-Xmx4096m # This option should only be used with decoupled projects. More details, visit # http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects org.gradle.parallel=true -VERSION_NAME=6.6.1 +VERSION_NAME=6.7.0 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git