From 2b596fa7b9a6e3c6ad4b711e53af06a783df4a07 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 3 Oct 2024 17:53:08 +0300 Subject: [PATCH 1/7] Check if appLaunchedInForeground and launch duration for native app start --- .../src/main/java/io/sentry/react/RNSentryModuleImpl.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index e72549f034..61360f2a8b 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -47,6 +47,7 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import io.sentry.Breadcrumb; import io.sentry.DateUtils; @@ -380,6 +381,13 @@ public void fetchNativeRelease(Promise promise) { } public void fetchNativeAppStart(Promise promise) { + AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); + if (!appStartMetrics.isAppLaunchedInForeground() || + appStartMetrics.getAppStartTimeSpan().getDurationMs() > TimeUnit.MINUTES.toMillis(1)) { + logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground or app start took too long (>60s)"); + promise.resolve(null); + } + final Map measurement = InternalSentrySdk.getAppStartMeasurement(); WritableMap mutableMeasurement = (WritableMap) RNSentryMapConverter.convertToWritable(measurement); From 177ed85ce595f1dfdfb4711d2cef8412d54ddb05 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 3 Oct 2024 17:59:52 +0300 Subject: [PATCH 2/7] Updates changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 190f5d04dd..b2763276ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Use `appLaunchedInForeground` to determine invalid app start data on Android ([#4146](https://github.com/getsentry/sentry-react-native/pull/4146)) + ### Dependencies - Bump CLI from v2.36.1 to v2.36.6 ([#4116](https://github.com/getsentry/sentry-react-native/pull/4116), [#4131](https://github.com/getsentry/sentry-react-native/pull/4131), [#4137](https://github.com/getsentry/sentry-react-native/pull/4137), [#4144](https://github.com/getsentry/sentry-react-native/pull/4144)) From 00e48fb841be02283152ceb5df32586d5c0809f3 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Oct 2024 10:10:50 +0300 Subject: [PATCH 3/7] Removes 1 minute check since this is already implemented --- .../src/main/java/io/sentry/react/RNSentryModuleImpl.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 61360f2a8b..7abf70e705 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -47,7 +47,6 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import io.sentry.Breadcrumb; import io.sentry.DateUtils; @@ -381,10 +380,8 @@ public void fetchNativeRelease(Promise promise) { } public void fetchNativeAppStart(Promise promise) { - AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); - if (!appStartMetrics.isAppLaunchedInForeground() || - appStartMetrics.getAppStartTimeSpan().getDurationMs() > TimeUnit.MINUTES.toMillis(1)) { - logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground or app start took too long (>60s)"); + if (!AppStartMetrics.getInstance().isAppLaunchedInForeground()) { + logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground."); promise.resolve(null); } From 8864ff12ab40beaa1ae09996632ffb73a364365d Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Oct 2024 13:02:56 +0300 Subject: [PATCH 4/7] Adds tests for the fetchNativeAppStart function logic --- .../io/sentry/react/RNSentryModuleImplTest.kt | 97 +++++++++++++++++++ .../io/sentry/react/RNSentryModuleImpl.java | 10 +- 2 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt diff --git a/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt new file mode 100644 index 0000000000..899a6d4ac2 --- /dev/null +++ b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -0,0 +1,97 @@ +package io.sentry.react + +import android.content.pm.PackageInfo +import android.content.pm.PackageManager +import com.facebook.react.bridge.Arguments +import com.facebook.react.bridge.Promise +import com.facebook.react.bridge.ReactApplicationContext +import com.facebook.react.bridge.WritableMap +import io.sentry.ILogger +import io.sentry.SentryLevel +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.mockito.ArgumentCaptor +import org.mockito.Captor +import org.mockito.Mockito.* +import org.mockito.MockitoAnnotations +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.whenever + +@RunWith(JUnit4::class) +class RNSentryModuleImplTest { + + private lateinit var module: RNSentryModuleImpl + private lateinit var promise: Promise + private lateinit var logger: ILogger + private var argumentsMock: MockedStatic? = null + + @Captor + private lateinit var writableMapCaptor: ArgumentCaptor + + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + val reactContext = mock(ReactApplicationContext::class.java) + promise = mock(Promise::class.java) + logger = mock(ILogger::class.java) + val packageManager = mock(PackageManager::class.java) + val packageInfo = mock(PackageInfo::class.java) + + whenever(reactContext.packageManager).thenReturn(packageManager) + whenever(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(packageInfo) + + module = RNSentryModuleImpl(reactContext) + + // Mock the Arguments class + argumentsMock = mockStatic(Arguments::class.java) + val writableMap = mock(WritableMap::class.java) + whenever(Arguments.createMap()).thenReturn(writableMap) + } + + @After + fun tearDown() { + argumentsMock?.close() + } + + @Test + fun `fetchNativeAppStart resolves promise with null when app is not launched in the foreground`() { + // Mock the app start measurement + val appStartMeasurement = mapOf() + + // Call the method + module.fetchNativeAppStart(promise, appStartMeasurement, logger, true) + + // Verify a warning log is emitted + verify(logger, org.mockito.kotlin.times(1)).log( + SentryLevel.WARNING, + "Invalid app start data: app not launched in foreground." + ) + + // Verify the promise is resolved with null + verify(promise).resolve(null) + } + + @Test + fun `fetchNativeAppStart resolves promise with app start data when app is launched in the foreground`() { + // Mock the app start measurement + val appStartMeasurement = mapOf() + + // Call the method + module.fetchNativeAppStart(promise, appStartMeasurement, logger, false) + + // Verify no logs are emitted + verify(logger, org.mockito.kotlin.times(0)).log(any(), any()) + + // Verify the promise is resolved with the expected data + verify(promise).resolve(any(WritableMap::class.java)) + verify(promise).resolve(writableMapCaptor.capture()) + val capturedMap = writableMapCaptor.value + assertEquals(false, capturedMap.getBoolean("has_fetched")) + } +} diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 7abf70e705..c8333e53f0 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -380,14 +380,16 @@ public void fetchNativeRelease(Promise promise) { } public void fetchNativeAppStart(Promise promise) { - if (!AppStartMetrics.getInstance().isAppLaunchedInForeground()) { + fetchNativeAppStart(promise, InternalSentrySdk.getAppStartMeasurement(), logger, AppStartMetrics.getInstance().isAppLaunchedInForeground()); + } + + protected void fetchNativeAppStart(Promise promise, final Map appStartMeasurement, ILogger logger, boolean isAppLaunchedInForeground) { + if (isAppLaunchedInForeground) { logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground."); promise.resolve(null); } - final Map measurement = InternalSentrySdk.getAppStartMeasurement(); - - WritableMap mutableMeasurement = (WritableMap) RNSentryMapConverter.convertToWritable(measurement); + WritableMap mutableMeasurement = (WritableMap) RNSentryMapConverter.convertToWritable(appStartMeasurement); mutableMeasurement.putBoolean("has_fetched", hasFetchedAppStart); // This is always set to true, as we would only allow an app start fetch to only From ee5b85ebf9ea4c5f9400701d7d92b234e797087d Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Oct 2024 13:09:30 +0300 Subject: [PATCH 5/7] Fixes inverted test logic --- .../src/test/java/io/sentry/react/RNSentryModuleImplTest.kt | 4 ++-- android/src/main/java/io/sentry/react/RNSentryModuleImpl.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index 899a6d4ac2..036729c262 100644 --- a/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -65,7 +65,7 @@ class RNSentryModuleImplTest { val appStartMeasurement = mapOf() // Call the method - module.fetchNativeAppStart(promise, appStartMeasurement, logger, true) + module.fetchNativeAppStart(promise, appStartMeasurement, logger, false) // Verify a warning log is emitted verify(logger, org.mockito.kotlin.times(1)).log( @@ -83,7 +83,7 @@ class RNSentryModuleImplTest { val appStartMeasurement = mapOf() // Call the method - module.fetchNativeAppStart(promise, appStartMeasurement, logger, false) + module.fetchNativeAppStart(promise, appStartMeasurement, logger, true) // Verify no logs are emitted verify(logger, org.mockito.kotlin.times(0)).log(any(), any()) diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index c8333e53f0..ff49c7f2cf 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -384,7 +384,7 @@ public void fetchNativeAppStart(Promise promise) { } protected void fetchNativeAppStart(Promise promise, final Map appStartMeasurement, ILogger logger, boolean isAppLaunchedInForeground) { - if (isAppLaunchedInForeground) { + if (!isAppLaunchedInForeground) { logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground."); promise.resolve(null); } From 6212a23fe4daa8e6a05c7a2163c198778f9a3475 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Oct 2024 13:27:47 +0300 Subject: [PATCH 6/7] Guards execution when app is not launched in the foreground --- android/src/main/java/io/sentry/react/RNSentryModuleImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index ff49c7f2cf..cd498308f3 100644 --- a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -387,6 +387,7 @@ protected void fetchNativeAppStart(Promise promise, final Map ap if (!isAppLaunchedInForeground) { logger.log(SentryLevel.WARNING, "Invalid app start data: app not launched in foreground."); promise.resolve(null); + return; } WritableMap mutableMeasurement = (WritableMap) RNSentryMapConverter.convertToWritable(appStartMeasurement); From 27bd810bc034eb24c7cf8848f3af57b6884c30fa Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 8 Oct 2024 10:31:38 +0300 Subject: [PATCH 7/7] Removes unnecessary line Co-authored-by: LucasZF --- .../app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt index 036729c262..5a6585ab80 100644 --- a/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt +++ b/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryModuleImplTest.kt @@ -33,7 +33,6 @@ class RNSentryModuleImplTest { @Captor private lateinit var writableMapCaptor: ArgumentCaptor - @Before fun setUp() { MockitoAnnotations.openMocks(this)