Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-5176 [CP] Allow reporting RUM error in sync #2180

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Datadog/Datadog.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,8 @@
D2EFA869286DA85700F1FAA6 /* DatadogContextProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2EFA867286DA85700F1FAA6 /* DatadogContextProvider.swift */; };
D2EFA875286E011900F1FAA6 /* DatadogContextProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2EFA874286E011900F1FAA6 /* DatadogContextProviderTests.swift */; };
D2EFA876286E011900F1FAA6 /* DatadogContextProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2EFA874286E011900F1FAA6 /* DatadogContextProviderTests.swift */; };
D2F448E12D43A3DC007BB995 /* CompletionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F448E02D43A3DC007BB995 /* CompletionHandler.swift */; };
D2F448E22D43A3DC007BB995 /* CompletionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F448E02D43A3DC007BB995 /* CompletionHandler.swift */; };
D2F44FB8299AA1DA0074B0D9 /* DataCompressionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D213532F270CA722000315AD /* DataCompressionTests.swift */; };
D2F44FB9299AA1DB0074B0D9 /* DataCompressionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D213532F270CA722000315AD /* DataCompressionTests.swift */; };
D2F44FBC299AA36D0074B0D9 /* Decompression.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F44FBB299AA36D0074B0D9 /* Decompression.swift */; };
Expand Down Expand Up @@ -3164,6 +3166,7 @@
D2EFA874286E011900F1FAA6 /* DatadogContextProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatadogContextProviderTests.swift; sourceTree = "<group>"; };
D2EFF3D22731822A00D09F33 /* RUMViewsHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RUMViewsHandler.swift; sourceTree = "<group>"; };
D2F1B81426D8E5FF009F3293 /* DDNoopTracerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DDNoopTracerTests.swift; sourceTree = "<group>"; };
D2F448E02D43A3DC007BB995 /* CompletionHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CompletionHandler.swift; sourceTree = "<group>"; };
D2F44FBB299AA36D0074B0D9 /* Decompression.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Decompression.swift; sourceTree = "<group>"; };
D2F44FC1299BD5600074B0D9 /* UIViewController+KeyboardControlling.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UIViewController+KeyboardControlling.swift"; sourceTree = "<group>"; };
D2F8235229915E12003C7E99 /* DatadogSite.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatadogSite.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -6158,6 +6161,7 @@
children = (
D23039DB298D5235001A1FA3 /* ReadWriteLock.swift */,
D2432CF829EDB22C00D93657 /* Flushable.swift */,
D2F448E02D43A3DC007BB995 /* CompletionHandler.swift */,
);
path = Concurrency;
sourceTree = "<group>";
Expand Down Expand Up @@ -8941,6 +8945,7 @@
614A708E2BF754D800D9AF42 /* ImmutableRequest.swift in Sources */,
D2160CF429C0EDFC00FAA9A5 /* UploadPerformancePreset.swift in Sources */,
D23039E1298D5236001A1FA3 /* AppState.swift in Sources */,
D2F448E22D43A3DC007BB995 /* CompletionHandler.swift in Sources */,
D2DE63532A30A7CA00441A54 /* CoreRegistry.swift in Sources */,
E2AA55EA2C32C76A002FEF28 /* WatchKitExtensions.swift in Sources */,
D2EBEE2829BA160F00B15732 /* W3CHTTPHeadersWriter.swift in Sources */,
Expand Down Expand Up @@ -9943,6 +9948,7 @@
614A708F2BF754D800D9AF42 /* ImmutableRequest.swift in Sources */,
D2160CF529C0EDFC00FAA9A5 /* UploadPerformancePreset.swift in Sources */,
D2DA236C298D57AA00C6C7E6 /* AppState.swift in Sources */,
D2F448E12D43A3DC007BB995 /* CompletionHandler.swift in Sources */,
D2DE63542A30A7CA00441A54 /* CoreRegistry.swift in Sources */,
E2AA55EC2C32C78B002FEF28 /* WatchKitExtensions.swift in Sources */,
D2EBEE3629BA161100B15732 /* W3CHTTPHeadersWriter.swift in Sources */,
Expand Down
7 changes: 4 additions & 3 deletions DatadogCore/Sources/Core/Storage/Writing/AsyncWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ internal struct AsyncWriter: Writer {
self.queue = queue
}

func write<T: Encodable, M: Encodable>(value: T, metadata: M?) {
queue.async { writer.write(value: value, metadata: metadata) }
func write<T: Encodable, M: Encodable>(value: T, metadata: M?, completion: @escaping CompletionHandler) {
queue.async { writer.write(value: value, metadata: metadata, completion: completion) }
}
}

internal struct NOPWriter: Writer {
func write<T: Encodable, M: Encodable>(value: T, metadata: M?) {
func write<T: Encodable, M: Encodable>(value: T, metadata: M?, completion: @escaping CompletionHandler) {
completion()
}
}
4 changes: 3 additions & 1 deletion DatadogCore/Sources/Core/Storage/Writing/FileWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ internal struct FileWriter: Writer {
/// - Parameters:
/// - value: Encodable value to write.
/// - metadata: Encodable metadata to write.
func write<T: Encodable, M: Encodable>(value: T, metadata: M?) {
func write<T: Encodable, M: Encodable>(value: T, metadata: M?, completion: @escaping CompletionHandler) {
defer { completion() }

var encoded: Data = .init()
if let metadata = metadata {
do {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class FileWriterTests: XCTestCase {
}

func testItWritesDataWithMetadataToSingleFileInTLVFormat() throws {
let expectation = expectation(description: "Writes complete")
expectation.expectedFulfillmentCount = 3

let writer = FileWriter(
orchestrator: FilesOrchestrator(
directory: directory,
Expand All @@ -35,9 +38,9 @@ class FileWriterTests: XCTestCase {
telemetry: NOPTelemetry()
)

writer.write(value: ["key1": "value1"], metadata: ["meta1": "metaValue1"])
writer.write(value: ["key2": "value2"]) // skipped metadata here
writer.write(value: ["key3": "value3"], metadata: ["meta3": "metaValue3"])
writer.write(value: ["key1": "value1"], metadata: ["meta1": "metaValue1"], completion: expectation.fulfill)
writer.write(value: ["key2": "value2"], completion: expectation.fulfill) // skipped metadata here
writer.write(value: ["key3": "value3"], metadata: ["meta3": "metaValue3"], completion: expectation.fulfill)

XCTAssertEqual(try directory.files().count, 1)
let stream = try directory.files()[0].stream()
Expand All @@ -58,6 +61,8 @@ class FileWriterTests: XCTestCase {
block = try reader.next()
XCTAssertEqual(block?.type, .event)
XCTAssertEqual(block?.data, #"{"key3":"value3"}"#.utf8Data)

wait(for: [expectation], timeout: 0)
}

func testItWritesEncryptedDataWithMetadataToSingleFileInTLVFormat() throws {
Expand Down Expand Up @@ -102,6 +107,9 @@ class FileWriterTests: XCTestCase {
}

func testItWritesDataToSingleFileInTLVFormat() throws {
let expectation = expectation(description: "Writes complete")
expectation.expectedFulfillmentCount = 3

let writer = FileWriter(
orchestrator: FilesOrchestrator(
directory: directory,
Expand All @@ -113,9 +121,9 @@ class FileWriterTests: XCTestCase {
telemetry: NOPTelemetry()
)

writer.write(value: ["key1": "value1"])
writer.write(value: ["key2": "value2"])
writer.write(value: ["key3": "value3"])
writer.write(value: ["key1": "value1"], completion: expectation.fulfill)
writer.write(value: ["key2": "value2"], completion: expectation.fulfill)
writer.write(value: ["key3": "value3"], completion: expectation.fulfill)

XCTAssertEqual(try directory.files().count, 1)
let stream = try directory.files()[0].stream()
Expand All @@ -130,9 +138,14 @@ class FileWriterTests: XCTestCase {
block = try reader.next()
XCTAssertEqual(block?.type, .event)
XCTAssertEqual(block?.data, #"{"key3":"value3"}"#.utf8Data)

wait(for: [expectation], timeout: 0)
}

func testGivenErrorVerbosity_whenIndividualDataExceedsMaxWriteSize_itDropsDataAndPrintsError() throws {
let expectation = expectation(description: "Writes complete")
expectation.expectedFulfillmentCount = 2

let dd = DD.mockWith(logger: CoreLoggerMock())
defer { dd.reset() }

Expand Down Expand Up @@ -161,24 +174,28 @@ class FileWriterTests: XCTestCase {
telemetry: NOPTelemetry()
)

writer.write(value: ["key1": "value1"]) // will be written
writer.write(value: ["key1": "value1"], completion: expectation.fulfill) // will be written

XCTAssertEqual(try directory.files().count, 1)
var reader = try BatchDataBlockReader(input: directory.files()[0].stream())
var blocks = try XCTUnwrap(reader.all())
XCTAssertEqual(blocks.count, 1)
XCTAssertEqual(blocks[0].data, #"{"key1":"value1"}"#.utf8Data)

writer.write(value: ["key2": "value3 that makes it exceed 23 bytes"]) // will be dropped
writer.write(value: ["key2": "value3 that makes it exceed 23 bytes"], completion: expectation.fulfill) // will be dropped

reader = try BatchDataBlockReader(input: directory.files()[0].stream())
blocks = try XCTUnwrap(reader.all())
XCTAssertEqual(blocks.count, 1) // same content as before
XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to encode value")
XCTAssertEqual(dd.logger.errorLog?.error?.message, "DataBlock with \(47) bytes exceeds limit of \(23) bytes")

wait(for: [expectation], timeout: 0)
}

func testGivenErrorVerbosity_whenDataCannotBeEncoded_itPrintsError() throws {
let expectation = expectation(description: "Writes complete")

let dd = DD.mockWith(logger: CoreLoggerMock())
defer { dd.reset() }

Expand All @@ -199,13 +216,18 @@ class FileWriterTests: XCTestCase {
telemetry: NOPTelemetry()
)

writer.write(value: FailingEncodableMock(errorMessage: "failed to encode `FailingEncodable`."))
writer.write(value: FailingEncodableMock(errorMessage: "failed to encode `FailingEncodable`."), completion: expectation.fulfill)

XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to encode value")
XCTAssertEqual(dd.logger.errorLog?.error?.message, "failed to encode `FailingEncodable`.")

wait(for: [expectation], timeout: 0)
}

func testGivenErrorVerbosity_whenIOExceptionIsThrown_itPrintsError() throws {
let expectation = expectation(description: "Writes complete")
expectation.expectedFulfillmentCount = 2

let dd = DD.mockWith(logger: CoreLoggerMock())
defer { dd.reset() }

Expand All @@ -226,13 +248,15 @@ class FileWriterTests: XCTestCase {
telemetry: NOPTelemetry()
)

writer.write(value: ["ok"]) // will create the file
writer.write(value: ["ok"], completion: expectation.fulfill) // will create the file
try? directory.files()[0].makeReadonly()
writer.write(value: ["won't be written"])
writer.write(value: ["won't be written"], completion: expectation.fulfill)
try? directory.files()[0].makeReadWrite()

XCTAssertEqual(dd.logger.errorLog?.message, "(rum) Failed to write 26 bytes to file")
XCTAssertTrue(dd.logger.errorLog!.error!.message.contains("You don’t have permission"))

wait(for: [expectation], timeout: 0)
}

/// NOTE: Test added after incident-4797
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ private final class FeatureScopeInterceptor: @unchecked Sendable {
let actualWriter: Writer
unowned var interception: FeatureScopeInterceptor?

func write<T: Encodable, M: Encodable>(value: T, metadata: M) {
func write<T: Encodable, M: Encodable>(value: T, metadata: M, completion: @escaping () -> Void) {
group.enter()
defer { group.leave() }

actualWriter.write(value: value, metadata: metadata)
actualWriter.write(value: value, metadata: metadata, completion: completion)

let event = value
let data = try! InterceptingWriter.jsonEncoder.encode(value)
Expand Down
12 changes: 8 additions & 4 deletions DatadogCore/Tests/Datadog/Mocks/RUMFeatureMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,15 @@ extension RUMAddCurrentViewErrorCommand: AnyMockable, RandomMockable {
error: Error = ErrorMock(),
source: RUMInternalErrorSource = .source,
globalAttributes: [AttributeKey: AttributeValue] = [:],
attributes: [AttributeKey: AttributeValue] = [:]
attributes: [AttributeKey: AttributeValue] = [:],
completionHandler: @escaping CompletionHandler = NOPCompletionHandler
) -> RUMAddCurrentViewErrorCommand {
return RUMAddCurrentViewErrorCommand(
time: time,
error: error,
source: source,
attributes: attributes
attributes: attributes,
completionHandler: completionHandler
)
}

Expand All @@ -286,15 +288,17 @@ extension RUMAddCurrentViewErrorCommand: AnyMockable, RandomMockable {
type: String? = .mockAny(),
source: RUMInternalErrorSource = .source,
stack: String? = "Foo.swift:10",
attributes: [AttributeKey: AttributeValue] = [:]
attributes: [AttributeKey: AttributeValue] = [:],
completionHandler: @escaping CompletionHandler = NOPCompletionHandler
) -> RUMAddCurrentViewErrorCommand {
return RUMAddCurrentViewErrorCommand(
time: time,
message: message,
type: type,
stack: stack,
source: source,
attributes: attributes
attributes: attributes,
completionHandler: completionHandler
)
}
}
Expand Down
10 changes: 9 additions & 1 deletion DatadogCore/Tests/DatadogObjc/DDRUMMonitorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,12 @@ class DDRUMMonitorTests: XCTestCase {
objcRUMMonitor.addError(error: error, source: .custom, attributes: ["event-attribute1": "foo1"])
objcRUMMonitor.addError(message: "error message", stack: "error stack", source: .source, attributes: [:])

objcRUMMonitor._internal_sync_addError(NSError.mockAny(), source: .custom, attributes: [:])

let rumEventMatchers = try core.waitAndReturnRUMEventMatchers()

let errorEvents = rumEventMatchers.filterRUMEvents(ofType: RUMErrorEvent.self)
XCTAssertEqual(errorEvents.count, 4)
XCTAssertEqual(errorEvents.count, 5)

let event1Matcher = errorEvents[0]
let event1: RUMErrorEvent = try event1Matcher.model()
Expand Down Expand Up @@ -382,6 +384,12 @@ class DDRUMMonitorTests: XCTestCase {
XCTAssertEqual(event4.error.message, "error message")
XCTAssertEqual(event4.error.source, .source)
XCTAssertEqual(event4.error.stack, "error stack")

let event5Matcher = errorEvents[4]
let event5: RUMErrorEvent = try event5Matcher.model()
XCTAssertEqual(event5.error.type, "abc - 0")
XCTAssertEqual(event5.error.source, .custom)
XCTAssertEqual(event5.error.message, #"Error Domain=abc Code=0 "(null)""#)
}

func testSendingActionEvents() throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ - (void)testDDRUMMonitorAPI {
[monitor addAttributeForKey:@"" value:@""];
[monitor addFeatureFlagEvaluationWithName: @"name" value: @"value"];

[monitor _internal_sync_addError:[NSError errorWithDomain:NSCocoaErrorDomain code:-100 userInfo:nil]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Can you create a mock out of this error? It will also adds meaning to it

source:DDRUMErrorSourceCustom attributes:@{}];

[monitor setDebug:YES];
[monitor setDebug:NO];
}
Expand Down
15 changes: 15 additions & 0 deletions DatadogInternal/Sources/Concurrency/CompletionHandler.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2019-Present Datadog, Inc.
*/

import Foundation

/// Alias from completion closure with no parameter.
public typealias CompletionHandler = () -> Void

/// No-op completion function.
///
/// Using a function prevent allocating a closure when applying a placeholder.
public func NOPCompletionHandler() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Why is a NOP completion handler needed to avoid optionality? It’s common to encounter optional completion handlers

29 changes: 25 additions & 4 deletions DatadogInternal/Sources/Storage/Writer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,38 @@ import Foundation
/// A type, writing data.
public protocol Writer {
/// Encodes given encodable value and metadata, and writes to the destination.
/// - Parameter value: Encodable value to write.
/// - Parameter metadata: Encodable metadata to write.
func write<T: Encodable, M: Encodable>(value: T, metadata: M?)
/// - Parameters:
/// - value: Encodable value to write.
/// - metadata: Encodable metadata to write.
/// - completion: The block to execute after the write task is completed.
func write<T: Encodable, M: Encodable>(value: T, metadata: M?, completion: @escaping CompletionHandler)
}

extension Writer {
/// Encodes given encodable value and writes to the destination.
/// Uses `write(value:metadata:)` with `nil` metadata.
///
/// - Parameter value: Encodable value to write.
public func write<T: Encodable>(value: T) {
write(value: value, completion: {})
}

/// Encodes given encodable value and writes to the destination.
/// Uses `write(value:metadata:)` with `nil` metadata.
///
/// - Parameters:
/// - value: Encodable value to write.
/// - completion: The block to execute after the write task is completed.
public func write<T: Encodable>(value: T, completion: @escaping CompletionHandler) {
let metadata: Data? = nil
write(value: value, metadata: metadata)
write(value: value, metadata: metadata, completion: completion)
}

/// Encodes given encodable value and metadata, and writes to the destination.
/// - Parameters:
/// - value: Encodable value to write.
/// - metadata: Encodable metadata to write.
public func write<T: Encodable, M: Encodable>(value: T, metadata: M?) {
write(value: value, metadata: metadata, completion: {})
}
}
33 changes: 33 additions & 0 deletions DatadogObjc/Sources/RUM/RUM+objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import Foundation
import UIKit
import DatadogInternal
@_spi(Internal)
import DatadogRUM

internal struct UIKitRUMViewsPredicateBridge: UIKitRUMViewsPredicate {
Expand Down Expand Up @@ -670,3 +671,35 @@ public class DDRUMMonitor: NSObject {
get { swiftRUMMonitor.debug }
}
}

extension DDRUMMonitor {
/// **For Datadog internal use only. Subject to changes.**
///
/// Adds RUM error to current RUM view in sync.
///
/// **This method will block the caller thread for maximum 2 seconds.**
///
/// - Parameters:
/// - error: the `Error` object. It will be used to infer error details.
/// - source: the origin of the error.
/// - attributes: custom attributes to attach to this error.
@objc
public func _internal_sync_addError(
_ error: Error,
source: DDRUMErrorSource,
attributes: [String: Any]
) {
let semaphore = DispatchSemaphore(value: 0)

swiftRUMMonitor.addError(
error: error,
source: source.swiftType,
attributes: attributes.dd.swiftAttributes,
completionHandler: {
semaphore.signal()
}
)

_ = semaphore.wait(timeout: .now() + .seconds(2))
}
}
Loading