Bug: fatal crash from unhandled 'error' event

[sphlabs]

• I'm submitting a ... [X ] bug report [ ] feature request [ ] question about the decisions made in the repository [ ] question about how to use this project

• Summary When trying to connect to a Hyperdeck that does not exist at the specified IP address, an unhanded error is thrown, causing a crash.

• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. StackOverflow, personal fork, etc.) Error log:

`events.js:353 throw er; // Unhandled 'error' event ^

Error: connect ETIMEDOUT 192.168.1.163:9993 at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1146:16) Emitted 'error' event on Hyperdeck instance at: at Socket. (/Users/stephen/Developer/companion/module-local-dev/companion-module-bmd-hyperdeck/node_modules/hyperdeck-connection/dist/hyperdeck.js:53:45) at Socket.emit (events.js:376:20) at emitErrorNT (internal/streams/destroy.js:106:8) at emitErrorCloseNT (internal/streams/destroy.js:74:3) at processTicksAndRejections (internal/process/task_queues.js:82:21) { errno: -60, code: 'ETIMEDOUT', syscall: 'connect', address: '192.168.1.163', port: 9993 } `

[nytamin]

This is kind of by design, try adding this to your code (I'll also update the README to clarify that this is needed), to catch any emitted errors.

myHyperdeck.on('error', (err) => {
    console.log('Hyperdeck error', err)
});

[sphlabs]

Hmm, I already have an error handler in our code:

https://github.com/bitfocus/companion-module-bmd-hyperdeck/blob/3da011613b536b0c8511c29028ca256ef7c7c5fb/index.js#L1011

Can you see any reason why this wouldn't be handling the error above?

[Julusian]

When you recreate the socket you are also removing all the event listeners. It looks like we arent handling an in-flight connection attempt when doing the disconnect, and also you arent waiting for the disconnect to complete before removing the listeners, so an error can slip through there

https://github.com/bitfocus/companion-module-bmd-hyperdeck/blob/3da011613b536b0c8511c29028ca256ef7c7c5fb/index.js#L996 ?

[sphlabs]

The (or at least a) situation we have that is causing crashes is as follows:

1. Try to open a connection to a deck that does not exist on the network
2. Change the IP address, which causes the connection to be destroyed and re-created
3. Eventually, a timeout error comes back, but is not handled because the original connection was destroyed

I don't think there is any way that we can catch the error in 3, because we don't know that one will ever arrive. We can't wait for a disconnected event, because one of those will never arrive as a connection was never made.

So I think the hyperdeck-connection module needs to somehow handle the error in this scenario? Or when hyperDeck.disconnect() is called, any open connection attempts are terminated?

This is the code we are using to reset our connections: if (this.hyperDeck !== undefined) { this.hyperDeck.disconnect() this.hyperDeck.removeAllListeners() delete this.hyperDeck }

[Julusian]

@sphlabs I have made a few changes to help with this:

• the code now uses socket.destroy() rather than socket.end(). Hopefully that wont have side effects, the difference being we now fully close the socket, rather than close our half and hope the other end does the same.
• when disconnect() was called, if it hadnt reached the connected state, socket.end() was never called, which is likely why the error was being emitted. socket.destroy() is now always called here
• Ive wrapped all the other emitted errors in a check to make sure that if disconnect has been called, they will be ignored

Please try with 0.5.0-nightly-latest-20220216-192753-87348bf.0

[sphlabs]

Great, that seems to have fixed things on this end -- at least for the scenarios I am aware of.