-
Notifications
You must be signed in to change notification settings - Fork 532
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-fs): fix fs.exists
when it's util.promisified
#1222
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1222 +/- ##
==========================================
+ Coverage 96.08% 96.20% +0.12%
==========================================
Files 14 15 +1
Lines 893 948 +55
Branches 191 195 +4
==========================================
+ Hits 858 912 +54
- Misses 35 36 +1
|
if (fName === 'exists') { | ||
// handling separately because of the inconsistent cb style: | ||
// `exists` doesn't have error as the first argument, but the result | ||
if (isWrapped(fs[fName])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no need to duplicate this if. could be moved up before if (fName === 'exists') {
once for all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! Thanks!
Which problem is this PR solving?
fs.exists
has an unusual signature with the callback never returning an error.To get around that when using
util.promisify
, custom promisifing function is defined.Our instrumentation doesn't take that into account.
Fixes #1185
Short description of the changes
Added separate patcher for
fs.exists
that is aware of the above and define a new custom promisifying function for our patched function.