Skip to content

RUM-2566: Convert pending resource to pending error when Resource scope completes with an error#1776

Merged
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-2566/convert-pending-resource-to-pending-error-when-resource-completes-with-an-error
Dec 20, 2023
Merged

RUM-2566: Convert pending resource to pending error when Resource scope completes with an error#1776
0xnm merged 1 commit into
developfrom
nogorodnikov/rum-2566/convert-pending-resource-to-pending-error-when-resource-completes-with-an-error

Conversation

@0xnm

@0xnm 0xnm commented Dec 19, 2023

Copy link
Copy Markdown
Member

What does this PR do?

There is a following bug: we increate RumViewScope#pendingResourceCount when we create resource scope, but resource scope can be finalized with 2 outcomes: either resource event or error event. In latter case we should instead decrease pendingResourceCount and increase pendingErrorCount, because ResourceSent / ResourceDropped will never be sent and we get instead ErrorSent / ErrorDropped.

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)

@0xnm
0xnm requested review from a team as code owners December 19, 2023 16:42
@codecov-commenter

codecov-commenter commented Dec 19, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1776 (3f5d97c) into develop (1972792) will increase coverage by 0.13%.
Report is 2 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1776      +/-   ##
===========================================
+ Coverage    83.46%   83.60%   +0.13%     
===========================================
  Files          469      469              
  Lines        16431    16435       +4     
  Branches      2465     2467       +2     
===========================================
+ Hits         13714    13739      +25     
+ Misses        2038     2027      -11     
+ Partials       679      669      -10     
Files Coverage Δ
.../android/rum/internal/domain/scope/RumViewScope.kt 94.44% <100.00%> (+0.35%) ⬆️

... and 19 files with indirect coverage changes

val entry = iterator.next()
val scope = entry.value.handleEvent(event, writer)
if (scope == null) {
// if we finalized this scope and it was by error, we won't have resource

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch

@xgouchet xgouchet left a comment

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.

Nice job

@0xnm
0xnm merged commit e06a3f3 into develop Dec 20, 2023
@0xnm
0xnm deleted the nogorodnikov/rum-2566/convert-pending-resource-to-pending-error-when-resource-completes-with-an-error branch December 20, 2023 11:03
@xgouchet xgouchet added this to the 2.4.0 milestone Feb 19, 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.

5 participants