Skip to content

RUM-2207 Call RUM error mapper even for crashes#1945

Merged
fuzzybinary merged 2 commits into
developfrom
jward/RUM-2207-crash-event-mapper
Mar 29, 2024
Merged

RUM-2207 Call RUM error mapper even for crashes#1945
fuzzybinary merged 2 commits into
developfrom
jward/RUM-2207-crash-event-mapper

Conversation

@fuzzybinary

@fuzzybinary fuzzybinary commented Mar 27, 2024

Copy link
Copy Markdown
Member

What does this PR do?

This changes the RUM error event mapper to call the user supplied mapper even for crashes, which it previously excluded.

The error event mapper still does not support discarding fatal errors, and a warning is sent if the user returns null for a fatal error in the mapper.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@fuzzybinary
fuzzybinary requested review from a team as code owners March 27, 2024 13:59
@fuzzybinary
fuzzybinary force-pushed the jward/RUM-2207-crash-event-mapper branch 2 times, most recently from f5fae32 to 84a7b1e Compare March 27, 2024 14:41
Comment on lines +45 to +55
val mappedEvent = errorEventMapper.map(event)
if (mappedEvent == null) {
internalLogger.log(
InternalLogger.Level.WARN,
InternalLogger.Target.USER,
{ NO_DROPPING_FATAL_ERRORS_WARNING_MESSAGE }
)
event
} else {
mappedEvent
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

Could you add the tests for this use case ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The null case should already be checked (I just added the check for the warning log). I've added a check specifically for making sure the mapper is called when passing a fatal error.

@fuzzybinary
fuzzybinary requested a review from xgouchet March 28, 2024 14:38
The error event mapper still does not support discarding fatal errors, and a warning is sent if the user returns null for a fatal error in the mapper.
@fuzzybinary
fuzzybinary force-pushed the jward/RUM-2207-crash-event-mapper branch from fa8e4cf to 031f2f3 Compare March 28, 2024 16:03
@codecov-commenter

codecov-commenter commented Mar 28, 2024

Copy link
Copy Markdown

Codecov Report

Merging #1945 (031f2f3) into develop (736e81c) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1945      +/-   ##
===========================================
- Coverage    83.19%   83.14%   -0.05%     
===========================================
  Files          486      486              
  Lines        17560    17571      +11     
  Branches      2603     2604       +1     
===========================================
  Hits         14609    14609              
- Misses        2249     2263      +14     
+ Partials       702      699       -3     
Files Coverage Δ
...ndroid/rum/internal/domain/event/RumEventMapper.kt 100.00% <100.00%> (ø)

... and 28 files with indirect coverage changes

@fuzzybinary
fuzzybinary merged commit 2b9548a into develop Mar 29, 2024
@fuzzybinary
fuzzybinary deleted the jward/RUM-2207-crash-event-mapper branch March 29, 2024 13:21
@xgouchet xgouchet added this to the 2.8.x milestone Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants