Fix seven protocol shape bugs, and sweep every request to find the next one - #26
Merged
Conversation
A payload that arrives and cannot be read came back as null, so the caller was told OBS had returned nothing. GetCanvasList over MessagePack reported a missing formatter as a server-side problem for every user. The receive loop keeps the tolerant form: one unmodellable event from a newer OBS must not tear the connection down.
Three arrays were read as the wrong stub and failed on required fields the payload never carries, so the messages never surfaced at all: - GetCanvasList had no stub, so MessagePack had no formatter for it. - InputVolumeMeters was read as InputStub; the meter payload has no input kind. - SceneItemListReindexed was read as SceneItemStub; the reindex event asks OBS for the basic list, which is id and index only. The generator keyed the mapping on the field name alone, so payloads sharing a name shared a stub. Parent-specific cases come first now.
GetInputAudioTracks could not be read and SetInputAudioTracks could not be sent, on MessagePack only. Found by a new sweep in the example that calls every read request and reports the responses it cannot deserialize, plus a log sink that fails the run on any unreadable payload, since a dropped event is silent by design. Two tests now walk the generated surface for types with no formatter, which is the shape both this and GetCanvasList had.
SetTBarPosition took an int for a 0.0 to 1.0 position, so only the two ends of the T-bar were reachable. The protocol states the range with a decimal point, which the generator now treats as the protocol saying the field is fractional, and refuses to build if the table disagrees. SetSceneItemTransform applies only the fields present, but the request took the full transform with every member required, so a partial one could not be expressed and a transform read back from OBS was refused. Requests take a patch type now; responses keep the full one. A request declaring no response payload no longer tries to read one. OBS sends a payload for some of them and there is no metadata for object, so ToggleRecordPause failed a request that had succeeded. Unmapped arrays, unclassified numbers, and a declared string enum with no field mapped are errors now rather than notes.
The read sweep ran against whatever input happened to be first, so the audio, media and property requests were declined rather than exercised, and a scene collection made fresh in the UI has no inputs at all and failed the suite outright. Both sweeps now create what they need: a scene, an audio input, a media input and a filter, plus studio mode. Read coverage goes from 52 to 59 of 60. Volume meters seeds its input into the program scene, since OBS meters only the inputs it considers active. SetOutputSettings is no longer sent: writing settings to a real output wedged the output subsystem for the rest of the session.
GetSourceFilterList had been dropped from the sweep when the filter discovery was replaced by a fixture, and the count did not notice because it was a running total against a hardcoded 60. It counts distinct request types now and fails if fewer than 60 are accounted for, so a probe cannot go missing quietly again. The write sweep reports what it actually covers: a request OBS declines still proves it serialized, which is the thing under test, so the total is stable across transports even when machine state is not.
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
1 participant
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.
Seven bugs, all shipping in 0.4.1-dev1. Six were invisible until something read the payload; the seventh was in our own numeric table.
Closes #24.
Fixes
GetCanvasListhad no stub, so it generated asList<JsonElement>, which no MessagePack formatter could build. Unreadable on that transport, fine on JSON.InputVolumeMeterswas read asInputStub. The meter payload carries name, uuid and levels, so it failed on the input kind it never sends. The event never fired, on either transport.SceneItemListReindexedwas read asSceneItemStub. The reindex event asks OBS for the basic scene item list, which is id and index only. Also never fired.GetInputAudioTracks/SetInputAudioTracksuseDictionary<string, bool>, which had no formatter either. Unreadable and unsendable on MessagePack.SetTBarPositiontook anintfor a 0.0 to 1.0 position, so only the two ends of the T-bar were reachable.SetSceneItemTransformrequired every member, but the endpoint applies only the fields present. A partial transform could not be expressed, and a transform read back from OBS was refused.object, soToggleRecordPausefailed a request that had succeeded.The generator keyed array mapping on the field name alone, so payloads sharing a name shared a stub. Parent-specific cases come first now.
Diagnosis, which is what made them findable
A payload that arrives and cannot be read used to come back as
null, so the caller was told OBS had returned nothing. Request paths raiseObsWebSocketSerializationExceptionnow. The receive loop stays tolerant on purpose: one unmodellable event from a newer OBS must not tear the connection down.Guards
OBSWSGEN010(unmapped array) andOBSWSGEN012(unclassified number) are errors, not notes.OBSWSGEN013: a whole-number field whose protocol restriction is written with a decimal point fails the build. That is whatSetTBarPositionwas.OBSWSGEN014: a declared string enum with no field mapped fails the build, so a protocol refresh cannot leave one as a plain string.Both new diagnostics were verified by re-breaking the tables.
Sweeps
run-transport-testsnow calls every request in the protocol and reports what it could not read or send, building the OBS state it needs rather than testing against whatever happens to be there. A log sink fails the run on any unreadable payload, since a dropped event is silent by design.On both transports against OBS 32.2.2: 59 of 60 read requests, 68 write requests serialized, zero failures. The read count is by distinct request type and fails if fewer than 60 are accounted for, so a probe cannot go missing quietly.
Not covered, deliberately:
GetLastReplayBufferReplayneeds a saved replay, and starting the replay buffer to make one crashes OBS (#25).SetOutputSettingsis not sent for the same reason.284 tests on net9.0/net10.0/net11.0, 0 warnings, full forced regeneration clean.