fix(protocol): return only the error body when a Block2 transfer fails - #61
Merged
Conversation
Block2Accumulator.add_response extended the accumulated representation with a non-success response's body before completing, so a 4.xx or 5.xx arriving mid-transfer produced one buffer holding assembled blocks followed by a diagnostic in the server's own format. get() returns (code, payload) with no boundary marker between the two, leaving the caller no way to split them. The error body now replaces the partial representation. Nothing observed the old behaviour: every caller gates on 2.05 before decoding, so the accumulated bytes had no reader once the code was not 2.xx. Where the error arrives before any block, this is unchanged from #36, which already returned the diagnostic where the pre-#36 path returned an empty buffer. This makes that the behaviour in every case. The payload bound moves with it, from the combined length to the error body's own, since the buffer now only ever holds one or the other.
This was referenced Aug 23, 2026
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.
Follow-up to #36, on one contract detail I raised in review and then took myself.
Block2Accumulator.add_responseextended the accumulated representation with a non-success response's body before completing. A 4.xx or 5.xx arriving mid-transfer produced one buffer holding assembled blocks followed by a diagnostic in the server's own format, andget()returns(code, payload)with no boundary marker between them, so the caller had no way to split it.The error body now replaces the partial representation.
Nothing observed the old behaviour
Every
get()consumer gates on the code before touching the payload:bridge.py:253,:486and:516all check for 2.05 first. Once the code is not 2.xx the accumulated bytes had no reader.The first-block case is unchanged
Where the error arrives before any block, #36 already returned the diagnostic body where the pre-#36 path returned an empty buffer. That was an improvement and it stays. This change makes it the behaviour in every case.
Bounds
The payload bound moves from the combined length to the error body's own, since the buffer now only ever holds one or the other. An error body that fits is accepted however many bytes arrived before it; an oversized one still raises
BlockwiseError.Tests
Two tests pinned the old contract and both are updated, one at the accumulator and one driving it end to end through
get():test_mid_transfer_error_replaces_the_partial_representationtest_get_returns_only_the_error_body_when_a_transfer_fails_mid_wayTwo added:
test_error_before_any_block_returns_the_diagnostic_body, so the case feat(protocol): discover advertised OCF secure ports #36 improved cannot quietly revert to the pre-feat(protocol): discover advertised OCF secure ports #36 empty buffertest_block2_accumulator_enforces_exact_block_and_payload_bounds, covering the loosened check in both directions403 pass. Ruff clean on all three touched files.
Hardware
Not separately validated. #36 itself ran on the reference bridge against one dryer and one oven across a container recreate, and this path needs a mid-transfer 4.xx from the appliance to fire, which neither device produced during that run.