From 58cb2e1c70c522f978e8c379abc24376037d02d9 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Fri, 30 Sep 2022 12:00:47 +0200 Subject: [PATCH 1/8] Make Scenario a little more error friendly --- .../android/mazerunner/scenarios/Scenario.kt | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 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 728028c5ab..d07386ef17 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 @@ -114,7 +114,7 @@ abstract class Scenario( protected fun waitForEventFile() { val dir = errorsDir() while (dir.listFiles()!!.isEmpty()) { - dir.listFiles().forEach { println(it) } + dir.listFiles()?.forEach { println(it) } Thread.sleep(100) } } @@ -122,7 +122,7 @@ abstract class Scenario( protected fun waitForNoEventFiles() { val dir = errorsDir() while (!dir.listFiles()!!.isEmpty()) { - dir.listFiles().forEach { println(it) } + dir.listFiles()?.forEach { println(it) } Thread.sleep(1000) } } @@ -136,20 +136,33 @@ abstract class Scenario( override fun onActivityDestroyed(activity: Activity) {} companion object { + private fun loadClass(className: String): Class<*> { + try { + return Class.forName(className) + } catch (exc: Throwable) { + throw IllegalStateException( + "Failed to load test case class $className. Is it a valid JVM class?", + exc + ) + } + } + fun load( context: Context, config: Configuration, eventType: String, eventMetaData: String? ): Scenario { + log("Loading scenario $eventType with metadata $eventMetaData") + val clz = loadClass("com.bugsnag.android.mazerunner.scenarios.$eventType") + try { - log("Loading scenario $eventType with metadata $eventMetaData") - val clz = Class.forName("com.bugsnag.android.mazerunner.scenarios.$eventType") val constructor = clz.constructors[0] return constructor.newInstance(config, context, eventMetaData) as Scenario } catch (exc: Throwable) { throw IllegalStateException( - "Failed to instantiate test case for $eventType. Is it a valid JVM class?", + "Failed to construct test case for $eventType. Please check the nested" + + " exceptions for more details.", exc ) } From fc4b43c2c8584f23b331be98079b5e20e4b3b745 Mon Sep 17 00:00:00 2001 From: Tom Longridge Date: Wed, 5 Oct 2022 16:31:58 +0100 Subject: [PATCH 2/8] chore: update danger to v9 and git gem to avoid vulnerability --- features/fixtures/minimalapp/Gemfile | 3 +- features/fixtures/minimalapp/Gemfile.lock | 81 +++++++++++++++-------- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/features/fixtures/minimalapp/Gemfile b/features/fixtures/minimalapp/Gemfile index 4d3ae7c785..68049d0cbe 100644 --- a/features/fixtures/minimalapp/Gemfile +++ b/features/fixtures/minimalapp/Gemfile @@ -6,4 +6,5 @@ git_source(:github) {|repo_name| "https://github.com/#{repo_name}" } # gem "rails" -gem "danger", "~> 6.1" +gem "danger", "~> 9" +gem "git", ">= 1.11.0" # Force update of git gem to avoid CVE-2022-25648 \ No newline at end of file diff --git a/features/fixtures/minimalapp/Gemfile.lock b/features/fixtures/minimalapp/Gemfile.lock index c2b7d96ea3..b3db855106 100644 --- a/features/fixtures/minimalapp/Gemfile.lock +++ b/features/fixtures/minimalapp/Gemfile.lock @@ -1,9 +1,9 @@ GEM remote: https://rubygems.org/ specs: - addressable (2.7.0) - public_suffix (>= 2.0.2, < 5.0) - claide (1.0.3) + addressable (2.8.1) + public_suffix (>= 2.0.2, < 6.0) + claide (1.1.0) claide-plugins (0.9.2) cork nap @@ -11,46 +11,75 @@ GEM colored2 (3.1.2) cork (0.3.0) colored2 (~> 3.1) - danger (6.1.0) + danger (9.0.0) claide (~> 1.0) claide-plugins (>= 0.9.2) colored2 (~> 3.1) cork (~> 0.1) - faraday (~> 0.9) + faraday (>= 0.9.0, < 2.0) faraday-http-cache (~> 2.0) - git (~> 1.5) - kramdown (~> 2.0) + git (~> 1.7) + kramdown (~> 2.3) kramdown-parser-gfm (~> 1.0) no_proxy_fix - octokit (~> 4.7) - terminal-table (~> 1) - faraday (0.17.0) - multipart-post (>= 1.2, < 3) - faraday-http-cache (2.0.0) - faraday (~> 0.8) - git (1.5.0) - kramdown (2.1.0) + octokit (~> 5.0) + terminal-table (>= 1, < 4) + faraday (1.10.2) + faraday-em_http (~> 1.0) + faraday-em_synchrony (~> 1.0) + faraday-excon (~> 1.1) + faraday-httpclient (~> 1.0) + faraday-multipart (~> 1.0) + faraday-net_http (~> 1.0) + faraday-net_http_persistent (~> 1.0) + faraday-patron (~> 1.0) + faraday-rack (~> 1.0) + faraday-retry (~> 1.0) + ruby2_keywords (>= 0.0.4) + faraday-em_http (1.0.0) + faraday-em_synchrony (1.0.0) + faraday-excon (1.1.0) + faraday-http-cache (2.4.1) + faraday (>= 0.8) + faraday-httpclient (1.0.1) + faraday-multipart (1.0.4) + multipart-post (~> 2) + faraday-net_http (1.0.1) + faraday-net_http_persistent (1.2.0) + faraday-patron (1.0.0) + faraday-rack (1.0.0) + faraday-retry (1.0.3) + git (1.12.0) + addressable (~> 2.8) + rchardet (~> 1.8) + kramdown (2.4.0) + rexml kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) - multipart-post (2.1.1) + multipart-post (2.2.3) nap (1.1.0) no_proxy_fix (0.1.2) - octokit (4.14.0) - sawyer (~> 0.8.0, >= 0.5.3) + octokit (5.6.1) + faraday (>= 1, < 3) + sawyer (~> 0.9) open4 (1.3.4) - public_suffix (4.0.1) - sawyer (0.8.2) + public_suffix (5.0.0) + rchardet (1.8.0) + rexml (3.2.5) + ruby2_keywords (0.0.5) + sawyer (0.9.2) addressable (>= 2.3.5) - faraday (> 0.8, < 2.0) - terminal-table (1.8.0) - unicode-display_width (~> 1.1, >= 1.1.1) - unicode-display_width (1.6.0) + faraday (>= 0.17.3, < 3) + terminal-table (3.0.2) + unicode-display_width (>= 1.1.1, < 3) + unicode-display_width (2.3.0) PLATFORMS ruby DEPENDENCIES - danger (~> 6.1) + danger (~> 9) + git (>= 1.11.0) BUNDLED WITH - 2.0.2 + 2.2.33 From 7437dfbd035799a08bcd33c0a5dc2f1c9ff778a3 Mon Sep 17 00:00:00 2001 From: Steve Kirkland-Walton Date: Tue, 11 Oct 2022 11:52:31 +0100 Subject: [PATCH 3/8] Use Android 13 [full ci] --- .buildkite/pipeline.full.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.buildkite/pipeline.full.yml b/.buildkite/pipeline.full.yml index cbef206180..9ffd3ba773 100644 --- a/.buildkite/pipeline.full.yml +++ b/.buildkite/pipeline.full.yml @@ -388,7 +388,7 @@ steps: concurrency_group: 'browserstack-app' concurrency_method: eager - - label: ':android: Android 13 Beta NDK r21 end-to-end tests - batch 1' + - label: ':android: Android 13 NDK r21 end-to-end tests - batch 1' depends_on: "fixture-r21" timeout_in_minutes: 60 plugins: @@ -405,7 +405,7 @@ steps: - "--exclude=features/full_tests/[^a-k].*.feature" - "--app=/app/build/fixture-r21.apk" - "--farm=bs" - - "--device=ANDROID_13_0_BETA" + - "--device=ANDROID_13_0" - "--fail-fast" env: TEST_FIXTURE_SYMBOL_DIR: "build/fixture-r21" @@ -413,7 +413,7 @@ steps: concurrency_group: 'browserstack-app' concurrency_method: eager - - label: ':android: Android 13 Beta NDK r21 end-to-end tests - batch 2' + - label: ':android: Android 13 NDK r21 end-to-end tests - batch 2' depends_on: "fixture-r21" timeout_in_minutes: 60 plugins: @@ -430,7 +430,7 @@ steps: - "--exclude=features/full_tests/[^l-z].*.feature" - "--app=/app/build/fixture-r21.apk" - "--farm=bs" - - "--device=ANDROID_13_0_BETA" + - "--device=ANDROID_13_0" - "--fail-fast" env: TEST_FIXTURE_SYMBOL_DIR: "build/fixture-r21" From 0b70cf5cce7b08bdc010ab3fc076e057a7a9c284 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 10 Oct 2022 11:59:35 +0100 Subject: [PATCH 4/8] fix(memory): update `libunwindestack` to work around very rare crash when stack unwinding --- CHANGELOG.md | 7 +++++++ .../src/main/jni/external/libunwindstack-ndk | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f48697e18..1551c1660a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Bug fixes + +* Fixed very rare crashes when attempting to unwind NDK stacks over protected memory pages + [#1761](https://github.com/bugsnag/bugsnag-android/pull/1761) + ## 5.27.0 (2022-10-06) ### Enhancements diff --git a/bugsnag-plugin-android-ndk/src/main/jni/external/libunwindstack-ndk b/bugsnag-plugin-android-ndk/src/main/jni/external/libunwindstack-ndk index 8a8ff02b08..8f61d34602 160000 --- a/bugsnag-plugin-android-ndk/src/main/jni/external/libunwindstack-ndk +++ b/bugsnag-plugin-android-ndk/src/main/jni/external/libunwindstack-ndk @@ -1 +1 @@ -Subproject commit 8a8ff02b08fd8c152c328150e00e02ff6f7654ce +Subproject commit 8f61d34602aa0f33fea103218df29f117abb31c2 From 6293601ff6f883b81c1ebea76b3409cf4a6f7dbf Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Wed, 5 Oct 2022 10:45:09 +0200 Subject: [PATCH 5/8] Allow more breadcrumbs, and trim if payload is too big --- CHANGELOG.md | 3 + bugsnag-android-core/detekt-baseline.xml | 5 +- .../android/ManifestConfigLoaderTest.kt | 2 +- .../com/bugsnag/android/BugsnagEventMapper.kt | 6 +- .../com/bugsnag/android/ConfigInternal.kt | 2 +- .../com/bugsnag/android/Configuration.java | 6 +- .../com/bugsnag/android/DefaultDelivery.kt | 54 ++++++--- .../java/com/bugsnag/android/EventInternal.kt | 26 +++++ .../java/com/bugsnag/android/EventPayload.kt | 6 +- .../android/InternalReportDelegate.java | 7 +- .../android/internal/ImmutableConfig.kt | 2 +- .../android/internal/InternalMetrics.kt | 7 ++ .../android/internal/InternalMetricsImpl.kt | 37 +++++- .../android/internal/InternalMetricsNoop.kt | 1 + .../bugsnag/android/internal/JsonHelper.kt | 17 +++ .../bugsnag/android/BreadcrumbStateTest.kt | 2 +- .../bugsnag/android/DeliveryHeadersTest.kt | 7 +- .../java/com/bugsnag/android/DeliveryTest.kt | 2 +- .../detekt-baseline.xml | 1 - .../java/com/bugsnag/android/TestData.java | 2 +- .../cxx-scenarios-bugsnag/detekt-baseline.xml | 1 + .../src/main/cpp/cxx-scenarios-bugsnag.cpp | 14 +++ .../scenarios/EventTooBigScenario.kt | 69 +++++++++++ .../java/com/bugsnag/android/JavaHooks.java | 2 +- features/full_tests/trimming.feature | 108 ++++++++++++++++++ 25 files changed, 349 insertions(+), 40 deletions(-) create mode 100644 features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/EventTooBigScenario.kt create mode 100644 features/full_tests/trimming.feature diff --git a/CHANGELOG.md b/CHANGELOG.md index 1551c1660a..37e72e12bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ ### Enhancements +* Bugsnag now supports up to 500 breadcrumbs, with a default max of 100. Note that breadcrumbs will be trimmed + (oldest first) if the payload exceeds 1MB. + [#1751](https://github.com/bugsnag/bugsnag-android/pull/1751) * Setting `Configuration.attemptDeliveryOnCrash` will cause Bugsnag to attempt error delivery during some crashes. Use of this feature is discouraged, see the method JavaDoc for more information. [#1749](https://github.com/bugsnag/bugsnag-android/pull/1749) diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index bb85130ef8..3b91d9d356 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -15,7 +15,7 @@ LongParameterList:DeviceIdStore.kt$DeviceIdStore$( context: Context, deviceIdfile: File = File(context.filesDir, "device-id"), deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, internalDeviceIdfile: File = File(context.filesDir, "internal-device-id"), internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, private val sharedPrefMigrator: SharedPrefMigrator, logger: Logger ) LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap<String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? ) LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null ) - LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, breadcrumbs: MutableList<Breadcrumb> = mutableListOf(), discardClasses: Set<String> = setOf(), errors: MutableList<Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection<String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList<Thread> = mutableListOf(), user: User = User(), redactionKeys: Set<String>? = null ) + LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, logger: Logger, breadcrumbs: MutableList<Breadcrumb> = mutableListOf(), discardClasses: Set<String> = setOf(), errors: MutableList<Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection<String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList<Thread> = mutableListOf(), user: User = User(), redactionKeys: Set<String>? = null ) LongParameterList:EventStorageModule.kt$EventStorageModule$( contextModule: ContextModule, configModule: ConfigModule, dataCollectionModule: DataCollectionModule, bgTaskService: BackgroundTaskService, trackerModule: TrackerModule, systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState ) LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * Whether this frame identifies the program counter */ var isPC: Boolean?, /** * The type of the error */ var type: ErrorType? = null, /** * Identifies the exact build this frame originates from. */ var codeIdentifier: String? = null, ) LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy ) @@ -40,7 +40,7 @@ ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun updateSeverityInternal(severity: Severity) ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun updateSeverityReason(@SeverityReason.SeverityReasonType reason: String) RethrowCaughtException:JsonHelper.kt$JsonHelper$throw ex - ReturnCount:DefaultDelivery.kt$DefaultDelivery$fun deliver( urlString: String, streamable: JsonStream.Streamable, headers: Map<String, String?> ): DeliveryStatus + ReturnCount:DefaultDelivery.kt$DefaultDelivery$fun deliver( urlString: String, json: ByteArray, headers: Map<String, String?> ): DeliveryStatus SwallowedException:BugsnagEventMapper.kt$BugsnagEventMapper$catch (pe: IllegalArgumentException) { ndkDateFormatHolder.get()!!.parse(this) ?: throw IllegalArgumentException("cannot parse date $this") } SwallowedException:ConnectivityCompat.kt$ConnectivityLegacy$catch (e: NullPointerException) { // in some rare cases we get a remote NullPointerException via Parcel.readException null } SwallowedException:ContextExtensions.kt$catch (exc: RuntimeException) { null } @@ -48,7 +48,6 @@ SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") } SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") } SwallowedException:DeviceIdFilePersistence.kt$DeviceIdFilePersistence$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) } - SwallowedException:InternalMetrics.kt$InternalMetrics$catch (exc: Exception) { null } SwallowedException:JsonHelperTest.kt$JsonHelperTest$catch (e: IllegalArgumentException) { didThrow = true } SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null } ThrowsCount:JsonHelper.kt$JsonHelper$ fun jsonToLong(value: Any?): Long? diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt index 3ebe2f3b96..d45de42725 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ManifestConfigLoaderTest.kt @@ -45,7 +45,7 @@ class ManifestConfigLoaderTest { assertEquals(setOf("password"), redactedKeys) // misc - assertEquals(maxBreadcrumbs, 50) + assertEquals(maxBreadcrumbs, 100) assertEquals(maxPersistedEvents, 32) assertEquals(maxPersistedSessions, 128) assertEquals(maxReportedThreads, 200) 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 62d5825a30..1cfdfd90f0 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 @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.DateUtils +import com.bugsnag.android.internal.InternalMetricsImpl import java.text.DateFormat import java.text.SimpleDateFormat import java.util.Date @@ -17,7 +18,7 @@ internal class BugsnagEventMapper( @Suppress("UNCHECKED_CAST") internal fun convertToEventImpl(map: Map, apiKey: String): EventInternal { - val event = EventInternal(apiKey) + val event = EventInternal(apiKey, logger) // populate exceptions. check this early to avoid unnecessary serialization if // no stacktrace was gathered. @@ -86,6 +87,9 @@ internal class BugsnagEventMapper( event.updateSeverityReasonInternal(reason) event.normalizeStackframeErrorTypes() + // populate internalMetrics + event.internalMetrics = InternalMetricsImpl(map["usage"] as MutableMap?) + return event } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt index 1fc62957e3..3dd00de3b0 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt @@ -147,7 +147,7 @@ internal class ConfigInternal( } companion object { - private const val DEFAULT_MAX_BREADCRUMBS = 50 + private const val DEFAULT_MAX_BREADCRUMBS = 100 private const val DEFAULT_MAX_PERSISTED_SESSIONS = 128 private const val DEFAULT_MAX_PERSISTED_EVENTS = 32 private const val DEFAULT_MAX_REPORTED_THREADS = 200 diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java index d1d5436cf5..fdb638ab89 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java @@ -18,7 +18,7 @@ public class Configuration implements CallbackAware, MetadataAware, UserAware, FeatureFlagAware { private static final int MIN_BREADCRUMBS = 0; - private static final int MAX_BREADCRUMBS = 100; + private static final int MAX_BREADCRUMBS = 500; private static final int VALID_API_KEY_LEN = 32; private static final long MIN_LAUNCH_CRASH_THRESHOLD_MS = 0; @@ -512,7 +512,7 @@ public void setEndpoints(@NonNull EndpointConfiguration endpoints) { * Sets the maximum number of breadcrumbs which will be stored. Once the threshold is reached, * the oldest breadcrumbs will be deleted. * - * By default, 50 breadcrumbs are stored: this can be amended up to a maximum of 100. + * By default, 100 breadcrumbs are stored: this can be amended up to a maximum of 500. */ public int getMaxBreadcrumbs() { return impl.getMaxBreadcrumbs(); @@ -522,7 +522,7 @@ public int getMaxBreadcrumbs() { * Sets the maximum number of breadcrumbs which will be stored. Once the threshold is reached, * the oldest breadcrumbs will be deleted. * - * By default, 50 breadcrumbs are stored: this can be amended up to a maximum of 100. + * By default, 100 breadcrumbs are stored: this can be amended up to a maximum of 500. */ public void setMaxBreadcrumbs(int maxBreadcrumbs) { if (maxBreadcrumbs >= MIN_BREADCRUMBS && maxBreadcrumbs <= MAX_BREADCRUMBS) { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt index 0ce2eec8c4..5d16179949 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt @@ -1,45 +1,66 @@ package com.bugsnag.android import android.net.TrafficStats -import java.io.ByteArrayOutputStream +import com.bugsnag.android.internal.JsonHelper import java.io.IOException -import java.io.PrintWriter import java.net.HttpURLConnection import java.net.HttpURLConnection.HTTP_BAD_REQUEST import java.net.HttpURLConnection.HTTP_CLIENT_TIMEOUT import java.net.HttpURLConnection.HTTP_OK import java.net.URL -/** - * Converts a [JsonStream.Streamable] into JSON, placing it in a [ByteArray] - */ -internal fun serializeJsonPayload(streamable: JsonStream.Streamable): ByteArray { - return ByteArrayOutputStream().use { baos -> - JsonStream(PrintWriter(baos).buffered()).use(streamable::toStream) - baos.toByteArray() - } -} - internal class DefaultDelivery( private val connectivity: Connectivity?, - val logger: Logger + private val apiKey: String, + private val logger: Logger ) : Delivery { + companion object { + // 1MB with some fiddle room in case of encoding overhead + const val maxPayloadSize = 999700 + } + override fun deliver(payload: Session, deliveryParams: DeliveryParams): DeliveryStatus { - val status = deliver(deliveryParams.endpoint, payload, deliveryParams.headers) + val status = deliver( + deliveryParams.endpoint, + JsonHelper.serialize(payload), + deliveryParams.headers + ) logger.i("Session API request finished with status $status") return status } + private fun serializePayload(payload: EventPayload): ByteArray { + var json = JsonHelper.serialize(payload) + + if (json.size > maxPayloadSize) { + var event = payload.event + if (event == null) { + event = MarshalledEventSource(payload.eventFile!!, apiKey, logger).invoke() + payload.event = event + payload.apiKey = apiKey + } + val breadcrumbAndBytesRemovedCounts = + event.impl.trimBreadcrumbsBy(json.size - maxPayloadSize) + event.impl.internalMetrics.setBreadcrumbTrimMetrics( + breadcrumbAndBytesRemovedCounts.itemsTrimmed, + breadcrumbAndBytesRemovedCounts.dataTrimmed + ) + json = JsonHelper.serialize(payload) + } + return json + } + override fun deliver(payload: EventPayload, deliveryParams: DeliveryParams): DeliveryStatus { - val status = deliver(deliveryParams.endpoint, payload, deliveryParams.headers) + val json = serializePayload(payload) + val status = deliver(deliveryParams.endpoint, json, deliveryParams.headers) logger.i("Error API request finished with status $status") return status } fun deliver( urlString: String, - streamable: JsonStream.Streamable, + json: ByteArray, headers: Map ): DeliveryStatus { @@ -50,7 +71,6 @@ internal class DefaultDelivery( var conn: HttpURLConnection? = null try { - val json = serializeJsonPayload(streamable) conn = makeRequest(URL(urlString), json, headers) // End the request, get the response code 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 07ae66567f..13121f2c22 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 @@ -3,6 +3,8 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.InternalMetrics import com.bugsnag.android.internal.InternalMetricsNoop +import com.bugsnag.android.internal.JsonHelper +import com.bugsnag.android.internal.TrimMetrics import java.io.IOException internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, MetadataAware, UserAware { @@ -16,6 +18,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata featureFlags: FeatureFlags = FeatureFlags() ) : this( config.apiKey, + config.logger, mutableListOf(), config.discardClasses.toSet(), when (originalError) { @@ -34,6 +37,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata internal constructor( apiKey: String, + logger: Logger, breadcrumbs: MutableList = mutableListOf(), discardClasses: Set = setOf(), errors: MutableList = mutableListOf(), @@ -46,6 +50,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata user: User = User(), redactionKeys: Set? = null ) { + this.logger = logger this.apiKey = apiKey this.breadcrumbs = breadcrumbs this.discardClasses = discardClasses @@ -66,6 +71,7 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata val originalError: Throwable? internal var severityReason: SeverityReason + val logger: Logger val metadata: Metadata val featureFlags: FeatureFlags private val discardClasses: Set @@ -241,6 +247,26 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata fun getSeverityReasonType(): String = severityReason.severityReasonType + fun trimBreadcrumbsBy(byteCount: Int): TrimMetrics { + var removedBreadcrumbCount = 0 + var removedByteCount = 0 + while (removedByteCount < byteCount && breadcrumbs.isNotEmpty()) { + val breadcrumb = breadcrumbs.removeAt(0) + removedByteCount += JsonHelper.serialize(breadcrumb).size + removedBreadcrumbCount++ + } + when (removedBreadcrumbCount) { + 1 -> breadcrumbs.add(Breadcrumb("Removed to reduce payload size", logger)) + else -> breadcrumbs.add( + Breadcrumb( + "Removed, along with ${removedBreadcrumbCount - 1} older breadcrumbs, to reduce payload size", + logger + ) + ) + } + return TrimMetrics(removedBreadcrumbCount, removedByteCount) + } + override fun setUser(id: String?, email: String?, name: String?) { userImpl = User(id, email, name) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventPayload.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventPayload.kt index f0f70e83f9..51f213016d 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventPayload.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventPayload.kt @@ -12,17 +12,21 @@ import java.io.IOException */ class EventPayload @JvmOverloads internal constructor( var apiKey: String?, - val event: Event? = null, + event: Event? = null, internal val eventFile: File? = null, notifier: Notifier, private val config: ImmutableConfig ) : JsonStream.Streamable { + var event = event + internal set(value) { field = value } + internal val notifier = Notifier(notifier.name, notifier.version, notifier.url).apply { dependencies = notifier.dependencies.toMutableList() } internal fun getErrorTypes(): Set { + val event = this.event return when { event != null -> event.impl.getErrorTypesFromStackframes() eventFile != null -> EventFilenameInfo.fromFile(eventFile, config).errorTypes diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java index b299ac201c..d4001aa3e5 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java @@ -4,6 +4,7 @@ import static com.bugsnag.android.SeverityReason.REASON_UNHANDLED_EXCEPTION; import com.bugsnag.android.internal.ImmutableConfig; +import com.bugsnag.android.internal.JsonHelper; import android.annotation.SuppressLint; import android.content.Context; @@ -121,7 +122,11 @@ public void run() { headers.put(HEADER_INTERNAL_ERROR, "bugsnag-android"); headers.remove(DeliveryHeadersKt.HEADER_API_KEY); DefaultDelivery defaultDelivery = (DefaultDelivery) delivery; - defaultDelivery.deliver(params.getEndpoint(), payload, headers); + defaultDelivery.deliver( + params.getEndpoint(), + JsonHelper.INSTANCE.serialize(payload), + headers + ); } } catch (Exception exception) { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index 1cde96634d..73b340f525 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -222,7 +222,7 @@ internal fun sanitiseConfiguration( @Suppress("SENSELESS_COMPARISON") if (configuration.delivery == null) { - configuration.delivery = DefaultDelivery(connectivity, configuration.logger!!) + configuration.delivery = DefaultDelivery(connectivity, configuration.apiKey, configuration.logger!!) } return convertToImmutableConfig( configuration, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt index ce83ac5153..1521aed114 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt @@ -16,4 +16,11 @@ interface InternalMetrics { fun notifyAddCallback(callback: String) fun notifyRemoveCallback(callback: String) + + fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) } + +internal data class TrimMetrics( + val itemsTrimmed: Int, // breadcrumbs, strings, whatever + val dataTrimmed: Int // chars, bytes, whatever +) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt index 640a874f6a..27c5e3472e 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt @@ -2,16 +2,42 @@ package com.bugsnag.android.internal import com.bugsnag.android.NdkPluginCaller -class InternalMetricsImpl : InternalMetrics { - private val configDifferences = hashMapOf() - private val callbackCounts = hashMapOf() +class InternalMetricsImpl(source: Map? = null) : InternalMetrics { + private val configDifferences: MutableMap + private val callbackCounts: MutableMap + private var breadcrumbsRemovedCount = 0 + private var breadcrumbBytesRemovedCount = 0 + + init { + if (source != null) { + @Suppress("UNCHECKED_CAST") + configDifferences = (source["config"] as MutableMap?) ?: hashMapOf() + @Suppress("UNCHECKED_CAST") + callbackCounts = (source["callbacks"] as MutableMap?) ?: hashMapOf() + @Suppress("UNCHECKED_CAST") + val system = source["system"] as MutableMap? + if (system != null) { + breadcrumbsRemovedCount = (system["breadcrumbsRemovedCount"] as Number?)?.toInt() ?: 0 + breadcrumbBytesRemovedCount = (system["breadcrumbBytesRemoved"] as Number?)?.toInt() ?: 0 + } + } else { + configDifferences = hashMapOf() + callbackCounts = hashMapOf() + } + } override fun toJsonableMap(): Map { val callbacks = allCallbacks() + val system = listOfNotNull( + if (breadcrumbsRemovedCount > 0) "breadcrumbsRemoved" to breadcrumbsRemovedCount else null, + if (breadcrumbBytesRemovedCount > 0) "breadcrumbBytesRemoved" to breadcrumbBytesRemovedCount else null, + ).toMap() + return listOfNotNull( if (configDifferences.isNotEmpty()) "config" to configDifferences else null, if (callbacks.isNotEmpty()) "callbacks" to callbacks else null, + if (system.isNotEmpty()) "system" to system else null, ).toMap() } @@ -66,4 +92,9 @@ class InternalMetricsImpl : InternalMetrics { return result } + + override fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) { + breadcrumbsRemovedCount = breadcrumbsRemoved + breadcrumbBytesRemovedCount = bytesRemoved + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt index bbf0384e7b..d94f20e49b 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt @@ -6,4 +6,5 @@ class InternalMetricsNoop : InternalMetrics { override fun setCallbackCounts(newCallbackCounts: Map) = Unit override fun notifyAddCallback(callback: String) = Unit override fun notifyRemoveCallback(callback: String) = Unit + override fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) = Unit } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/JsonHelper.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/JsonHelper.kt index 6e69f71c6c..2453e35974 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/JsonHelper.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/JsonHelper.kt @@ -1,7 +1,9 @@ package com.bugsnag.android.internal +import com.bugsnag.android.JsonStream import com.bugsnag.android.repackaged.dslplatform.json.DslJson import com.bugsnag.android.repackaged.dslplatform.json.JsonWriter +import java.io.ByteArrayOutputStream import java.io.File import java.io.FileInputStream import java.io.FileNotFoundException @@ -9,6 +11,7 @@ import java.io.FileOutputStream import java.io.IOException import java.io.InputStream import java.io.OutputStream +import java.io.PrintWriter import java.util.Date internal object JsonHelper { @@ -31,6 +34,20 @@ internal object JsonHelper { } } + fun serialize(streamable: JsonStream.Streamable): ByteArray { + return ByteArrayOutputStream().use { baos -> + JsonStream(PrintWriter(baos)).use(streamable::toStream) + baos.toByteArray() + } + } + + fun serialize(value: Any): ByteArray { + return ByteArrayOutputStream().use { baos -> + serialize(value, baos) + baos.toByteArray() + } + } + fun serialize(value: Any, stream: OutputStream) { dslJson.serialize(value, stream) } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbStateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbStateTest.kt index 30ecf3b8b9..bac40ff033 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbStateTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/BreadcrumbStateTest.kt @@ -196,7 +196,7 @@ class BreadcrumbStateTest { @Test fun testMaxBreadcrumbAccessors() { val config = generateConfiguration() - assertEquals(50, config.maxBreadcrumbs) + assertEquals(100, config.maxBreadcrumbs) config.maxBreadcrumbs = 25 assertEquals(25, config.maxBreadcrumbs) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryHeadersTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryHeadersTest.kt index f578bca911..46259c7eaa 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryHeadersTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryHeadersTest.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import com.bugsnag.android.BugsnagTestUtils.generateConfiguration import com.bugsnag.android.BugsnagTestUtils.generateEventPayload import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig +import com.bugsnag.android.internal.JsonHelper import com.bugsnag.android.internal.convertToImmutableConfig import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals @@ -19,9 +20,9 @@ class DeliveryHeadersTest { @Test fun computeSha1Digest() { val payload = generateEventPayload(generateImmutableConfig()) - val payload1 = serializeJsonPayload(payload) + val payload1 = JsonHelper.serialize(payload) val firstSha = requireNotNull(computeSha1Digest(payload1)) - val payload2 = serializeJsonPayload(payload) + val payload2 = JsonHelper.serialize(payload) val secondSha = requireNotNull(computeSha1Digest(payload2)) // the hash equals the expected value @@ -32,7 +33,7 @@ class DeliveryHeadersTest { // altering the streamable alters the hash payload.event!!.device.id = "50923" - val payload3 = serializeJsonPayload(payload) + val payload3 = JsonHelper.serialize(payload) val differentSha = requireNotNull(computeSha1Digest(payload3)) assertNotEquals(firstSha, differentSha) assertTrue(differentSha.matches(sha1Regex)) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt index 243bc15a38..b518a4b787 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt @@ -7,7 +7,7 @@ class DeliveryTest { @Test fun testResponseCodeMapping() { - val delivery = DefaultDelivery(null, NoopLogger) + val delivery = DefaultDelivery(null, "myApiKey", NoopLogger) assertEquals(DeliveryStatus.DELIVERED, delivery.getDeliveryStatus(202)) assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(503)) assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(0)) diff --git a/bugsnag-plugin-android-ndk/detekt-baseline.xml b/bugsnag-plugin-android-ndk/detekt-baseline.xml index 266786cb4c..23563b755e 100644 --- a/bugsnag-plugin-android-ndk/detekt-baseline.xml +++ b/bugsnag-plugin-android-ndk/detekt-baseline.xml @@ -13,7 +13,6 @@ LongMethod:EventMigrationV9Tests.kt$EventMigrationV9Tests$@Test fun testMigrateEventToLatest() LongParameterList:NativeBridge.kt$NativeBridge$( apiKey: String, reportingDirectory: String, lastRunInfoPath: String, consecutiveLaunchCrashes: Int, autoDetectNdkCrashes: Boolean, apiLevel: Int, is32bit: Boolean, threadSendPolicy: Int ) NestedBlockDepth:NativeBridge.kt$NativeBridge$private fun deliverPendingReports() - SwallowedException:NdkPlugin.kt$NdkPlugin$catch (exc: IOException) { // Ignore } TooManyFunctions:NativeBridge.kt$NativeBridge : StateObserver diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java index 4408361a25..371b4dfcc8 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java @@ -31,7 +31,7 @@ static ImmutableConfig generateConfig() throws IOException { "1.4.3", 55, "android", - new DefaultDelivery(null, NoopLogger.INSTANCE), + new DefaultDelivery(null, "myApiKey", NoopLogger.INSTANCE), new EndpointConfiguration(), true, 55, diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml index 48d0c94c28..66b0a1c032 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml @@ -7,6 +7,7 @@ MagicNumber:CXXExceptionWithUsageScenario.kt$CXXExceptionWithUsageScenario$10 MagicNumber:CXXExceptionWithUsageScenario.kt$CXXExceptionWithUsageScenario$1000 MagicNumber:CXXSigsegvWithUsageScenario.kt$CXXSigsegvWithUsageScenario$10 + MagicNumber:EventTooBigScenario.kt$EventTooBigScenario$3 MagicNumber:HandledExceptionWithUsageScenario.kt$HandledExceptionWithUsageScenario$10 MagicNumber:UnhandledExceptionWithUsageScenario.kt$UnhandledExceptionWithUsageScenario$10 diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp index 151ccdcfbe..092b34d2a7 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp @@ -378,4 +378,18 @@ Java_com_bugsnag_android_mazerunner_scenarios_UnhandledExceptionWithUsageScenari bugsnag_add_on_error(on_error_do_nothing); } +extern "C" +JNIEXPORT jint JNICALL +Java_com_bugsnag_android_mazerunner_scenarios_EventTooBigScenario_nativeCrash(JNIEnv *env, + jobject thiz, + jint value) { + int x = 38; + if (value > 0) { + raise(SIGSEGV); + } + printf("Phasers locked onto their warp core, captain." + " Please, please let me shoot their warp core! I have been very good this month!\n"); + return value / x / 8; +} + } diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/EventTooBigScenario.kt b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/EventTooBigScenario.kt new file mode 100644 index 0000000000..54395b0196 --- /dev/null +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/EventTooBigScenario.kt @@ -0,0 +1,69 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration +import com.bugsnag.android.Telemetry + +class EventTooBigScenario( + config: Configuration, + context: Context, + eventMetadata: String +) : Scenario(config, context, eventMetadata) { + + val breadcrumbSize: Int + val breadcrumbCount: Int + val crashType: String + + init { + System.loadLibrary("bugsnag-ndk") + System.loadLibrary("cxx-scenarios-bugsnag") + + if (eventMetadata.isEmpty()) { + crashType = "error: unconfigured" + breadcrumbSize = 1 + breadcrumbCount = 1 + } else { + // Args: crash type (handled, java, native), breadcrumbSize (int), breadcrumbCount (int) + val args = eventMetadata.split(",").map { it.trim() } + val expectedArgs = 3 + if (args.size != expectedArgs) { + throw IllegalArgumentException( + "Maze Runner configuration error: Expected $expectedArgs arguments in" + + " eventMetadata but got ${args.size}" + ) + } + + crashType = args[0] + breadcrumbSize = args[1].toInt() + breadcrumbCount = args[2].toInt() + config.setTelemetry(config.getTelemetry() + Telemetry.USAGE) + } + } + + external fun nativeCrash(value: Int): Int + + private fun jvmCrash() { + listOf()[0] + } + + private fun crash() { + when (crashType) { + "handled" -> Bugsnag.notify(generateException()) + "jvm" -> jvmCrash() + "native" -> nativeCrash(1) + else -> throw IllegalArgumentException( + "Maze Runner configuration error: Unknown crash type \"${crashType}\"" + ) + } + } + + override fun startScenario() { + super.startScenario() + + repeat(breadcrumbCount) { + Bugsnag.leaveBreadcrumb("$it" + "*".repeat(breadcrumbSize)) + } + crash() + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java index a7fadea1fc..f6b024999d 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java @@ -53,6 +53,6 @@ public DeliveryStatus deliver(@NonNull Session payload, @NonNull public static Delivery createDefaultDelivery() { - return new DefaultDelivery(null, NoopLogger.INSTANCE); + return new DefaultDelivery(null, "test-api-key", NoopLogger.INSTANCE); } } diff --git a/features/full_tests/trimming.feature b/features/full_tests/trimming.feature new file mode 100644 index 0000000000..6896485a09 --- /dev/null +++ b/features/full_tests/trimming.feature @@ -0,0 +1,108 @@ +Feature: Excess data is trimmed when the payload is too big + + Background: + Given I clear all persistent data + + # This scenario may flake if the payload structure changes significantly because it relies upon overflowing + # the max payload size by a certain amount. If it fails after a structural change, verify + # maze_output/failed/Payload_is_too_big_by_3_breadcrumbs/errors.log to make sure it's behaving as expected, + # and then modify "When I configure the app to run in the "100" state" to a number that generates 3 breadcrumbs + # worth of data too much. + + Scenario: Payload is too big by 3 breadcrumbs, handled exception that failed initial delivery + When I set the HTTP status code for the next request to 500 + And I configure the app to run in the "handled, 10000, 100" state + And I run "EventTooBigScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "EventTooBigScenario" + And the event has 98 breadcrumbs + And the event "breadcrumbs.97.name" equals "Removed, along with 2 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 3 + And the event "usage.system.breadcrumbBytesRemoved" is not null + And I close and relaunch the app + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "EventTooBigScenario" + And the event has 98 breadcrumbs + And the event "breadcrumbs.97.name" equals "Removed, along with 2 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 3 + And the event "usage.system.breadcrumbBytesRemoved" is not null + + Scenario: Payload is too big by 3 breadcrumbs, handled exception + When I configure the app to run in the "handled, 10000, 100" state + And I run "EventTooBigScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "EventTooBigScenario" + And the event has 98 breadcrumbs + And the event "breadcrumbs.97.name" equals "Removed, along with 2 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 3 + And the event "usage.system.breadcrumbBytesRemoved" is not null + + Scenario: Payload is too big by 3 breadcrumbs, jvm exception + When I configure the app to run in the "jvm, 10000, 100" state + And I run "EventTooBigScenario" and relaunch the crashed app + And I configure the app to run in the "none, 10000, 100" state + And I configure Bugsnag for "EventTooBigScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "Empty list doesn't contain element at index 0." + And the event has 98 breadcrumbs + And the event "breadcrumbs.97.name" equals "Removed, along with 2 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 3 + And the event "usage.system.breadcrumbBytesRemoved" is not null + + # Note: Disabled until native hard limits are removed. + # Scenario: Payload is too big by 3 breadcrumbs, native crash + # When I configure the app to run in the "native, 10000, 100" state + # And I run "EventTooBigScenario" and relaunch the crashed app + # And I configure the app to run in the "none, 10000, 100" state + # And I configure Bugsnag for "EventTooBigScenario" + # Then I wait to receive an error + # And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + # And the exception "message" equals "Segmentation violation (invalid memory reference)" + # And the event has 98 breadcrumbs + # And the event "breadcrumbs.97.name" equals "Removed, along with 2 older breadcrumbs, to reduce payload size" + # And the event "usage.system.breadcrumbsRemoved" equals 3 + # And the event "usage.system.breadcrumbBytesRemoved" is not null + + # =========================================================================== + + Scenario: Breadcrumb is too big, handled exception + When I configure the app to run in the "handled, 1100000, 1" state + And I run "EventTooBigScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "EventTooBigScenario" + And the event has 1 breadcrumbs + And the event "breadcrumbs.0.name" equals "Removed, along with 1 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 2 + And the event "usage.system.breadcrumbBytesRemoved" is not null + + Scenario: Breadcrumb is too big, jvm exception + When I configure the app to run in the "jvm, 1100000, 1" state + And I run "EventTooBigScenario" and relaunch the crashed app + And I configure the app to run in the "none, 1100000, 1" state + And I configure Bugsnag for "EventTooBigScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "Empty list doesn't contain element at index 0." + And the event has 1 breadcrumbs + And the event "breadcrumbs.0.name" equals "Removed, along with 1 older breadcrumbs, to reduce payload size" + And the event "usage.system.breadcrumbsRemoved" equals 2 + And the event "usage.system.breadcrumbBytesRemoved" is not null + + # Note: Disabled until native hard limits are removed. + # Scenario: Breadcrumb is too big, native crash + # When I configure the app to run in the "native, 1100000, 1" state + # And I run "EventTooBigScenario" and relaunch the crashed app + # And I configure the app to run in the "none, 1100000, 1" state + # And I configure Bugsnag for "EventTooBigScenario" + # Then I wait to receive an error + # And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + # And the exception "message" equals "Segmentation violation (invalid memory reference)" + # And the event has 1 breadcrumbs + # And the event "breadcrumbs.0.name" equals "Removed, along with 1 older breadcrumbs, to reduce payload size" + # And the event "usage.system.breadcrumbsRemoved" equals 2 + # And the event "usage.system.breadcrumbBytesRemoved" is not null From 22c5413cb1ad5ae732e6c8254bee3871c504ada9 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Thu, 6 Oct 2022 09:32:16 +0200 Subject: [PATCH 6/8] Also trim metadata when the payload is too big --- .../com/bugsnag/android/BreadcrumbInternal.kt | 7 ++ .../com/bugsnag/android/ConfigInternal.kt | 2 + .../com/bugsnag/android/Configuration.java | 26 +++++++ .../com/bugsnag/android/DefaultDelivery.kt | 43 ++++++++---- .../java/com/bugsnag/android/EventInternal.kt | 15 ++++ .../main/java/com/bugsnag/android/Metadata.kt | 13 ++++ .../android/internal/ImmutableConfig.kt | 7 +- .../android/internal/InternalMetrics.kt | 2 + .../android/internal/InternalMetricsImpl.kt | 11 +++ .../android/internal/InternalMetricsNoop.kt | 1 + .../bugsnag/android/internal/StringUtils.kt | 42 +++++++++++ .../java/com/bugsnag/android/DeliveryTest.kt | 2 +- .../java/com/bugsnag/android/TestData.java | 2 +- .../cxx-scenarios-bugsnag/detekt-baseline.xml | 2 + .../src/main/cpp/cxx-scenarios-bugsnag.cpp | 12 ++++ .../MetadataStringsTooLargeScenario.kt | 69 +++++++++++++++++++ .../java/com/bugsnag/android/JavaHooks.java | 2 +- features/full_tests/trimming.feature | 40 +++++++++++ features/steps/breadcrumb_metadata_steps.rb | 5 ++ 19 files changed, 284 insertions(+), 19 deletions(-) create mode 100644 bugsnag-android-core/src/main/java/com/bugsnag/android/internal/StringUtils.kt create mode 100644 features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/MetadataStringsTooLargeScenario.kt diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbInternal.kt index 49499b770d..be7f0c4435 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BreadcrumbInternal.kt @@ -1,5 +1,7 @@ package com.bugsnag.android +import com.bugsnag.android.internal.StringUtils +import com.bugsnag.android.internal.TrimMetrics import java.io.IOException import java.util.Date @@ -22,6 +24,11 @@ internal class BreadcrumbInternal internal constructor( Date() ) + internal fun trimMetadataStringsTo(maxStringLength: Int): TrimMetrics { + val metadata = this.metadata ?: return TrimMetrics(0, 0) + return StringUtils.trimNullableStringValuesTo(maxStringLength, metadata) + } + @Throws(IOException::class) override fun toStream(writer: JsonStream) { writer.beginObject() diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt index 3dd00de3b0..bd1e449895 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt @@ -42,6 +42,7 @@ internal class ConfigInternal( var maxPersistedEvents: Int = DEFAULT_MAX_PERSISTED_EVENTS var maxPersistedSessions: Int = DEFAULT_MAX_PERSISTED_SESSIONS var maxReportedThreads: Int = DEFAULT_MAX_REPORTED_THREADS + var maxStringValueLength: Int = DEFAULT_MAX_STRING_VALUE_LENGTH var context: String? = null var redactedKeys: Set @@ -152,6 +153,7 @@ internal class ConfigInternal( private const val DEFAULT_MAX_PERSISTED_EVENTS = 32 private const val DEFAULT_MAX_REPORTED_THREADS = 200 private const val DEFAULT_LAUNCH_CRASH_THRESHOLD_MS: Long = 5000 + private const val DEFAULT_MAX_STRING_VALUE_LENGTH = 10000 @JvmStatic fun load(context: Context): Configuration = load(context, null) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java index fdb638ab89..56f987d4cc 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java @@ -612,6 +612,32 @@ public void setMaxPersistedSessions(int maxPersistedSessions) { } } + /** + * Gets the maximum string length in any metadata field. Once the threshold is + * reached in a particular string, all excess characters will be deleted. + * + * By default, the limit is 10,000. + */ + public int getMaxStringValueLength() { + return impl.getMaxStringValueLength(); + } + + /** + * Sets the maximum string length in any metadata field. Once the threshold is + * reached in a particular string, all excess characters will be deleted. + * + * By default, the limit is 10,000. + */ + public void setMaxStringValueLength(int maxStringValueLength) { + if (maxStringValueLength >= 0) { + impl.setMaxStringValueLength(maxStringValueLength); + } else { + getLogger().e("Invalid configuration value detected. " + + "Option maxStringValueLength should be a positive integer." + + "Supplied value is " + maxStringValueLength); + } + } + /** * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts * represent what was happening in your application at the time an error occurs. diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt index 5d16179949..72c8a9200f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt @@ -12,6 +12,7 @@ import java.net.URL internal class DefaultDelivery( private val connectivity: Connectivity?, private val apiKey: String, + private val maxStringValueLength: Int, private val logger: Logger ) : Delivery { @@ -32,23 +33,35 @@ internal class DefaultDelivery( private fun serializePayload(payload: EventPayload): ByteArray { var json = JsonHelper.serialize(payload) + if (json.size <= maxPayloadSize) { + return json + } - if (json.size > maxPayloadSize) { - var event = payload.event - if (event == null) { - event = MarshalledEventSource(payload.eventFile!!, apiKey, logger).invoke() - payload.event = event - payload.apiKey = apiKey - } - val breadcrumbAndBytesRemovedCounts = - event.impl.trimBreadcrumbsBy(json.size - maxPayloadSize) - event.impl.internalMetrics.setBreadcrumbTrimMetrics( - breadcrumbAndBytesRemovedCounts.itemsTrimmed, - breadcrumbAndBytesRemovedCounts.dataTrimmed - ) - json = JsonHelper.serialize(payload) + var event = payload.event + if (event == null) { + event = MarshalledEventSource(payload.eventFile!!, apiKey, logger).invoke() + payload.event = event + payload.apiKey = apiKey } - return json + + val (itemsTrimmed, dataTrimmed) = event.impl.trimMetadataStringsTo(maxStringValueLength) + event.impl.internalMetrics.setMetadataTrimMetrics( + itemsTrimmed, + dataTrimmed + ) + + json = JsonHelper.serialize(payload) + if (json.size <= maxPayloadSize) { + return json + } + + val breadcrumbAndBytesRemovedCounts = + event.impl.trimBreadcrumbsBy(json.size - maxPayloadSize) + event.impl.internalMetrics.setBreadcrumbTrimMetrics( + breadcrumbAndBytesRemovedCounts.itemsTrimmed, + breadcrumbAndBytesRemovedCounts.dataTrimmed + ) + return JsonHelper.serialize(payload) } override fun deliver(payload: EventPayload, deliveryParams: DeliveryParams): DeliveryStatus { 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 13121f2c22..a7224b2f93 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 @@ -247,6 +247,21 @@ internal class EventInternal : FeatureFlagAware, JsonStream.Streamable, Metadata fun getSeverityReasonType(): String = severityReason.severityReasonType + fun trimMetadataStringsTo(maxLength: Int): TrimMetrics { + var stringCount = 0 + var charCount = 0 + + var stringAndCharCounts = metadata.trimMetadataStringsTo(maxLength) + stringCount += stringAndCharCounts.itemsTrimmed + charCount += stringAndCharCounts.dataTrimmed + for (breadcrumb in breadcrumbs) { + stringAndCharCounts = breadcrumb.impl.trimMetadataStringsTo(maxLength) + stringCount += stringAndCharCounts.itemsTrimmed + charCount += stringAndCharCounts.dataTrimmed + } + return TrimMetrics(stringCount, charCount) + } + fun trimBreadcrumbsBy(byteCount: Int): TrimMetrics { var removedBreadcrumbCount = 0 var removedByteCount = 0 diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt index 4dc8a51668..070ed31055 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt @@ -2,6 +2,8 @@ package com.bugsnag.android +import com.bugsnag.android.internal.StringUtils +import com.bugsnag.android.internal.TrimMetrics import java.io.IOException import java.util.concurrent.ConcurrentHashMap @@ -137,4 +139,15 @@ internal data class Metadata @JvmOverloads constructor( return this.copy(store = toMap()) .also { it.redactedKeys = redactedKeys.toSet() } } + + fun trimMetadataStringsTo(maxStringLength: Int): TrimMetrics { + var stringCount = 0 + var charCount = 0 + store.forEach { entry -> + val stringAndCharCounts = StringUtils.trimStringValuesTo(maxStringLength, entry.value) + stringCount += stringAndCharCounts.itemsTrimmed + charCount += stringAndCharCounts.dataTrimmed + } + return TrimMetrics(stringCount, charCount) + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index 73b340f525..07f2dcb835 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -222,7 +222,12 @@ internal fun sanitiseConfiguration( @Suppress("SENSELESS_COMPARISON") if (configuration.delivery == null) { - configuration.delivery = DefaultDelivery(connectivity, configuration.apiKey, configuration.logger!!) + configuration.delivery = DefaultDelivery( + connectivity, + configuration.apiKey, + configuration.maxStringValueLength, + configuration.logger!! + ) } return convertToImmutableConfig( configuration, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt index 1521aed114..0fbdb53f63 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetrics.kt @@ -17,6 +17,8 @@ interface InternalMetrics { fun notifyRemoveCallback(callback: String) + fun setMetadataTrimMetrics(stringsTrimmed: Int, charsRemoved: Int) + fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt index 27c5e3472e..6e648e5f48 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsImpl.kt @@ -5,6 +5,8 @@ import com.bugsnag.android.NdkPluginCaller class InternalMetricsImpl(source: Map? = null) : InternalMetrics { private val configDifferences: MutableMap private val callbackCounts: MutableMap + private var metadataStringsTrimmedCount = 0 + private var metadataCharsTruncatedCount = 0 private var breadcrumbsRemovedCount = 0 private var breadcrumbBytesRemovedCount = 0 @@ -17,6 +19,8 @@ class InternalMetricsImpl(source: Map? = null) : InternalMetrics { @Suppress("UNCHECKED_CAST") val system = source["system"] as MutableMap? if (system != null) { + metadataStringsTrimmedCount = (system["stringsTruncated"] as Number?)?.toInt() ?: 0 + metadataCharsTruncatedCount = (system["stringCharsTruncated"] as Number?)?.toInt() ?: 0 breadcrumbsRemovedCount = (system["breadcrumbsRemovedCount"] as Number?)?.toInt() ?: 0 breadcrumbBytesRemovedCount = (system["breadcrumbBytesRemoved"] as Number?)?.toInt() ?: 0 } @@ -30,6 +34,8 @@ class InternalMetricsImpl(source: Map? = null) : InternalMetrics { val callbacks = allCallbacks() val system = listOfNotNull( + if (metadataStringsTrimmedCount > 0) "stringsTruncated" to metadataStringsTrimmedCount else null, + if (metadataCharsTruncatedCount > 0) "stringCharsTruncated" to metadataCharsTruncatedCount else null, if (breadcrumbsRemovedCount > 0) "breadcrumbsRemoved" to breadcrumbsRemovedCount else null, if (breadcrumbBytesRemovedCount > 0) "breadcrumbBytesRemoved" to breadcrumbBytesRemovedCount else null, ).toMap() @@ -93,6 +99,11 @@ class InternalMetricsImpl(source: Map? = null) : InternalMetrics { return result } + override fun setMetadataTrimMetrics(stringsTrimmed: Int, charsRemoved: Int) { + metadataStringsTrimmedCount = stringsTrimmed + metadataCharsTruncatedCount = charsRemoved + } + override fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) { breadcrumbsRemovedCount = breadcrumbsRemoved breadcrumbBytesRemovedCount = bytesRemoved diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt index d94f20e49b..743ef8d20d 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/InternalMetricsNoop.kt @@ -6,5 +6,6 @@ class InternalMetricsNoop : InternalMetrics { override fun setCallbackCounts(newCallbackCounts: Map) = Unit override fun notifyAddCallback(callback: String) = Unit override fun notifyRemoveCallback(callback: String) = Unit + override fun setMetadataTrimMetrics(stringsTrimmed: Int, charsRemoved: Int) = Unit override fun setBreadcrumbTrimMetrics(breadcrumbsRemoved: Int, bytesRemoved: Int) = Unit } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/StringUtils.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/StringUtils.kt new file mode 100644 index 0000000000..3dbabc296f --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/StringUtils.kt @@ -0,0 +1,42 @@ +package com.bugsnag.android.internal + +internal object StringUtils { + val trimMessageLength = "***<9> CHARS TRUNCATED***".length + fun stringTrimmedTo(maxLength: Int, str: String): String { + val excessCharCount = str.length - maxLength + return when { + excessCharCount < trimMessageLength -> str + else -> "${str.substring(0, maxLength)}***<$excessCharCount> CHARS TRUNCATED***" + } + } + + fun trimNullableStringValuesTo(maxStringLength: Int, map: MutableMap): TrimMetrics { + var stringCount = 0 + var charCount = 0 + for (entry in map.entries) { + val value = entry.value + if (value is String && value.length > maxStringLength) { + entry.setValue(stringTrimmedTo(maxStringLength, value)) + charCount += value.length - maxStringLength + stringCount++ + } + } + + return TrimMetrics(stringCount, charCount) + } + + fun trimStringValuesTo(maxStringLength: Int, map: MutableMap): TrimMetrics { + var stringCount = 0 + var charCount = 0 + for (entry in map.entries) { + val value = entry.value + if (value is String && value.length > maxStringLength) { + entry.setValue(stringTrimmedTo(maxStringLength, value)) + charCount += value.length - maxStringLength + stringCount++ + } + } + + return TrimMetrics(stringCount, charCount) + } +} diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt index b518a4b787..b39dfb1364 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt @@ -7,7 +7,7 @@ class DeliveryTest { @Test fun testResponseCodeMapping() { - val delivery = DefaultDelivery(null, "myApiKey", NoopLogger) + val delivery = DefaultDelivery(null, "myApiKey", 10000, NoopLogger) assertEquals(DeliveryStatus.DELIVERED, delivery.getDeliveryStatus(202)) assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(503)) assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(0)) diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java index 371b4dfcc8..67bd5d4e16 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java @@ -31,7 +31,7 @@ static ImmutableConfig generateConfig() throws IOException { "1.4.3", 55, "android", - new DefaultDelivery(null, "myApiKey", NoopLogger.INSTANCE), + new DefaultDelivery(null, "myApiKey", 10000, NoopLogger.INSTANCE), new EndpointConfiguration(), true, 55, diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml index 66b0a1c032..1c1387a874 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml @@ -9,6 +9,8 @@ MagicNumber:CXXSigsegvWithUsageScenario.kt$CXXSigsegvWithUsageScenario$10 MagicNumber:EventTooBigScenario.kt$EventTooBigScenario$3 MagicNumber:HandledExceptionWithUsageScenario.kt$HandledExceptionWithUsageScenario$10 + MagicNumber:MetadataStringsTooLargeScenario.kt$MetadataStringsTooLargeScenario$3 + MagicNumber:MetadataStringsTooLargeScenario.kt$MetadataStringsTooLargeScenario$995000 MagicNumber:UnhandledExceptionWithUsageScenario.kt$UnhandledExceptionWithUsageScenario$10 diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp index 092b34d2a7..7a95d11486 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp @@ -392,4 +392,16 @@ Java_com_bugsnag_android_mazerunner_scenarios_EventTooBigScenario_nativeCrash(JN return value / x / 8; } +extern "C" +JNIEXPORT jint JNICALL +Java_com_bugsnag_android_mazerunner_scenarios_MetadataStringsTooLargeScenario_nativeCrash( + JNIEnv *env, jobject thiz, jint value) { + int x = 38; + if (value > 0) { + raise(SIGSEGV); + } + printf("SHE FOLDS! You all fold! Every time, you all fold! You fold!\n"); + return value / x / 8; +} + } diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/MetadataStringsTooLargeScenario.kt b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/MetadataStringsTooLargeScenario.kt new file mode 100644 index 0000000000..c35b83cb70 --- /dev/null +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/MetadataStringsTooLargeScenario.kt @@ -0,0 +1,69 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.BreadcrumbType +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration + +class MetadataStringsTooLargeScenario( + config: Configuration, + context: Context, + eventMetadata: String +) : Scenario(config, context, eventMetadata) { + + val metadataTestValue: String + val crashType: String + + init { + System.loadLibrary("bugsnag-ndk") + System.loadLibrary("cxx-scenarios-bugsnag") + + if (eventMetadata.isEmpty()) { + crashType = "error: unconfigured" + metadataTestValue = "" + } else { + // Args: crash type (handled, java, native), maxStringValue (int), testStringLength (int) + val args = eventMetadata.split(",").map { it.trim() } + val expectedArgs = 3 + if (args.size != expectedArgs) { + throw IllegalArgumentException( + "Maze Runner configuration error: Expected $expectedArgs arguments in" + + " eventMetadata but got ${args.size}: $args" + ) + } + + crashType = args[0] + config.maxStringValueLength = args[1].toInt() + metadataTestValue = "0".repeat(args[2].toInt()) + } + } + + external fun nativeCrash(value: Int): Int + + private fun jvmCrash() { + listOf()[0] + } + + private fun crash() { + when (crashType) { + "handled" -> Bugsnag.notify(generateException()) + "jvm" -> jvmCrash() + "native" -> nativeCrash(1) + else -> throw IllegalArgumentException( + "Maze Runner configuration error: Unknown crash type \"${crashType}\"" + ) + } + } + + override fun startScenario() { + super.startScenario() + Bugsnag.addMetadata("custom", "foo", metadataTestValue) + Bugsnag.leaveBreadcrumb("big" + "*".repeat(995000)) + Bugsnag.leaveBreadcrumb( + "test", + mutableMapOf("a" to metadataTestValue), + BreadcrumbType.MANUAL + ) + crash() + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java index f6b024999d..432f81aa2f 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/JavaHooks.java @@ -53,6 +53,6 @@ public DeliveryStatus deliver(@NonNull Session payload, @NonNull public static Delivery createDefaultDelivery() { - return new DefaultDelivery(null, "test-api-key", NoopLogger.INSTANCE); + return new DefaultDelivery(null, "test-api-key", 10000, NoopLogger.INSTANCE); } } diff --git a/features/full_tests/trimming.feature b/features/full_tests/trimming.feature index 6896485a09..938aa8f384 100644 --- a/features/full_tests/trimming.feature +++ b/features/full_tests/trimming.feature @@ -3,6 +3,46 @@ Feature: Excess data is trimmed when the payload is too big Background: Given I clear all persistent data + Scenario: metadata truncated, handled exception + When I configure the app to run in the "handled, 100, 1000" state + And I run "MetadataStringsTooLargeScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "MetadataStringsTooLargeScenario" + And the event "metaData.custom.foo" matches ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + And the breadcrumb named "test" has "metaData.a" matching ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + And the event "usage.system.stringsTruncated" equals 2 + And the event "usage.system.stringCharsTruncated" is greater than 0 + + Scenario: metadata truncated, JVM exception + When I configure the app to run in the "jvm, 100, 1000" state + And I run "MetadataStringsTooLargeScenario" and relaunch the crashed app + And I configure the app to run in the "none, 100, 1000" state + And I configure Bugsnag for "MetadataStringsTooLargeScenario" + Then I wait to receive an error + And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the exception "message" equals "Empty list doesn't contain element at index 0." + And the event "metaData.custom.foo" matches ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + And the breadcrumb named "test" has "metaData.a" matching ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + And the event "usage.system.stringsTruncated" equals 2 + And the event "usage.system.stringCharsTruncated" is greater than 0 + + # Note: Disabled until native hard limits are removed. + # Scenario: metadata truncated, native exception + # When I configure the app to run in the "native, 100, 1000" state + # And I run "MetadataStringsTooLargeScenario" and relaunch the crashed app + # And I configure the app to run in the "none, 100, 1000" state + # And I configure Bugsnag for "MetadataStringsTooLargeScenario" + # Then I wait to receive an error + # And the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + # And the exception "message" equals "Segmentation violation (invalid memory reference)" + # And the event "metaData.custom.foo" matches ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + # And the breadcrumb named "test" has "metaData.a" matching ".*\*\*\*<\d+> CHARS? TRUNCATED\*\*\*" + # And the event "usage.system.stringsTruncated" equals 2 + # And the event "usage.system.stringCharsTruncated" is greater than 0 + + # =========================================================================== + # This scenario may flake if the payload structure changes significantly because it relies upon overflowing # the max payload size by a certain amount. If it fails after a structural change, verify # maze_output/failed/Payload_is_too_big_by_3_breadcrumbs/errors.log to make sure it's behaving as expected, diff --git a/features/steps/breadcrumb_metadata_steps.rb b/features/steps/breadcrumb_metadata_steps.rb index b2b2ab5e16..5ab582506e 100644 --- a/features/steps/breadcrumb_metadata_steps.rb +++ b/features/steps/breadcrumb_metadata_steps.rb @@ -2,6 +2,11 @@ match_breadcrumb_metadata(message, path) { |v| Maze.check.equal(expected_value, v) } end +Then("the breadcrumb named {string} has {string} matching {string}") do |message, path, message_regex| + regex = Regexp.new(message_regex) + match_breadcrumb_metadata(message, path) { |v| Maze.check.match(regex, v) } +end + Then("the breadcrumb named {string} has {string} equal to {int}") do |message, path, expected_value| match_breadcrumb_metadata(message, path) { |v| Maze.check.equal(expected_value, v) } end From d1b60080d06eb2a4f11c36ecc909dfc352385210 Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 12 Oct 2022 17:16:12 +0100 Subject: [PATCH 7/8] fix(tests): use a dedicated Handler to deliver work to the main thread in Maze Runner --- .../android/mazerunner/MainActivity.kt | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) 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 78e028d0ea..561e90f271 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 @@ -6,6 +6,8 @@ import android.content.Intent import android.content.SharedPreferences import android.os.Build import android.os.Bundle +import android.os.Handler +import android.os.Looper import android.view.Window import android.widget.Button import android.widget.EditText @@ -20,6 +22,8 @@ import kotlin.math.max class MainActivity : Activity() { + private val mainHandler = Handler(Looper.getMainLooper()) + private val apiKeyKey = "BUGSNAG_API_KEY" lateinit var prefs: SharedPreferences @@ -118,7 +122,7 @@ class MainActivity : Activity() { log("command.sessionsUrl: $sessionsUrl") log("command.notifyUrl: $notifyUrl") - runOnUiThread { + mainHandler.post { // Display some feedback of the action being run on he UI val actionField = findViewById(R.id.command_action) val scenarioField = findViewById(R.id.command_scenario) @@ -196,13 +200,10 @@ class MainActivity : Activity() { * Enqueues the test case with a delay on the main thread. This avoids the Activity wrapping * unhandled Exceptions */ - window.decorView.postDelayed( - { - log("Executing scenario") - scenario?.startScenario() - }, - 1 - ) + mainHandler.post { + log("Executing scenario") + scenario?.startScenario() + } } // Clear persistent data (used to stop scenarios bleeding into each other) @@ -269,6 +270,7 @@ class MainActivity : Activity() { private fun getStoredApiKey() = prefs.getString(apiKeyKey, "") - private val String.width get() = - lineSequence().fold(0) { maxWidth, line -> max(maxWidth, line.length) } + private val String.width + get() = + lineSequence().fold(0) { maxWidth, line -> max(maxWidth, line.length) } } From 3133bcd76d0e264edc6022deaa942bfba0d331a9 Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 13 Oct 2022 11:58:59 +0100 Subject: [PATCH 8/8] v5.28.0 --- CHANGELOG.md | 11 +++++++---- .../src/main/java/com/bugsnag/android/Notifier.kt | 2 +- examples/sdk-app-example/app/build.gradle | 2 +- gradle.properties | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e72e12bd..eddfe9ae0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Changelog -## TBD +## 5.28.0 (2022-10-13) + +### Enhancements + +* Bugsnag now supports up to 500 breadcrumbs, with a default max of 100. Note that breadcrumbs will be trimmed + (oldest first) if the payload exceeds 1MB. + [#1751](https://github.com/bugsnag/bugsnag-android/pull/1751) ### Bug fixes @@ -11,9 +17,6 @@ ### Enhancements -* Bugsnag now supports up to 500 breadcrumbs, with a default max of 100. Note that breadcrumbs will be trimmed - (oldest first) if the payload exceeds 1MB. - [#1751](https://github.com/bugsnag/bugsnag-android/pull/1751) * Setting `Configuration.attemptDeliveryOnCrash` will cause Bugsnag to attempt error delivery during some crashes. Use of this feature is discouraged, see the method JavaDoc for more information. [#1749](https://github.com/bugsnag/bugsnag-android/pull/1749) 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 d479e4c684..db2d4a83ad 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 = "5.27.0", + var version: String = "5.28.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 65324eec46..12f7e4fbe5 100644 --- a/examples/sdk-app-example/app/build.gradle +++ b/examples/sdk-app-example/app/build.gradle @@ -38,7 +38,7 @@ android { } dependencies { - implementation "com.bugsnag:bugsnag-android:5.27.0" + implementation "com.bugsnag:bugsnag-android:5.28.0" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "androidx.appcompat:appcompat:1.4.0" implementation "com.google.android.material:material:1.4.0" diff --git a/gradle.properties b/gradle.properties index 2a03ba3a8e..8bd2d282e6 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=5.27.0 +VERSION_NAME=5.28.0 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git