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

fs.exists with util.promisify causes to unhandled rejection #1185

Closed
nodify-at opened this issue Sep 19, 2022 · 1 comment · Fixed by #1222
Closed

fs.exists with util.promisify causes to unhandled rejection #1185

nodify-at opened this issue Sep 19, 2022 · 1 comment · Fixed by #1222
Assignees
Labels
bug Something isn't working pkg:instrumentation-fs priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies

Comments

@nodify-at
Copy link

nodify-at commented Sep 19, 2022

What version of OpenTelemetry are you using?

"name": "@opentelemetry/api",
"version": "1.2.0"

Latest installed globally.

Installed dependencies:
npm i --unsafe-perm -g @splunk/otel @opentelemetry/api @opentelemetry/instrumentation-http @opentelemetry/instrumentation-express @opentelemetry/instrumentation-pino @opentelemetry/instrumentation-winston @opentelemetry/instrumentation-nestjs-core @opentelemetry/instrumentation-ioredis @opentelemetry/instrumentation-fs opentelemetry-instrumentation-mongoose @opentelemetry/instrumentation-dns @opentelemetry/instrumentation-amqplib

Note: Also tried to install everything except fs but same behaviour.

What version of Node are you using?

v16.17.0

What did you do?

Reproducing is easy:

test.mjs

import * as fs from 'fs';
import * as util from 'util'; 

const result = await util.promisify(fs.exists)('./index.js');
console.log(result);

What did you expect to see?

fs.exists should return 'true' as a resolved value and not as a rejected value.

What did you see instead?

Executing node with node -r @splunk/otel/instrument test.mjs causes to this error:

node:internal/process/esm_loader:97
internalBinding('errors').triggerUncaughtException(
^
true
(Use node --trace-uncaught ... to show where the exception was thrown)

Without -r @splunk/otel/instrument it works as expected.

Additional context

It looks like the promisified fs.exists is handled incorrectly (CB parameters) by opentelemetry and Promise.reject is called with (true) instead of resolve.

@nodify-at nodify-at added the bug Something isn't working label Sep 19, 2022
@rauno56 rauno56 added priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies pkg:instrumentation-fs labels Sep 20, 2022
@rauno56 rauno56 self-assigned this Sep 20, 2022
@rauno56
Copy link
Member

rauno56 commented Oct 7, 2022

fs.exists is the only fs function that defines it's custom promisify function and that probably causes the issue... Looking into it.

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-fs priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants