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

Cannot use on latest FreeBSD due to DTrace failure #1676

Closed
lapo-luchini opened this issue Jun 8, 2018 · 4 comments
Closed

Cannot use on latest FreeBSD due to DTrace failure #1676

lapo-luchini opened this issue Jun 8, 2018 · 4 comments
Labels

Comments

@lapo-luchini
Copy link

Bug Report

Restify Version

7.2.1

Node.js Version

10.3.0

Expected behaviour

Start correctly.

Actual behaviour

% node index              
Fails to start:
/home/lapo/test/node_modules/restify/lib/dtrace.js:68
        PROVIDER.enable();
                 ^

Error: failed to load DOF: No such file or directory
    at exportStaticProvider (/home/lapo/test/node_modules/restify/lib/dtrace.js:68:18)
    at Object.<anonymous> (/home/lapo/test/node_modules/restify/lib/dtrace.js:80:3)
    at Module._compile (internal/modules/cjs/loader.js:702:30)

Repro case

I minimized the test case from restify's dtrace.js file:

var dtrace = require('dtrace-provider');
PROVIDER = dtrace.createDTraceProvider('restify');
PROVIDER.addProbe('route-start', 'char *', 'char *', 'int', 'char *', 'char *', 'json');
PROVIDER.enable();

Cause

DTrace instantiation is protected by try/catch but when DTrace is not available in the kernel (as the module is not loaded) it actually fails on PROVIDER.enable(), which is outside of the try.

Are you willing and able to fix this?

Yes, I could do a PR that extends the try to the whole section.
This is connected to chrisa/node-dtrace-provider#117 but we can probably avoid it on our side that way.

@retrohacker
Copy link
Member

Thank you for the awesome detective work on this @lapo-luchini!

If this can throw I’m in favor of catching as long as we can do it in a way that doesn’t leave the dtrace module in a half-instatiated state. The code should gracefully fallback to dropping dtrace support in the same way that it does if the install isn’t there.

CC’ing @yunong since I think he has a much better understanding of the dtrace provider than I do.

@lapo-luchini
Copy link
Author

Turns out fixing restify's dtrace.js is not enough, as it is used (without try/catch) also in bunyan.

@stale
Copy link

stale bot commented Aug 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Aug 10, 2018
@stale
Copy link

stale bot commented Aug 24, 2018

This issue has been automatically closed as stale because it has not had recent activity.

@stale stale bot closed this as completed Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants