RUMM-1718 PR comments addressed

errorSourceType is extracted in RUMCommand level instead of Scopes level
DataDog · Nov 5, 2021 · c683734 · c683734
1 parent a5905e4
commit c683734
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 59 deletions.
diff --git a/Sources/Datadog/RUM/RUMMonitor/RUMCommand.swift b/Sources/Datadog/RUM/RUMMonitor/RUMCommand.swift
@@ -69,6 +69,8 @@ internal struct RUMAddCurrentViewErrorCommand: RUMCommand {
     let stack: String?
     /// The origin of this error.
     let source: RUMInternalErrorSource
+    /// The platform type of the error (iOS, React Native, ...)
+    let errorSourceType: RUMErrorEvent.Error.SourceType
 
     init(
         time: Date,
@@ -84,6 +86,8 @@ internal struct RUMAddCurrentViewErrorCommand: RUMCommand {
         self.message = message
         self.type = type
         self.stack = stack
+
+        self.errorSourceType = RUMErrorSourceType.extract(from: &self.attributes)
     }
 
     init(
@@ -100,6 +104,8 @@ internal struct RUMAddCurrentViewErrorCommand: RUMCommand {
         self.message = dderror.message
         self.type = dderror.type
         self.stack = dderror.stack
+
+        self.errorSourceType = RUMErrorSourceType.extract(from: &self.attributes)
     }
 }
 
@@ -176,6 +182,8 @@ internal struct RUMStopResourceWithErrorCommand: RUMResourceCommand {
     let errorType: String?
     /// The origin of the error (network, webview, ...)
     let errorSource: RUMInternalErrorSource
+    /// The platform type of the error (iOS, React Native, ...)
+    let errorSourceType: RUMErrorEvent.Error.SourceType
     /// Error stacktrace.
     let stack: String?
     /// HTTP status code of the Ressource error.
@@ -199,6 +207,8 @@ internal struct RUMStopResourceWithErrorCommand: RUMResourceCommand {
         self.httpStatusCode = httpStatusCode
         // The stack will be meaningless in most cases as it will go down to the networking code:
         self.stack = nil
+
+        self.errorSourceType = RUMErrorSourceType.extract(from: &self.attributes)
     }
 
     init(
@@ -220,6 +230,8 @@ internal struct RUMStopResourceWithErrorCommand: RUMResourceCommand {
         self.errorType = dderror.type
         // The stack will give the networking error (`NSError`) description in most cases:
         self.stack = dderror.stack
+
+        self.errorSourceType = RUMErrorSourceType.extract(from: &self.attributes)
     }
 }
 

diff --git a/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMResourceScope.swift b/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMResourceScope.swift
@@ -208,10 +208,6 @@ internal class RUMResourceScope: RUMScope {
 
     private func sendErrorEvent(on command: RUMStopResourceWithErrorCommand) -> Bool {
         attributes.merge(rumCommandAttributes: command.attributes)
-        let sourceType = (attributes.removeValue(forKey: RUMAttribute.internalErrorSourceType) as? String)
-            .flatMap {
-                return RUMErrorEvent.Error.SourceType(rawValue: $0)
-            } ?? .ios
 
         let eventData = RUMErrorEvent(
             dd: .init(
@@ -237,7 +233,7 @@ internal class RUMResourceScope: RUMScope {
                     url: resourceURL
                 ),
                 source: command.errorSource.toRUMDataFormat,
-                sourceType: sourceType,
+                sourceType: command.errorSourceType,
                 stack: command.stack,
                 type: command.errorType
             ),

diff --git a/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift b/Sources/Datadog/RUM/RUMMonitor/Scopes/RUMViewScope.swift
@@ -413,10 +413,6 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
 
     private func sendErrorEvent(on command: RUMAddCurrentViewErrorCommand) -> Bool {
         attributes.merge(rumCommandAttributes: command.attributes)
-        let sourceType = (attributes.removeValue(forKey: RUMAttribute.internalErrorSourceType) as? String)
-            .flatMap {
-                return RUMErrorEvent.Error.SourceType(rawValue: $0)
-            } ?? .ios
 
         let eventData = RUMErrorEvent(
             dd: .init(
@@ -437,7 +433,7 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
                 message: command.message,
                 resource: nil,
                 source: command.source.toRUMDataFormat,
-                sourceType: sourceType,
+                sourceType: command.errorSourceType,
                 stack: command.stack,
                 type: command.type
             ),

diff --git a/Sources/Datadog/RUMMonitor.swift b/Sources/Datadog/RUMMonitor.swift
@@ -62,6 +62,17 @@ internal extension RUMResourceType {
     }
 }
 
+internal typealias RUMErrorSourceType = RUMErrorEvent.Error.SourceType
+
+internal extension RUMErrorSourceType {
+    static func extract(from attributes: inout [AttributeKey: AttributeValue]) -> Self {
+        return (attributes.removeValue(forKey: RUMAttribute.internalErrorSourceType) as? String)
+            .flatMap {
+                return RUMErrorEvent.Error.SourceType(rawValue: $0)
+            } ?? .ios
+    }
+}
+
 /// Describes the type of a RUM Action.
 public enum RUMUserActionType {
     case tap

diff --git a/Tests/DatadogTests/Datadog/RUM/RUMMonitor/RUMCommandTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMMonitor/RUMCommandTests.swift
@@ -8,30 +8,33 @@ import XCTest
 @testable import Datadog
 
 class RUMCommandTests: XCTestCase {
+    struct SwiftError: Error, CustomDebugStringConvertible {
+        let debugDescription = "error description"
+    }
+
+    enum SwiftEnumeratedError: Error {
+        case errorLabel
+    }
+
+    let nsError = NSError(
+        domain: "custom-domain",
+        code: 10,
+        userInfo: [NSLocalizedDescriptionKey: "error description"]
+    )
+
     func testWhenRUMAddCurrentViewErrorCommand_isBuildWithErrorObject() {
-        struct SwiftError: Error, CustomDebugStringConvertible {
-            let debugDescription = "error description"
-        }
         var command = RUMAddCurrentViewErrorCommand(time: .mockAny(), error: SwiftError(), source: .source, attributes: [:])
 
         XCTAssertEqual(command.type, "SwiftError")
         XCTAssertEqual(command.message, "error description")
         XCTAssertEqual(command.stack, "error description")
 
-        enum SwiftEnumeratedError: Error {
-            case errorLabel
-        }
         command = RUMAddCurrentViewErrorCommand(time: .mockAny(), error: SwiftEnumeratedError.errorLabel, source: .source, attributes: [:])
 
         XCTAssertEqual(command.type, "SwiftEnumeratedError")
         XCTAssertEqual(command.message, "errorLabel")
         XCTAssertEqual(command.stack, "errorLabel")
 
-        let nsError = NSError(
-            domain: "custom-domain",
-            code: 10,
-            userInfo: [NSLocalizedDescriptionKey: "error description"]
-        )
         command = RUMAddCurrentViewErrorCommand(time: .mockAny(), error: nsError, source: .source, attributes: [:])
 
         XCTAssertEqual(command.type, "custom-domain - 10")
@@ -43,4 +46,28 @@ class RUMCommandTests: XCTestCase {
             """
         )
     }
+
+    func testWhenRUMAddCurrentViewErrorCommand_isPassedErrorSourceTypeAttribute() {
+        let command1 = RUMAddCurrentViewErrorCommand(time: .mockAny(), error: SwiftError(), source: .source, attributes: [RUMAttribute.internalErrorSourceType: "react-native"])
+
+        XCTAssertEqual(command1.errorSourceType, .reactNative)
+        XCTAssertTrue(command1.attributes.isEmpty)
+
+        let command2 = RUMAddCurrentViewErrorCommand(time: .mockAny(), message: .mockAny(), type: .mockAny(), stack: .mockAny(), source: .source, attributes: [RUMAttribute.internalErrorSourceType: "react-native"])
+
+        XCTAssertEqual(command2.errorSourceType, .reactNative)
+        XCTAssertTrue(command2.attributes.isEmpty)
+    }
+
+    func testWhenRUMStopResourceWithErrorCommand_isPassedErrorSourceTypeAttribute() {
+        let command1 = RUMStopResourceWithErrorCommand(resourceKey: .mockAny(), time: .mockAny(), error: SwiftError(), source: .source, httpStatusCode: .mockAny(), attributes: [RUMAttribute.internalErrorSourceType: "react-native"])
+
+        XCTAssertEqual(command1.errorSourceType, .reactNative)
+        XCTAssertTrue(command1.attributes.isEmpty)
+
+        let command2 = RUMStopResourceWithErrorCommand(resourceKey: .mockAny(), time: .mockAny(), message: .mockAny(), type: .mockAny(), source: .source, httpStatusCode: .mockAny(), attributes: [RUMAttribute.internalErrorSourceType: "react-native"])
+
+        XCTAssertEqual(command2.errorSourceType, .reactNative)
+        XCTAssertTrue(command2.attributes.isEmpty)
+    }
 }
diff --git a/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMMonitor/Scopes/RUMViewScopeTests.swift
@@ -631,44 +631,6 @@ class RUMViewScopeTests: XCTestCase {
         XCTAssertEqual(viewUpdate.model.view.error.count, 1, "Failed Resource should be counted as Error")
     }
 
-    func testWhenErrorSourceTypeIsPassed_itSendsNonDefaultErrorSourceType() throws {
-        var currentTime: Date = .mockDecember15th2019At10AMUTC()
-        let scope = RUMViewScope(
-            parent: parent,
-            dependencies: dependencies,
-            identity: mockView,
-            path: "UIViewController",
-            name: "ViewName",
-            attributes: ["_dd.error.source_type": "react-native"],
-            customTimings: [:],
-            startTime: currentTime
-        )
-
-        _ = scope.process(
-            command: RUMStartViewCommand.mockWith(time: currentTime, attributes: ["foo": "bar"], identity: mockView, isInitialView: true)
-        )
-
-        currentTime.addTimeInterval(1)
-
-        _ = scope.process(
-            command: RUMAddCurrentViewErrorCommand.mockWithErrorMessage(time: currentTime, message: "view error", source: .source, stack: nil)
-        )
-
-        let error = try XCTUnwrap(output.recordedEvents(ofType: RUMEvent<RUMErrorEvent>.self).last)
-        XCTAssertValidRumUUID(error.model.view.id)
-        XCTAssertEqual(error.model.error.type, "abc")
-        XCTAssertEqual(error.model.error.message, "view error")
-        XCTAssertEqual(error.model.error.source, .source)
-        XCTAssertEqual(error.model.error.sourceType, .reactNative)
-        XCTAssertNil(error.model.error.stack)
-        XCTAssertNil(error.model.error.isCrash)
-        XCTAssertNil(error.model.error.resource)
-        XCTAssertNil(error.model.action)
-
-        let viewUpdate = try XCTUnwrap(output.recordedEvents(ofType: RUMEvent<RUMViewEvent>.self).last)
-        XCTAssertEqual(viewUpdate.model.view.error.count, 1)
-    }
-
     // MARK: - Long tasks
 
     func testWhenLongTaskIsAdded_itSendsLongTaskEventAndViewUpdateEvent() throws {