RUM-5176 [CP] Allow reporting RUM error in sync #2180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxep
force-pushed
the
maxep/RUM-5176/sync-error
branch
from
9f9f963 to
ab9a5a4
Compare
Datadog ReportBranch report: ✅ 0 Failed, 930 Passed, 2819 Skipped, 53.72s Total duration (1m 37.66s time saved) |
maxep
force-pushed
the
maxep/RUM-5176/sync-error
branch
from
ab9a5a4 to
4cc5952
Compare
maxep
changed the title
maxep
force-pushed
the
maxep/RUM-5176/sync-error
branch
from
4cc5952 to
cdd9251
Compare
simaoseica-dd
previously approved these changes
Jan 27, 2025
| @@ -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] | |||
There was a problem hiding this comment.
suggestion: Can you create a mock out of this error? It will also adds meaning to it
| /// No-op completion function. | ||
| /// | ||
| /// Using a function prevent allocating a closure when applying a placeholder. | ||
| public func NOPCompletionHandler() {} |
There was a problem hiding this comment.
question: Why is a NOP completion handler needed to avoid optionality? It’s common to encounter optional completion handlers
mariedm
previously approved these changes
Feb 3, 2025
| } | ||
|
|
||
| extension RUMMonitorProtocol { | ||
| /// It cannot be declared '@_spi' without a default implementation in a protocol extension | ||
| func addViewLoadingTime(overwrite: Bool) { | ||
| // no-op | ||
| } | ||
|
|
||
| /// It cannot be declared '@_spi' without a default implementation in a protocol extension |
Co-authored-by: Marie Denis <[email protected]>
mariedm
approved these changes
Feb 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and why?
Cross-platform SDKs can report fatal errors before the native process crashes. However, RUM and the Core execute processing and write asynchronously, and the crash can suspend these executions, leading to the loss of the cross-platform error information.
We should let cross-platform SDKs to report error in sync, blocking the caller-thread.
How?
The change involve changes at different layers:
Core module
Add
completionargument toWriterinterface. The completion callback is called when the data is written to disk.RUM Module
Add
completionHandlerproperty toRUMErrorCommand. TheRUMViewScopecalls the completion handler after writing the error event and the view update.The RUM Monitor now expose a method to report an error with a completion handler, The method is exposed using the
@_spi(Internal)attribute.Objc Module
The
DDRUMMonitornow expose a_internal_sync_addErrorto report an error in sync. This method will wait for the underlying completion handler to be called before returning.Review checklist
make api-surface)