WIP: Socket.IO protocol on top of Engine.IO - #6
Draft
Redouane64 wants to merge 21 commits into
Draft
Conversation
Both transports hardcoded "/engine.io", which is where a bare Engine.IO server listens but not where a Socket.IO server carries the same protocol. ClientOptions.Path now names it and both transports take it. The normalized form keeps a trailing slash. That is not cosmetic: a server normalizes its own path the same way and matches it against the start of the request, so a Socket.IO server answers "/socket.io/?EIO=4" and 404s "/socket.io?EIO=4". The bundled Engine.IO sample accepted the slashless form only because it forwards every request past engine.io's own path check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
Socket.IO encodes its own packets and has to say which Engine.IO packet carries the result, a decision the text and binary overloads make on the caller's behalf. It also encodes straight to bytes, so the new plain-text factory overload spares it a transcode through a string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
Both target net10.0 and carry no PackageReference of their own: everything they need is in the shared framework or in Directory.Build.props, and adding one back would trip NU1510. The test project mirrors EngineIO.Client.Tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
PacketType values are the ASCII byte of the digit they are written as, the
same trick the Engine.IO packet uses, so encoding a type is a cast.
Serialization writes into a caller-owned buffer and holds no state of its
own, which is what lets the shared Connect and Disconnect packets be sent
more than once. Three details the format insists on:
- a namespace is normalized to its leading-slash form on construction, so
"admin" and "/admin" cannot encode two different ways;
- an acknowledgement answers an event rather than naming one, so only the
event types seed an event name into the payload;
- binary never travels inside the JSON. Each attachment leaves a
placeholder behind and is exposed for the caller to send separately.
AddItem takes a byte array explicitly. Without that overload an array binds
to the generic one and ships as a base64 Json string with no complaint.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
The expected wire strings are the specification these tests exist to pin down, including the cases the format is easy to get wrong: a namespace however it was spelled, an acknowledgement payload without an event name, attachment numbering, text and binary mixed in one payload, a byte array staying binary, and the same packet encoding identically twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
Sending is complete: the header goes out as a plain-text Engine.IO message and each attachment follows as its own binary message, in the order its placeholder named it. Receiving is not. ListenAsync says so rather than yielding something it cannot produce, and the namespace map it would fill is waiting on the same missing piece — nothing parses an inbound packet yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
A second npm workspace beside the Engine.IO one, listening on 9855 so both can run at once, echoing "message" events and acknowledging them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wpg9UqbFRzXvmpp2Msbieu
The WebSocket URI is built by concatenating the base address with the normalized path, and that path already opens with a slash. A base address the caller ended with one — "http://host/" — produced "//socket.io/", which a server matching on the start of the request path does not recognise: it answers 404. Long-polling was immune, because it resolves a relative Uri against HttpClient.BaseAddress and that normalizes, so the failure showed up as the confusing "polling works, the upgrade silently doesn't". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
Connected read _transport.Connected, and _transport does not exist until ConnectAsync has run, so asking before connecting threw a NullReferenceException. That pushed callers into keeping a shadow flag of their own, which then had to be maintained by hand. ConnectAsync reports a failure by completing the packet stream with its reason rather than by throwing, which suits a listener but leaves a caller that only ever sends with no way to see it. Record the reason so a protocol layered on top can decide whether it may send. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
_transport was repointed at the WebSocket transport before the probe had been answered, and a failed upgrade then tore the whole session down. Both are wrong: the upgrade is an optimisation over a polling connection that is already working, and the protocol says to carry on with long-polling when it does not succeed. Build the transport into a local, hand it the connection only once it has taken over, and dispose it otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
The packet channel and the polling cancellation source are both one-shot, and whatever ends a connection — a handshake that failed, a close the server sent, a heartbeat that ran out — completes and cancels them. A second ConnectAsync therefore handshook successfully and handed back a connection whose receive loop exited on its first check, so it could send but never receive. Replace both when connecting on top of a connection that has ended. Full reconnection — backoff, rejoining, resuming a session — is still to come; this only stops a retry being quietly broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
The Socket.IO tests were written in camelCase with a public modifier, while every Engine.IO test is a Pascal_Snake_Case method with no access modifier. Two conventions in one solution make a test list harder to read than either would on its own. Renames only: no test was added, removed or changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
Five shapes encoded cleanly here and were then thrown out by the server.
A decode error is not a dropped message: socket.io answers one by closing
the connection, so each of these cost the whole session.
- A binary packet announces its attachment count, and the decoder
refuses a count below one. Both the marker and the requirement were
missing: a BinaryEvent carrying only text encoded as `5["a","b"]`
("Illegal attachments").
- A negative ack id encoded as `2-1[...]`; the parser reads digits
only, so the sign began the payload.
- A namespace holding a comma split the header, the comma being what
ends the namespace: `2/a,b,["message"]`.
- An Ack with no id encoded as `3[...]`, which the server looks up
among the callbacks it is waiting on and discards.
- An event named after a reserved one — connect, disconnect and the
rest — was refused outright as an invalid payload.
Fold the four constructors onto one private constructor so that every way
in is validated, and refuse an attachment-less binary packet when it is
serialized, since its attachments arrive after it is built.
Checked each encoding against socket.io-parser, the reference decoder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
IO had no seam, so nothing about the client itself was under test: only the packet encoding was. Give it the internal HttpClient constructor that Engine and the transports already expose for their tests, and grant SocketIO.Client the InternalsVisibleTo it needs to reach Engine's. The handler is hand-rolled rather than mocked: the ordering tests need the requests as a sequence, and this project takes no Moq dependency. Pins what the client does today — the CONNECT packet, the namespace form, the path it asks for, and a binary event going out as a header followed by its attachment — before any of it changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
Engine.ConnectAsync completes normally whether or not the handshake worked, and IO took that as proof, setting a _connected flag it never cleared. Against a server that was down, the client latched: the caller saw an HttpRequestException from the CONNECT send rather than a connect failure, and every later ConnectAsync skipped the handshake, so it stayed unusable even once the server came back — a second attempt posted CONNECT with no sid and was answered 400. Drop the flag and ask the engine, which now answers before it has a transport and answers false again once the connection has gone. A handshake that did not come up is raised as IOConnectionException, carrying the engine's reason, and sending before connecting says so instead of dereferencing a transport that is not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
A binary packet is several Engine.io packets that a decoder reads as one run: the header, then exactly as many binary packets as it announced. The transports serialize sends one at a time, but nothing held a run together, so two concurrent SendAsync(byte[]) calls could put hdrA, hdrB, attA, attB on the wire. The decoder fails that on the second header — "got plaintext data when reconstructing a packet" — and the server closes the connection. Verified by removing the lock again: the test fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
Two things this session leaned on that are not obvious from the code: socket.io-parser in node_modules settles what the wire accepts faster and more reliably than the spec does, and SocketIO.Client now reaches EngineIO.Client's internals so IO can be driven from a stubbed handler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
Packet was an accumulator: construct it, AddItem until the payload is complete, serialize. None of that describes what arrives from the server, where the arguments already exist and want reading. A single type would have had half its members meaningless in either direction. So the outbound type becomes PacketBuilder, which is what it always was, and Packet is now the inbound one — the plain name going to the type consumers actually handle, since every listener writes `await foreach` and only some code builds a packet by hand. IO.ListenAsync already declared IAsyncEnumerable<Packet> while throwing; that signature is now true. Packet carries its public API and no implementation yet, so the surface can be reviewed before the parser is written against it. The rename and the new declaration land together because ListenAsync's signature names both, and neither compiles alone. The grammar predicates move onto PacketType, where they describe the thing they are about and neither side can drift from the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
A binary packet is a header followed by exactly as many binary Engine.io messages as it announced. Parsing cannot own that: TryParse is a function of one buffer, and making it remember what came before would make a parse call depend on the order it was made in. Decoder holds the header back until the attachments have arrived. Its two errors — an attachment with no header waiting, a header arriving while one is still owed — are the pair socket.io's own decoder raises, and both mean the run of messages making up a packet has been broken into. They are raised rather than swallowed because a peer answers an undecodable packet by closing the transport. Only the attachment-side error can be exercised yet; everything through AddHeader waits on Packet.TryParse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
ListenAsync threw. It now hands back a stream per namespace, fed by a single loop that drains Engine.ListenAsync through the Decoder. One loop rather than one per listener: the engine exposes a channel, so two listeners draining it would compete and each would see roughly half the packets. The queues are created on demand, which also means a listener that subscribed before connecting misses nothing. Route splits protocol concerns from consumer concerns the way PollAsync does for the heartbeat: CONNECT, DISCONNECT and CONNECT_ERROR belong to the client rather than the caller, and are marked for the namespace work that will own them. A packet for a namespace nobody listens to is dropped rather than buffered, since buffering it would grow without limit. A listener that subscribes after the stream has ended is completed rather than left waiting — found by the disposal test, which passed alone and failed under the parallel run where disposal won the race. Everything here is exercised for its lifecycle only. Nothing decodes until Packet.TryParse exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP. Sending works end to end against a real Socket.IO server; receiving is not implemented yet. Opening early so the packet model and the Engine.IO changes underneath it can be reviewed before the parser is built on top.
This supersedes the earlier
features/socketiobranch, which forked ~40 commits back and predates the .NET 10 migration, the protocol-compliance pass and the test-coverage pass. Its concepts and structures are carried over here, rebuilt on currentmainand on the API it actually has. That branch's tip wasb513982if any of it is wanted back.Done
Engine.IO, to make room for a protocol on top of it
ClientOptions.Path— both transports hardcoded/engine.io, which is not where a Socket.IO server carries the protocol. The normalized form keeps a trailing slash: a server normalizes its own path the same way and matches it against the start of the request, so a real Socket.IO server answers/socket.io/?EIO=4and 404s/socket.io?EIO=4. The bundled Engine.IO sample accepted the slashless form only because it forwards every request past engine.io's own path check, which is why nothing caught this before.Engine.SendAsync(Packet)and a plain-textPacket.CreateMessagePacket(ReadOnlyMemory<byte>)— a layered protocol encodes its own bytes and has to say which Engine.IO packet carries them.Engine.Connectedanswers before there is a transport instead of throwing, andEngine.ConnectionErrorrecords why an attempt failed.ConnectAsyncreports failure by completing the packet stream with the reason, which suits a listener but leaves a caller that only ever sends —IO— with nothing to check._transportis repointed only once the socket has taken over./no longer doubles the slash in the upgrade URI. Polling was immune — it resolves a relativeUriagainstHttpClient.BaseAddress, which normalizes — so this showed up as "polling works, the upgrade silently doesn't".Socket.IO
src/SocketIO.Client— packet type, packet model, payload arguments (text, Json, binary placeholder), and the client surface.ConnectPacketincluded — encodes identically every time it is sent.adminand/admincannot encode two different ways.{"_placeholder":true,"num":N}behind, andIOsends the header as a plain-text Engine.IO message followed by one binary message per attachment, under a lock so the run cannot be broken into. Text and binary arguments can be mixed in a single payload.AddItemandSendAsync. Without them an array binds to the generic Json overload and ships as a base64 string with no complaint — worth keeping in mind when adding overloads here.IOConnectionExceptioncarrying the engine's reason, rather than being reported as a connection.npm run start:socket-server, port 9855).Protocol compliance. The encoder was checked against
socket.io-parser— the reference decoder, innode_modules— rather than against the spec prose. Five shapes encoded cleanly here and were refused by the server, and a decode error is not a dropped message: socket.io answers one by closing the connection.5["a","b"]Illegal attachments— the count must be ≥ 12-1[…]invalid payload— the sign starts the payload,2/a,b,[…]invalid payload— the comma ends the namespaceAckwith no id3[…]2["disconnect",…]invalid payloadAll are refused at construction now, bar the attachment rule, which is checked when the packet is serialized because its attachments arrive after it is built. Two client-level defects went with them:
IOlatched a failed handshake as a connection and stayed unusable even once the server came back, and a header and its attachments were not held together, so two concurrent binary sends could interleave and cost the connection.Tests. 67 Socket.IO and 86 Engine.IO, 153 in all and green; every commit builds and tests green on its own.
IOnow has the same internalHttpClientseamEngineand the transports use, so the client is under test and not only the encoder; its stub is hand-rolled, since this project takes no Moq dependency.Verified against
socket.io@4.8, not only in unit tests: handshake on/socket.io/from a base address written with a trailing slash, upgrade to WebSocket,40CONNECT accepted into the namespace, a text event, a Json event, and eight concurrent binary events the server reassembled one by one, then41DISCONNECT.Remaining, roughly in order
Packet.TryParsesits on the Engine.IO side.Nattachments have arrived, then substitute the placeholders.ListenAsync. Currently throws. Should drain the Engine.IO message stream into per-namespaceIAsyncEnumerable<Packet>, in the shapeEngine.ListenAsyncalready established.0{"sid":"..."}reply, route inbound packets to the right listener, and reject sends to a namespace that was refused.Task<T>, and answer server-initiated acks.CONNECTauth payload.0{"token":"..."}— currently no payload is allowed on a Connect packet.CONNECT_ERROR. Surface it to the caller rather than letting a namespace connect hang.JsonPacketData<T>uses reflection-basedSystem.Text.Json, which throws outright in a trimmed or AOT app. Taking aJsonTypeInfo<T>would let callers pass a source-generated context, as the Engine.IO handshake already does.🤖 Generated with Claude Code
https://claude.ai/code/session_01CsxPBM9F82FBXVQDV1jFmN