diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt index c85fc9e5f9..46e2f3e069 100644 --- a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/rnsentryandroidtester/RNSentryBreadcrumbTest.kt @@ -1,15 +1,20 @@ package io.sentry.rnsentryandroidtester import com.facebook.react.bridge.JavaOnlyMap +import io.sentry.ILogger import io.sentry.SentryLevel import io.sentry.react.RNSentryBreadcrumb import junit.framework.TestCase.assertEquals +import junit.framework.TestCase.assertNotNull import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 +import org.mockito.Mockito @RunWith(JUnit4::class) class RNSentryBreadcrumbTest { + private val logger = Mockito.mock(ILogger::class.java) + @Test fun generatesSentryBreadcrumbFromMap() { val testData = @@ -32,7 +37,7 @@ class RNSentryBreadcrumbTest { "data", testData, ) - val actual = RNSentryBreadcrumb.fromMap(map) + val actual = RNSentryBreadcrumb.fromMap(map, logger) assertEquals(SentryLevel.ERROR, actual.level) assertEquals("testCategory", actual.category) assertEquals("testOrigin", actual.origin) @@ -41,6 +46,28 @@ class RNSentryBreadcrumbTest { assertEquals(testData.toHashMap(), actual.data) } + @Test + fun defaultsToInfoLevelWhenMissing() { + val map = + JavaOnlyMap.of( + "message", + "testMessage", + ) + val actual = RNSentryBreadcrumb.fromMap(map, logger) + assertEquals(SentryLevel.INFO, actual.level) + } + + @Test + fun setsTimestamp() { + val map = + JavaOnlyMap.of( + "message", + "testMessage", + ) + val actual = RNSentryBreadcrumb.fromMap(map, logger) + assertNotNull(actual.timestamp) + } + @Test fun reactNativeForMissingOrigin() { val map = @@ -48,7 +75,7 @@ class RNSentryBreadcrumbTest { "message", "testMessage", ) - val actual = RNSentryBreadcrumb.fromMap(map) + val actual = RNSentryBreadcrumb.fromMap(map, logger) assertEquals("testMessage", actual.message) assertEquals("react-native", actual.origin) } diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift index 0002c11e4f..ad06980a9d 100644 --- a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift +++ b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryBreadcrumbTests.swift @@ -40,6 +40,14 @@ final class RNSentryBreadcrumbTests: XCTestCase { XCTAssertEqual(actualCrumb!.origin, "someOrigin") } + func testSetsTimestamp() { + let actualCrumb = RNSentryBreadcrumb.from([ + "message": "testMessage" + ]) + + XCTAssertNotNil(actualCrumb!.timestamp) + } + func testUsesInfoAsDefaultSentryLevel() { let actualCrumb = RNSentryBreadcrumb.from([ "message": "testMessage" diff --git a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryUserTests.m b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryUserTests.m index 9c603940a3..fe91d5f913 100644 --- a/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryUserTests.m +++ b/packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryUserTests.m @@ -50,17 +50,17 @@ - (void)testNullUser - (void)testEmptyUser { - SentryUser *expected = [[SentryUser alloc] init]; - [expected setData:@{ }]; - SentryUser *actual = [RNSentry userFrom:@{ } otherUserKeys:@{ }]; - XCTAssertTrue([actual isEqualToUser:expected]); + XCTAssertNotNil(actual); + XCTAssertNil(actual.userId); + XCTAssertNil(actual.email); + XCTAssertNil(actual.username); + XCTAssertNil(actual.ipAddress); + XCTAssertEqualObjects(actual.data, @{ }); } - (void)testInvalidUser { - SentryUser *expected = [[SentryUser alloc] init]; - SentryUser *actual = [RNSentry userFrom:@{ @"id" : @123, @"ip_address" : @ { }, @@ -69,14 +69,16 @@ - (void)testInvalidUser } otherUserKeys:nil]; - XCTAssertTrue([actual isEqualToUser:expected]); + // initWithDictionary: ignores non-string values for known string fields + XCTAssertNotNil(actual); + XCTAssertNil(actual.userId); + XCTAssertNil(actual.email); + XCTAssertNil(actual.username); + XCTAssertNil(actual.ipAddress); } - (void)testPartiallyInvalidUser { - SentryUser *expected = [[SentryUser alloc] init]; - [expected setUserId:@"123"]; - SentryUser *actual = [RNSentry userFrom:@{ @"id" : @"123", @"ip_address" : @ { }, @@ -85,13 +87,15 @@ - (void)testPartiallyInvalidUser } otherUserKeys:nil]; - XCTAssertTrue([actual isEqualToUser:expected]); + XCTAssertNotNil(actual); + XCTAssertEqualObjects(actual.userId, @"123"); + XCTAssertNil(actual.email); + XCTAssertNil(actual.username); + XCTAssertNil(actual.ipAddress); } - (void)testNullValuesUser { - SentryUser *expected = [[SentryUser alloc] init]; - SentryUser *actual = [RNSentry userFrom:@{ @"id" : [NSNull null], @"ip_address" : [NSNull null], @@ -100,7 +104,12 @@ - (void)testNullValuesUser } otherUserKeys:nil]; - XCTAssertTrue([actual isEqualToUser:expected]); + // initWithDictionary: ignores non-string values for known string fields + XCTAssertNotNil(actual); + XCTAssertNil(actual.userId); + XCTAssertNil(actual.email); + XCTAssertNil(actual.username); + XCTAssertNil(actual.ipAddress); } - (void)testUserWithGeo diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java index 45885adc9c..487f8c3b7e 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryBreadcrumb.java @@ -1,8 +1,12 @@ package io.sentry.react; import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.ReadableMapKeySetIterator; +import com.facebook.react.bridge.ReadableType; import io.sentry.Breadcrumb; +import io.sentry.ILogger; import io.sentry.SentryLevel; +import io.sentry.util.MapObjectReader; import java.util.Map; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -36,59 +40,41 @@ public static String getCurrentScreenFrom(ReadableMap from) { } @NotNull - public static Breadcrumb fromMap(ReadableMap from) { - final @NotNull Breadcrumb breadcrumb = new Breadcrumb(); - - if (from.hasKey("message")) { - breadcrumb.setMessage(from.getString("message")); - } - - if (from.hasKey("type")) { - breadcrumb.setType(from.getString("type")); - } - - if (from.hasKey("category")) { - breadcrumb.setCategory(from.getString("category")); - } - - if (from.hasKey("origin")) { - breadcrumb.setOrigin(from.getString("origin")); - } else { - breadcrumb.setOrigin("react-native"); - } + public static Breadcrumb fromMap(ReadableMap from, @NotNull ILogger logger) { + try { + final @NotNull MapObjectReader reader = new MapObjectReader(toDeepHashMap(from)); + final @NotNull Breadcrumb breadcrumb = + new Breadcrumb.Deserializer().deserialize(reader, logger); - if (from.hasKey("level")) { - switch (from.getString("level")) { - case "fatal": - breadcrumb.setLevel(SentryLevel.FATAL); - break; - case "warning": - breadcrumb.setLevel(SentryLevel.WARNING); - break; - case "debug": - breadcrumb.setLevel(SentryLevel.DEBUG); - break; - case "error": - breadcrumb.setLevel(SentryLevel.ERROR); - break; - case "info": - default: - breadcrumb.setLevel(SentryLevel.INFO); - break; + if (breadcrumb.getLevel() == null) { + breadcrumb.setLevel(SentryLevel.INFO); } + if (breadcrumb.getOrigin() == null) { + breadcrumb.setOrigin("react-native"); + } + + return breadcrumb; + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Failed to deserialize breadcrumb from map.", e); + final Breadcrumb fallback = new Breadcrumb(); + fallback.setOrigin("react-native"); + return fallback; } + } - if (from.hasKey("data")) { - final ReadableMap data = from.getMap("data"); - for (final Map.Entry entry : data.toHashMap().entrySet()) { - final Object value = entry.getValue(); - // data is ConcurrentHashMap and can't have null values - if (value != null) { - breadcrumb.setData(entry.getKey(), entry.getValue()); + @NotNull + static Map toDeepHashMap(@NotNull ReadableMap from) { + final Map map = from.toHashMap(); + final ReadableMapKeySetIterator iterator = from.keySetIterator(); + while (iterator.hasNextKey()) { + final String key = iterator.nextKey(); + if (from.getType(key) == ReadableType.Map) { + final ReadableMap nested = from.getMap(key); + if (nested != null) { + map.put(key, toDeepHashMap(nested)); } } } - - return breadcrumb; + return map; } } diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 0e1053d42b..69501ab5d7 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -53,7 +53,6 @@ import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.profilemeasurements.ProfileMeasurement; import io.sentry.profilemeasurements.ProfileMeasurementValue; -import io.sentry.protocol.Geo; import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; @@ -62,6 +61,7 @@ import io.sentry.util.FileUtils; import io.sentry.util.JsonSerializationUtils; import io.sentry.util.LoadClass; +import io.sentry.util.MapObjectReader; import io.sentry.vendor.Base64; import java.io.BufferedInputStream; import java.io.BufferedReader; @@ -575,60 +575,35 @@ public void setUser(final ReadableMap userKeys, final ReadableMap userDataKeys) if (userKeys == null && userDataKeys == null) { scope.setUser(null); } else { - User userInstance = new User(); - - if (userKeys != null) { - if (userKeys.hasKey("email")) { - userInstance.setEmail(userKeys.getString("email")); - } - - if (userKeys.hasKey("id")) { - userInstance.setId(userKeys.getString("id")); - } - - if (userKeys.hasKey("username")) { - userInstance.setUsername(userKeys.getString("username")); - } - - if (userKeys.hasKey("ip_address")) { - userInstance.setIpAddress(userKeys.getString("ip_address")); - } - - if (userKeys.hasKey("geo")) { - ReadableMap geoMap = userKeys.getMap("geo"); - if (geoMap != null) { - Geo geoData = new Geo(); - if (geoMap.hasKey("city")) { - geoData.setCity(geoMap.getString("city")); - } - if (geoMap.hasKey("country_code")) { - geoData.setCountryCode(geoMap.getString("country_code")); + try { + final MapObjectReader reader = + new MapObjectReader( + userKeys != null + ? RNSentryBreadcrumb.toDeepHashMap(userKeys) + : new HashMap<>()); + final User userInstance = new User.Deserializer().deserialize(reader, logger); + + if (userDataKeys != null) { + Map userDataMap = new HashMap<>(); + ReadableMapKeySetIterator it = userDataKeys.keySetIterator(); + while (it.hasNextKey()) { + String key = it.nextKey(); + String value = userDataKeys.getString(key); + + // other is ConcurrentHashMap and can't have null values + if (value != null) { + userDataMap.put(key, value); } - if (geoMap.hasKey("region")) { - geoData.setRegion(geoMap.getString("region")); - } - userInstance.setGeo(geoData); } - } - } - - if (userDataKeys != null) { - Map userDataMap = new HashMap<>(); - ReadableMapKeySetIterator it = userDataKeys.keySetIterator(); - while (it.hasNextKey()) { - String key = it.nextKey(); - String value = userDataKeys.getString(key); - // other is ConcurrentHashMap and can't have null values - if (value != null) { - userDataMap.put(key, value); - } + userInstance.setData(userDataMap); } - userInstance.setData(userDataMap); + scope.setUser(userInstance); + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Failed to deserialize user from map.", e); + scope.setUser(null); } - - scope.setUser(userInstance); } }); } @@ -636,7 +611,7 @@ public void setUser(final ReadableMap userKeys, final ReadableMap userDataKeys) public void addBreadcrumb(final ReadableMap breadcrumb) { Sentry.configureScope( scope -> { - scope.addBreadcrumb(RNSentryBreadcrumb.fromMap(breadcrumb)); + scope.addBreadcrumb(RNSentryBreadcrumb.fromMap(breadcrumb, logger)); final @Nullable String screen = RNSentryBreadcrumb.getCurrentScreenFrom(breadcrumb); if (screen != null) { diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index f6754e0e4b..c64cc6bb5e 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -27,6 +27,10 @@ #import #import +@interface SentryUser () +- (instancetype)initWithDictionary:(NSDictionary *)dictionary; +@end + // This guard prevents importing Hermes in JSC apps #if SENTRY_PROFILING_ENABLED # import @@ -654,24 +658,9 @@ + (SentryUser *_Nullable)userFrom:(NSDictionary *)userKeys { // we can safely ignore userDataKeys since if original JS user was null userKeys will be null if ([userKeys isKindOfClass:NSDictionary.class]) { - SentryUser *userInstance = [[SentryUser alloc] init]; - - id userId = [userKeys valueForKey:@"id"]; - if ([userId isKindOfClass:NSString.class]) { - [userInstance setUserId:userId]; - } - id ipAddress = [userKeys valueForKey:@"ip_address"]; - if ([ipAddress isKindOfClass:NSString.class]) { - [userInstance setIpAddress:ipAddress]; - } - id email = [userKeys valueForKey:@"email"]; - if ([email isKindOfClass:NSString.class]) { - [userInstance setEmail:email]; - } - id username = [userKeys valueForKey:@"username"]; - if ([username isKindOfClass:NSString.class]) { - [userInstance setUsername:username]; - } + NSMutableDictionary *filteredKeys = [userKeys mutableCopy]; + [filteredKeys removeObjectForKey:@"geo"]; + SentryUser *userInstance = [[SentryUser alloc] initWithDictionary:filteredKeys]; id geo = [userKeys valueForKey:@"geo"]; if ([geo isKindOfClass:NSDictionary.class]) { diff --git a/packages/core/ios/RNSentryBreadcrumb.m b/packages/core/ios/RNSentryBreadcrumb.m index cc77dce70a..7ec33cf2a3 100644 --- a/packages/core/ios/RNSentryBreadcrumb.m +++ b/packages/core/ios/RNSentryBreadcrumb.m @@ -1,37 +1,25 @@ #import "RNSentryBreadcrumb.h" @import Sentry; +@interface SentryBreadcrumb () +- (instancetype _Nonnull)initWithDictionary:(NSDictionary *_Nonnull)dictionary; +@end + @implementation RNSentryBreadcrumb + (SentryBreadcrumb *)from:(NSDictionary *)dict { - SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] init]; - - NSString *levelString = dict[@"level"]; - SentryLevel sentryLevel; - if ([levelString isEqualToString:@"fatal"]) { - sentryLevel = kSentryLevelFatal; - } else if ([levelString isEqualToString:@"warning"]) { - sentryLevel = kSentryLevelWarning; - } else if ([levelString isEqualToString:@"error"]) { - sentryLevel = kSentryLevelError; - } else if ([levelString isEqualToString:@"debug"]) { - sentryLevel = kSentryLevelDebug; - } else { - sentryLevel = kSentryLevelInfo; - } + SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithDictionary:dict]; - [crumb setLevel:sentryLevel]; - [crumb setCategory:dict[@"category"]]; - id origin = dict[@"origin"]; - if (origin != nil) { - [crumb setOrigin:origin]; - } else { + if (crumb.timestamp == nil) { + [crumb setTimestamp:[NSDate date]]; + } + if (dict[@"level"] == nil) { + [crumb setLevel:kSentryLevelInfo]; + } + if (dict[@"origin"] == nil) { [crumb setOrigin:@"react-native"]; } - [crumb setType:dict[@"type"]]; - [crumb setMessage:dict[@"message"]]; - [crumb setData:dict[@"data"]]; return crumb; } @@ -39,8 +27,8 @@ + (SentryBreadcrumb *)from:(NSDictionary *)dict + (NSString *_Nullable)getCurrentScreenFrom:(NSDictionary *_Nonnull)dict { NSString *_Nullable maybeCategory = [dict valueForKey:@"category"]; - if ([maybeCategory isKindOfClass:[NSString class]] - && ![maybeCategory isEqualToString:@"navigation"]) { + if (![maybeCategory isKindOfClass:[NSString class]] + || ![maybeCategory isEqualToString:@"navigation"]) { return nil; }