From d0f5b63d8c9aabfb148f72e0cf5add3c4528ad64 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 6 Jul 2023 12:11:34 +0200 Subject: [PATCH 1/5] Fix iOS beforeBreadcrumb discarding breadcrumbs if the callback is not set --- .../extensions/SentryOptionsExtensions.kt | 8 +- .../multiplatform/BreadcrumbConfigurator.kt | 23 +++++ .../multiplatform/BreadcrumbConfigurator.kt | 25 ++++++ .../BeforeBreadcrumbIntegrationTest.kt | 89 +++++++++++++++++++ .../multiplatform/BeforeBreadcrumbTest.kt | 19 ++++ .../multiplatform/BreadcrumbConfigurator.kt | 13 +++ 6 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 sentry-kotlin-multiplatform/src/commonAppleTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt create mode 100644 sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt create mode 100644 sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt create mode 100644 sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt create mode 100644 sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt diff --git a/sentry-kotlin-multiplatform/src/commonAppleMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt b/sentry-kotlin-multiplatform/src/commonAppleMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt index 973de837..5d73d6cd 100644 --- a/sentry-kotlin-multiplatform/src/commonAppleMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt +++ b/sentry-kotlin-multiplatform/src/commonAppleMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt @@ -67,8 +67,12 @@ internal fun CocoaSentryOptions.applyCocoaBaseOptions(options: SentryOptions) { PrivateSentrySDKOnly.setSdkName(sdkName, sdkVersion) beforeBreadcrumb = { cocoaBreadcrumb -> - cocoaBreadcrumb?.toKmpBreadcrumb() - ?.let { options.beforeBreadcrumb?.invoke(it) }?.toCocoaBreadcrumb() + if (options.beforeBreadcrumb == null) { + cocoaBreadcrumb + } else { + cocoaBreadcrumb?.toKmpBreadcrumb() + ?.let { options.beforeBreadcrumb?.invoke(it) }?.toCocoaBreadcrumb() + } } enableCaptureFailedRequests = options.enableCaptureFailedRequests diff --git a/sentry-kotlin-multiplatform/src/commonAppleTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt b/sentry-kotlin-multiplatform/src/commonAppleTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt new file mode 100644 index 00000000..944d7514 --- /dev/null +++ b/sentry-kotlin-multiplatform/src/commonAppleTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt @@ -0,0 +1,23 @@ +package io.sentry.kotlin.multiplatform + +import io.sentry.kotlin.multiplatform.extensions.applyCocoaBaseOptions +import io.sentry.kotlin.multiplatform.extensions.toKmpBreadcrumb +import io.sentry.kotlin.multiplatform.protocol.Breadcrumb + +actual class BreadcrumbConfigurator { + private val cocoaBreadcrumb = CocoaBreadcrumb() + actual val originalBreadcrumb: Breadcrumb = cocoaBreadcrumb.toKmpBreadcrumb() + + actual fun applyOptions(optionsConfiguration: OptionsConfiguration): Breadcrumb? { + val kmpOptions = SentryOptions() + optionsConfiguration.invoke(kmpOptions) + return applyOptions(kmpOptions) + } + + actual fun applyOptions(options: SentryOptions): Breadcrumb? { + val cocoaOptions = CocoaSentryOptions() + cocoaOptions.applyCocoaBaseOptions(options) + val cocoaModifiedBreadcrumb = cocoaOptions.beforeBreadcrumb?.invoke(cocoaBreadcrumb) + return cocoaModifiedBreadcrumb?.toKmpBreadcrumb() + } +} diff --git a/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt b/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt new file mode 100644 index 00000000..e16c5151 --- /dev/null +++ b/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt @@ -0,0 +1,25 @@ +package io.sentry.kotlin.multiplatform + +import io.sentry.Hint +import io.sentry.kotlin.multiplatform.extensions.applyJvmBaseOptions +import io.sentry.kotlin.multiplatform.extensions.toKmpBreadcrumb +import io.sentry.kotlin.multiplatform.protocol.Breadcrumb + +actual class BreadcrumbConfigurator { + private val jvmBreadcrumb = JvmBreadcrumb() + actual val originalBreadcrumb: Breadcrumb = jvmBreadcrumb.toKmpBreadcrumb() + + actual fun applyOptions(optionsConfiguration: OptionsConfiguration): Breadcrumb? { + val kmpOptions = SentryOptions() + optionsConfiguration.invoke(kmpOptions) + return applyOptions(kmpOptions) + } + + actual fun applyOptions(options: SentryOptions): Breadcrumb? { + val jvmOptions = JvmSentryOptions() + jvmOptions.applyJvmBaseOptions(options) + val jvmHint = Hint() + val jvmModifiedBreadcrumb = jvmOptions.beforeBreadcrumb?.execute(jvmBreadcrumb, jvmHint) + return jvmModifiedBreadcrumb?.toKmpBreadcrumb() + } +} \ No newline at end of file diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt new file mode 100644 index 00000000..eb30f20f --- /dev/null +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt @@ -0,0 +1,89 @@ +package io.sentry.kotlin.multiplatform + +import io.sentry.kotlin.multiplatform.utils.fakeDsn +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class BeforeBreadcrumbIntegrationTest { + private val breadcrumbConfigurator = BreadcrumbConfigurator() + + @Test + fun `breadcrumb is not null if KMP beforeBreadcrumb callback config is null`() { + val breadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + } + assertNotNull(breadcrumb) + } + + @Test + fun `breadcrumb is null if KMP beforeBreadcrumb callback config returns null`() { + val breadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { + null + } + } + assertNull(breadcrumb) + } + + @Test + fun `breadcrumb is not null if KMP beforeBreadcrumb callback config returns not null`() { + val breadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb + } + } + assertNotNull(breadcrumb) + } + + @Test + fun `breadcrumb level is modified if KMP beforeBreadcrumb callback config modifies it`() { + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb.level = SentryLevel.WARNING + breadcrumb + } + } + assertEquals(SentryLevel.WARNING, modifiedBreadcrumb?.level) + } + + @Test + fun `breadcrumb category is modified if KMP beforeBreadcrumb callback config modifies it`() { + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb.category = "category" + breadcrumb + } + } + assertEquals("category", modifiedBreadcrumb?.category) + } + + @Test + fun `breadcrumb type is modified if KMP beforeBreadcrumb callback config modifies it`() { + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb.type = "type" + breadcrumb + } + } + assertEquals("type", modifiedBreadcrumb?.type) + } + + @Test + fun `breadcrumb message is modified if KMP beforeBreadcrumb callback config modifies it`() { + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb.message = "message" + breadcrumb + } + } + assertEquals("message", modifiedBreadcrumb?.message) + } +} \ No newline at end of file diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt new file mode 100644 index 00000000..2e5925c1 --- /dev/null +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt @@ -0,0 +1,19 @@ +package io.sentry.kotlin.multiplatform + +import io.sentry.kotlin.multiplatform.protocol.Breadcrumb +import kotlin.test.Test +import kotlin.test.assertEquals + +class BeforeBreadcrumbTest { + @Test + fun `beforeBreadcrumb drops breadcrumb`() { + val options = SentryOptions() + options.beforeBreadcrumb = { + null + } + + val breadcrumb = options.beforeBreadcrumb?.invoke(Breadcrumb()) + + assertEquals(null, breadcrumb) + } +} \ No newline at end of file diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt new file mode 100644 index 00000000..fdf0f297 --- /dev/null +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt @@ -0,0 +1,13 @@ +package io.sentry.kotlin.multiplatform + +import io.sentry.kotlin.multiplatform.protocol.Breadcrumb + +/** + * This class deals with configuring and modifying a Breadcrumb. + * It is used to test any code that can alter a Breadcrumb. + */ +expect class BreadcrumbConfigurator() { + val originalBreadcrumb: Breadcrumb + fun applyOptions(optionsConfiguration: OptionsConfiguration): Breadcrumb? + fun applyOptions(options: SentryOptions = SentryOptions()): Breadcrumb? +} \ No newline at end of file From 790af080b932cedfa4767046fda35869091d4453 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 6 Jul 2023 12:33:11 +0200 Subject: [PATCH 2/5] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56e8b3d3..e970f3b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- fix: beforeBreadcrumb discarding breadcrumbs on Apple platforms ([#101](https://github.com/getsentry/sentry-kotlin-multiplatform/pull/101)) + ## 0.2.0 ### Features From 4abe2c62fb33de6edaad4df79839d8237971ecb4 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 6 Jul 2023 12:33:41 +0200 Subject: [PATCH 3/5] Apply formatting --- .../io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt | 2 +- .../kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt | 2 +- .../io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt | 2 +- .../io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt b/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt index e16c5151..1d4912b5 100644 --- a/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt +++ b/sentry-kotlin-multiplatform/src/commonJvmTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt @@ -22,4 +22,4 @@ actual class BreadcrumbConfigurator { val jvmModifiedBreadcrumb = jvmOptions.beforeBreadcrumb?.execute(jvmBreadcrumb, jvmHint) return jvmModifiedBreadcrumb?.toKmpBreadcrumb() } -} \ No newline at end of file +} diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt index eb30f20f..abe76302 100644 --- a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt @@ -86,4 +86,4 @@ class BeforeBreadcrumbIntegrationTest { } assertEquals("message", modifiedBreadcrumb?.message) } -} \ No newline at end of file +} diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt index 2e5925c1..aad7f79a 100644 --- a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbTest.kt @@ -16,4 +16,4 @@ class BeforeBreadcrumbTest { assertEquals(null, breadcrumb) } -} \ No newline at end of file +} diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt index fdf0f297..4dcda258 100644 --- a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BreadcrumbConfigurator.kt @@ -10,4 +10,4 @@ expect class BreadcrumbConfigurator() { val originalBreadcrumb: Breadcrumb fun applyOptions(optionsConfiguration: OptionsConfiguration): Breadcrumb? fun applyOptions(options: SentryOptions = SentryOptions()): Breadcrumb? -} \ No newline at end of file +} From e8a440aad18be5ee6d9e823550a5c63989db5f6f Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 6 Jul 2023 13:19:20 +0200 Subject: [PATCH 4/5] Fix test --- .../multiplatform/extensions/SentryOptionsExtensions.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sentry-kotlin-multiplatform/src/commonJvmMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt b/sentry-kotlin-multiplatform/src/commonJvmMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt index f793dda9..8b6ef6ed 100644 --- a/sentry-kotlin-multiplatform/src/commonJvmMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt +++ b/sentry-kotlin-multiplatform/src/commonJvmMain/kotlin/io/sentry/kotlin/multiplatform/extensions/SentryOptionsExtensions.kt @@ -41,7 +41,11 @@ internal fun JvmSentryOptions.applyJvmBaseOptions(options: SentryOptions) { maxAttachmentSize = options.maxAttachmentSize maxBreadcrumbs = options.maxBreadcrumbs setBeforeBreadcrumb { jvmBreadcrumb, _ -> - options.beforeBreadcrumb?.invoke(jvmBreadcrumb.toKmpBreadcrumb())?.toJvmBreadcrumb() + if (options.beforeBreadcrumb == null) { + jvmBreadcrumb + } else { + options.beforeBreadcrumb?.invoke(jvmBreadcrumb.toKmpBreadcrumb())?.toJvmBreadcrumb() + } } setBeforeSend { jvmSentryEvent, hint -> if (options.beforeSend == null) { From 03242b2c4783786d1daf2d17d5498dd4d8bd2ad7 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 6 Jul 2023 13:33:42 +0200 Subject: [PATCH 5/5] Add additional beforeBreadcrumb tests --- .../BeforeBreadcrumbIntegrationTest.kt | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt index abe76302..27fe6165 100644 --- a/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt +++ b/sentry-kotlin-multiplatform/src/commonTest/kotlin/io/sentry/kotlin/multiplatform/BeforeBreadcrumbIntegrationTest.kt @@ -39,6 +39,18 @@ class BeforeBreadcrumbIntegrationTest { assertNotNull(breadcrumb) } + @Test + fun `breadcrumb level is not modified if KMP beforeBreadcrumb callback config does not modify it`() { + val originalBreadcrumb = breadcrumbConfigurator.originalBreadcrumb + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb + } + } + assertEquals(originalBreadcrumb.level, modifiedBreadcrumb?.level) + } + @Test fun `breadcrumb level is modified if KMP beforeBreadcrumb callback config modifies it`() { val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { @@ -51,6 +63,18 @@ class BeforeBreadcrumbIntegrationTest { assertEquals(SentryLevel.WARNING, modifiedBreadcrumb?.level) } + @Test + fun `breadcrumb category is not modified if KMP beforeBreadcrumb callback config does not modify it`() { + val originalBreadcrumb = breadcrumbConfigurator.originalBreadcrumb + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb + } + } + assertEquals(originalBreadcrumb.category, modifiedBreadcrumb?.category) + } + @Test fun `breadcrumb category is modified if KMP beforeBreadcrumb callback config modifies it`() { val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { @@ -63,6 +87,18 @@ class BeforeBreadcrumbIntegrationTest { assertEquals("category", modifiedBreadcrumb?.category) } + @Test + fun `breadcrumb type is not modified if KMP beforeBreadcrumb callback config does not modify it`() { + val originalBreadcrumb = breadcrumbConfigurator.originalBreadcrumb + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb + } + } + assertEquals(originalBreadcrumb.type, modifiedBreadcrumb?.type) + } + @Test fun `breadcrumb type is modified if KMP beforeBreadcrumb callback config modifies it`() { val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { @@ -75,6 +111,18 @@ class BeforeBreadcrumbIntegrationTest { assertEquals("type", modifiedBreadcrumb?.type) } + @Test + fun `breadcrumb message is not modified if KMP beforeBreadcrumb callback config does not modify it`() { + val originalBreadcrumb = breadcrumbConfigurator.originalBreadcrumb + val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions { + it.dsn = fakeDsn + it.beforeBreadcrumb = { breadcrumb -> + breadcrumb + } + } + assertEquals(originalBreadcrumb.level, modifiedBreadcrumb?.level) + } + @Test fun `breadcrumb message is modified if KMP beforeBreadcrumb callback config modifies it`() { val modifiedBreadcrumb = breadcrumbConfigurator.applyOptions {