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

incompatible with socket.io@3 and @4 - Exception thrown "TypeError: Cannot read property 'on' of undefined" #649

Open
frostmar opened this issue Mar 26, 2021 · 5 comments

Comments

@frostmar
Copy link

The socket.io probe https://github.com/RuntimeTools/appmetrics/blob/master/probes/socketio-probe.js works fine with socket.io@2

However socket.io was rewritten for it's v3 and v4. The socket.io interface has changed slightly, causing the following exception to be thrown when socket.io is require'd and the probe attempts to hook in:

FATAL Node server exiting due to exception: TypeError: Cannot read property 'on' of undefined
                        at C:\firefly\repos\firefly-ui\node_modules\appmetrics\lib\aspect.js:56:26
                        at Array.forEach (<anonymous>)
                        at Object.exports.before (C:\firefly\repos\firefly-ui\node_modules\appmetrics\lib\aspect.js:55:9)
                        at SocketioProbe.attach (C:\firefly\repos\firefly-ui\node_modules\appmetrics\probes\socketio-probe.js:79:12)


I've done a little debugging and can see:

  • socket.io@2 exports a function with a prototype containing the methods the probe wants to patch.

  • socket.io@4 exports a factory function. The class is available as as export .Server (ie requre('socket.io').Server) however it's now a proper javascript Class, and it's constructor can't be hooked in the way used previously.

@frostmar
Copy link
Author

frostmar commented Mar 26, 2021

Until this is fixed, is there any way to prevent the socket.io probe from installing? We only use data from other probes.

@mattcolegate
Copy link
Member

@frostmar
Copy link
Author

appMetrics.disable('socketio') doesn't help. It looks to me that all probes are installed very early, by the appmetrics index.js, and whether a probe is disabled doesn't affect it attempting to hook in during the require of the target library.

Our code is approximately:

    appMetrics = require('appmetrics')
    appMetrics.configure({ mqtt: 'off' })
    const appMetricsMonitor = appMetrics.monitor()
    appMetrics.disable('socketio')

    // ... later ...
    require('socket.io')    

@EMJzero
Copy link

EMJzero commented May 16, 2021

Same issue, I need socket.io v3, and that impedes me to use appmetrics-dash due to this issue with appmatrics. I used it in the past, before using v3, and I'd like to continue...
Has this problem been acknowledged by the devs?

@streboryaj
Copy link

If you require socket.io BEFORE appmetrics then it will not be instrumented by appmetrics regardless of the later disable call.

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

No branches or pull requests

4 participants