Open dmcgowan opened 9 years ago
Ping @mcollina for review of initial draft
Matteo, I am still uncertain about the method for doing acks, you were as well. Think of possibly keeping acks much simpler, a simple integer message, 0 for non-error ack, and > 0 for error ack. When an error is received a string message can follow, then a remote close. If no string message is given then the close should still happen resulting in an EOF. The simple integer type will work well with msgpack and cbor as most messages will only end up being a single byte.
Sorry for not reading this through sooner, I was stuck in the consulting businness :(. I think we should add a header for the message encoding. This is necessary to ensure things works with different encodings.
I propose to add a mime type of application/libchan+msgpack5
or application/libchan+cbor
.
Should I send a PR against this PR with my updates?
Thanks for adding me as an author :dancers:.
Excellent point, any newly created stream which will send data should have a content-type.
Types
application/vnd.libchan.send+msgpack5
- Encoded messages in msgpack5application/vnd.libchan.recv+msgpack5
- Encoded responses in msgpack5application/vnd.libchan.send+cbor
- Encoded messages in cborapplication/vnd.libchan.recv+cbor
- Encoded responses in cborapplication/octet-stream
- Byte streamUpdated based on original feedback, still need to update for content types
:+1: for the types.
Just one question: what happens if the other party does not support that encoding?
I think the ":status" as used currently in spdy and defined in HTTP is missing here. I will add ":status" as the response header which should contain an HTTP status code. In the case of unsupported content-type, a 415 would be used.
Hmm looking more at the streams interface, I am thinking streams will need to have a function to handle the reply unless it would be expected the stream provider would be configured to know how to accept content-types and expose that to the stream user. My goal would be after a new stream is accepted, it is always good and would never need to explicitly failed by the stream user because of invalid content-type.
Finally pushed the update. Going to revive this and get this work completed.
Updated with last comments and content types
Pushed an update to the streams library in support of this change https://github.com/dmcgowan/streams/commit/f02a9e7b00caabc2b0bc3d166a329354ea57ce4d. This will enable proper content type handling by libchan.
Update to the protocol as a result of libchan meeting with Matteo Collina. Bump the version to 0.2.0 and name the set the previous version as 0.1.0.
Protocol changes:
Other changes: