#73 Adopt Stream API

[xixixao]

Implements #73

Points to discuss:

• data is raw utf-8 string, not a buffer, because socket.io doesn't support binary.
• how should timeout and "* error" be handled against the nodejs api?
  • + replace onClose calls by onEarlyClose which emits error and calls onClose
  • - changes socket.io api, sorta (some stuff in dev examples)
  • _ I think we should only emit "end" on server close, transport close

Also:
emitting 'error' before 'close' makes tests fail
socket.io 'close' clashes with node api

should we support setEncoding method?

• + yes, so that socket is pluggable
• - no, so people dont expect encodings we dont support (can we support
  other encodings?)
• _ unless we can support different encodings, I would not support it, or
  throw an error when user tries to call it

we can't support pause and resume (surely?) - or do we just dummy pause?

Includes Stream API event emitting and fix from socketio/socket.io#476

[rauchg]

data is raw utf-8 string, not a buffer, because socket.io doesn't support binary.

This is fine for the moment.

_ I think we should only emit "end" on server close, transport close

Is this consistent with what Node does?

should we support setEncoding method?

I think maybe not for the moment.

socket.io 'close' clashes with node api

How so ?

[xixixao]

_ I think we should only emit "end" on server close, transport close

Node Stream emits either error or end, but right now in Socket most errors go through onClose without onError (that's only one type of error) and therefore they won't be reported to Stream API as errors, only as ends.

I would have to do a "switch" depending on the reason string or rather rewrite the errors to go through a separate handler.

Question is how important is to report these as errors (ping timeout, client error, parse error).

Also, as I noted, emitting error before close breaks some of the error tests:

1) server close should trigger if a poll request is ongoing and the underlying
   socket closes, as in a browser tab close:
  Error: poll connection closed prematurely
  at XHR.Transport.onError (EngineIO\lib\transport.js:78:15)

---

socket.io 'close' clashes with node api

nvm

[rauchg]

I think we need to address that. We always need to emit error if there was an error, followed by close. For the purposes of compatibility with the Node API, we can emit end upon examination of the close reason.

This new behavior also merits new tests separate from the stream tests as well just to retain our sanity.

[xixixao]

Ok, let me add it to this request.

[roamm]

send() or flush() doesn't necessarily really flush writeBuffer to the underlying transport, so data may get lost.

[xixixao]

Close does not close the immediately, so no data will be lost.

[roamm]

For websocket, close() immediately invokes the close method of the underlying socket. And at the time, this.writable might be false, causing the send() or flush() buffers the data only in this.writeBuffer, not into the underlying socket.

[tarqd]

+1, this would be really useful especially if you want to do things like have a dnode client/server over engine.io
If anyone is looking for this functionality now it looks like this package provides a shim for it

[mikermcneil]

Sexy- let me know if I can help.

[xixixao]

@mikermcneil I haven't found the time to update this to the new node stream API. If anyone does find the time, this is one of the things that are missing for socket.io 1.0 release.

[rauchg]

We definitely need to prioritize this.