Skip to content

Return responses before an error in the order they were parsed - #509

Merged
ericmj merged 1 commit into
mainfrom
return-responses-in-parse-order
Sep 24, 2026
Merged

ericmj merged 1 commit into
mainfrom
return-responses-in-parse-order

Conversation

@ericmj

@ericmj ericmj commented Sep 21, 2026

Copy link
Copy Markdown
Member

stream/2 and recv/3 document responses in {:error, conn, reason, responses} as "a list of responses that were correctly parsed before the error", in the same order as the {:ok, conn, responses} list. HTTP1.handle_data/2 and the two HTTP/2 catch clauses reversed the accumulator only on the success path, so on the error path the list came back newest first: {:done, ref} before {:status, ref, 200}.

A caller that walks the list in order, or takes List.first/1 to find the status, gets the wrong answer, and only on the error path, which is the path least likely to be covered by a caller's tests.

Two tests, one per protocol: a chunked HTTP/1 response with a bad chunk size after the first chunk, and an HTTP/2 response followed by a GOAWAY. The existing max_header_list_size test in test/mint/http1/conn_test.exs and the WINDOW_UPDATE test in test/mint/http2/conn_test.exs asserted the reversed order and now assert parse order.

HTTP1.handle_data/2 and the HTTP2 stream/2 and recv/3 catch clauses
reversed the accumulated responses only on the success path, so the
responses in {:error, conn, error, responses} came back newest first.
The docs describe them as the responses parsed before the error.
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 0

Coverage remained the same at 88.627%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 3 of 3 lines across 2 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1741
Covered Lines: 1543
Line Coverage: 88.63%
Coverage Strength: 561.55 hits per line

馃挍 - Coveralls

@ericmj
ericmj merged commit 85fe1bd into main Sep 24, 2026
3 checks passed
@ericmj
ericmj deleted the return-responses-in-parse-order branch September 24, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants