feat(ws): complete the S6 frame-type rename to module.message/module.error - #354
Merged
Conversation
…error S6 put one extension (Lua) in the wire type, where no second extension could ever reuse it. #330 shipped the mechanism - the producing extension travels in the payload's `module` key - but not the rename, because nova 0.15.1 crashed the connection process on a list-valued reply, so dual-emitting old and new was impossible. novaframework/nova#400 fixed that and asobi carries the fixed nova by git ref (#349), so both frames can now go out on one reply. Extension pushes are `module.message` and `module.error`. `game.message` and `game.error` are emitted alongside them with identical payloads, so every SDK built before the rename keeps working, and are removed at the 1.0 wire break. `asobi.ws_legacy_game_frames` (default true) drops the legacy pair. `game.message` is asobi_lua's `game.send/2`, which a script may call per player per tick, so the compat frame doubles asobi's hottest extension-produced egress path. An operator whose clients all dispatch `module.*` gets that back without waiting for 1.0. Fixtures for both new types; the old two keep theirs. asobi_protocol_coverage_tests learns to read extension_frames/3, which is now the only emit site for either pair.
🟡 Code Coverage — 73.4%5774 of 7863 lines covered. |
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.
Plan item S6, the half of it that #330 could not ship.
What S6 was for
game.message/game.errorput ONE extension (Lua) in the wire type.No second extension could reuse a type named after the first. #330 shipped
the mechanism - the producing extension travels in the payload's
modulekey - and deferred the type rename. The plan gives S6 a hard clock: before
the wire freeze.
The constraint that deferred it is gone
nova 0.15.1's
nova_basic_handler:handle_ws/2consed a list-valued replyonto cowboy's command list as a single command, so
{reply, [F1, F2], S}reached
cow_ws:frame/2as one frame and killed the connection process.Dual-emit was therefore impossible, and a bare rename would have broken
every shipped SDK.
novaframework/nova#400 replaced the cons with a splice, and asobi carries
the fixed nova by git ref (#349). Verified before writing any of this:
Two commands, order preserved, and
nova_ws_handlerhandscommandsto cowboy untouched.
asobi_ws_SUITEproves it end to end over a realsocket, which is the only place the splice is actually exercised.
What this PR does
module.messageandmodule.error.game.messageandgame.errorare emitted alongside them, identicalpayload, same reply, legacy frame first. No shipped SDK breaks.
game.*pair deprecated, says new SDK codedispatches on
module.*, and says the pair is removed at the 1.0 wirebreak.
The hot path, honestly
You asked for the reasoning rather than a default, so:
erroris free. It is dev-mode only (ASOBI_DEV_ERRORS=true), ratelimited to one per second per match, and never emitted in production.
Dual-emitting it costs nothing an operator will ever measure.
messageis not free.game.messageis asobi_lua'sgame.send/2.A script may call it per player per tick; at 30Hz with 8 players that is
240 frames/s per match becoming 480, each with its own
json:encode/1and its own cowboy frame. It is the hottest extension-produced egress
path asobi has, and dual-emit doubles it.
But the alternative is worse. Five of seven SDKs cannot dispatch an
unknown frame type at runtime, and Godot/LOVE users vendor by copying
source, so emitting only
module.messagemakes per-player messagesvanish silently in every shipped build - the exact silent dev-facing
failure class we treat as a defect.
So the split is not "dual-emit error, rename message". It is: dual-emit
both, and give the hot path an exit that does not require waiting for
1.0.
asobi.ws_legacy_game_frames, defaulttrue:false: onlymodule.*. An operator whose clients all dispatch thenew types stops paying the doubling immediately.
One flag, not two, and it governs
erroras well - not for the cost, butbecause an SDK author verifying their new dispatch path wants the old
frame out of the way while they do it. At 1.0 the legacy pair goes and
the flag becomes a no-op.
If you would rather not carry a flag at all, the fallback is unconditional
dual-emit and a note in the performance-tuning guide. I did not pick that
because it leaves a real per-tick doubling with no operator remedy until
a major version.
Ordering
Legacy frame first,
module.*second, so a shipped client seesbyte-identical frame ordering to before this PR. The guide warns not to
dispatch on both - a client handling
game.messageandmodule.messageprocesses every message twice.
The commit-message defect on main
Commit
a6bc2eb("refactor: generalise the two Lua-specific WebSocketframes", #330) is a squash whose message says:
That is not what merged. The second commit inside the same squash
("fix: keep the extension frames to one wire frame per message") removed
the dual-emit because of the nova bug above, and its text is further down
the same message - so the message contradicts itself and the top half is
the part people read. No release before this
change emits
module.*- not v0.54.0, and not v0.53.0 wherea6bc2eblanded.
A merged commit cannot be rewritten, so the correction lives where
somebody would actually look for it:
guides/websocket-protocol.mdnow has a "Wire history" paragraph naminga6bc2eband stating what each release actually emits.Coverage gate
asobi_protocol_coverage_testsscansencode_reply(_, ~"type", _)literals in the handler. Both pairs now go out through
extension_frames/3, where the encode takes a variable, so the scannerlearns
extension_frames(~"new", ~"legacy", _)too. Without that thefour fixtures read as stale (which is how I found it).
Checks
fmt --check,xref,dialyzerclean.eunit1236/1236.ct --suite=asobi_ws_SUITE8/8.Revert verification. Reduced
extension_frames/3to the pre-PRsingle legacy frame: 6 eunit failures (the four payload tests, the
frame-count test, the flag test) and 1 CT failure
(
ws_script_error_rendered_as_extension_error). Restored, all green.One caveat I will not paper over:
no_stale_fixtures_testdid notfail under that revert, because the
~"module.message"literal is stillpresent as an argument in the source the scanner reads. The fixture gate
proves a type name appears at an emit site, not that the frame reaches a
socket. The frame-count eunit test and the CT case are what cover that.
nova_splices_a_list_reply_into_separate_commands_testis a pin guard,not a test of asobi code - it fails if the nova pin is reverted to a
build without #400, which would otherwise only show up as a dead
connection at runtime.