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

fix(instrumentation-redis-4): fix unhandledRejection in client.multi(...) handling #1730

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Oct 12, 2023

The instrumentation was not handling a rejection of the promise from client.multi(), resulting in unended spans and an unhandleRejection event.

Fixes: #1708


This is based on work by @FredrikAugust in #1717.

…) handling

The instrumentation was not handling a rejection of the promise from
client.multi(), resulting in unended spans and an unhandleRejection
event.

Fixes: open-telemetry#1708
@trentm trentm requested a review from a team October 12, 2023 04:46
@github-actions github-actions bot requested a review from blumamir October 12, 2023 04:46
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #1730 (3a7c993) into main (a8c225d) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 85.71%.

❗ Current head 3a7c993 differs from pull request most recent head 5000091. Consider uploading reports for the commit 5000091 to get more accurate results

@@            Coverage Diff             @@
##             main    #1730      +/-   ##
==========================================
- Coverage   91.72%   91.71%   -0.01%     
==========================================
  Files         139      139              
  Lines        7128     7137       +9     
  Branches     1433     1434       +1     
==========================================
+ Hits         6538     6546       +8     
- Misses        590      591       +1     
Files Coverage Δ
...try-instrumentation-redis-4/src/instrumentation.ts 81.00% <85.71%> (+0.37%) ⬆️

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for taking care of this so quickly - and thanks for the detailed explaination on the original issue. 🙂

@pichlermarc pichlermarc added bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies pkg:instrumentation-redis-4 labels Oct 12, 2023
@pichlermarc pichlermarc merged commit d953531 into open-telemetry:main Oct 13, 2023
@dyladan dyladan mentioned this pull request Oct 13, 2023
@trentm trentm deleted the tm-fix-redis4-multi-crash branch October 13, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:instrumentation-redis-4 priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redis 4.x.x instrumentation causes application to crash when watched keys change as errors are thrown twice
3 participants