Fix duplicate RUM Errors #538
Merged
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
louiszawadzki
force-pushed
the
louiszawadzki/RUM-1359/fix-duplicate-rum-crashes-ios-devices
branch
from
8504f5d
to
e95d7fb
Compare
fuzzybinary
approved these changes
Oct 13, 2023
| @@ -13,6 +13,8 @@ import type { | |||
| import { DdRumErrorTracking } from '../../../rum/instrumentation/DdRumErrorTracking'; | |||
| import { BufferSingleton } from '../../../sdk/DatadogProvider/Buffer/BufferSingleton'; | |||
|
|
|||
| jest.mock('../../../utils/jsUtils'); | |||
There was a problem hiding this comment.
Curious why this needed to be added?
There was a problem hiding this comment.
I don't want to add a 50ms delay in all my tests here - it will make them harder to debug in case they go wrong.
I added a default mock under utils/__mocks__/jsUtils.ts that is used when I add this line in a test file.
louiszawadzki
deleted the
louiszawadzki/RUM-1359/fix-duplicate-rum-crashes-ios-devices
branch
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 does this PR do?
This makes sure real iOS devices have time to update the crash context on the iOS SDK before actually crashing the app.
This way, when the app restarts it will pick up that a crash had happened and not send the duplicate native RUM Error.
I don't think we can have a "proper" fix that would wait for the crash context to be updated, or that would imply heavy changes.
The delay of 50ms is empirical. 5ms is too little for the crash context to be updated, 50ms gives us enough time in all my tests and the lag between the action and the crash is not noticeable.
Review checklist (to be filled by reviewers)