search

mcollina / mows

Using MQTT.js in the browser over WebSocket -- Built with browserify!
72 stars 12 forks source link

client.end() doesn't emit "close" event on server #16

Closed RangerMauve closed 9 years ago

RangerMauve commented 10 years ago

I've got logic hooked into the close events on incoming connections. However if I manually close a socket with client.end() or event client.stream.end(), the close event never gets emitted. A quick fix has been to manually emit the close event, but I wonder if this behavior is intentional.

RangerMauve commented 10 years ago

After falling into the rabbit hole, it appears that client.end() doesn't actually close the socket that's upstream. I'm assuming this is the case based on this

Actually, it seems this behaviour is counter to what the docs say for MQTT.js

mcollina commented 10 years ago

The behavior is not intentional.

The interface is end(), as this package should offer the same interface of MQTT.js. We should find a way to make it work, even changing websocket-stream or MQTT.js. What do you think it's the best route?

RangerMauve commented 10 years ago

I'm currently looking into it at the moment. It seems that websocket-stream doesn't expose the raw WS instance, so it might need to be modified.

I'll look into it further after lunch

RangerMauve commented 10 years ago

Taking a look further, it doesn't look like it's related to websocket-stream since that isn't being touched by the server. From what I'm understanding, we need client.end() to call WebSocket#close() on the actual websocket instance somehow.

Edit: Nevermind, I didn't notice that websocket-stream is being used.

RangerMauve commented 10 years ago

By the looks of it, the object we're getting withing the server handler is is WebSocket -> WebSocketStream -> MqttConnection. close() needs to be called on WebSocket, which is accessible via WebsocketStream#socket, which is piped into MqttConnection, a Writable stream, that references the WebsocketStream as MqttConnection#stream.

RangerMauve commented 10 years ago

So a mows server would have to call client.stream.socket.close() to actually close the socket. With a TCP server in MQTT.js, it's client.stream.[end || destroy]().

mcollina commented 10 years ago

Woa, that was good :). Can you actually hack something so we fix it in here?

RangerMauve commented 10 years ago

Sure, and AFAIK it affects MQTT.js, so I'll give it some love as well.