Skip to content

fix(http2): wait when outbound flow control window is negative#1095

Open
l46983284-cpu wants to merge 2 commits into
encode:masterfrom
l46983284-cpu:fix/http2-negative-flow-control-window
Open

fix(http2): wait when outbound flow control window is negative#1095
l46983284-cpu wants to merge 2 commits into
encode:masterfrom
l46983284-cpu:fix/http2-negative-flow-control-window

Conversation

@l46983284-cpu

Copy link
Copy Markdown

Summary

HTTP/2 request bodies could fail with LocalProtocolError: Cannot send N bytes, flow control window is -M when h2 reported a negative local flow-control window. _wait_for_outgoing_flow only waited while flow == 0, so negative windows were returned as max_flow and send_data raised.

This change waits on any non-positive flow (same as zero) until WINDOW_UPDATE credit is available.

Test plan

  • tests/_sync/test_http2.py::test_http2_wait_for_outgoing_flow_handles_negative_window
  • tests/_async/test_http2.py::test_http2_wait_for_outgoing_flow_handles_negative_window
  • existing test_http2_connection_with_flow_control

Fixes #1082

h2 can report a negative local flow-control window after DATA has already
been accounted. `_wait_for_outgoing_flow` only blocked on `flow == 0`, so a
negative window was returned as max_flow and `send_data` raised
LocalProtocolError ("Cannot send N bytes, flow control window is -M").

Treat any non-positive flow like zero and wait for WINDOW_UPDATE credit.

Fixes encode#1082

Signed-off-by: Alex Chen <l46983284@gmail.com>
CI mypy reported unused-ignore on method-assign comments in the encode#1082
regression. Use setattr for the temporary monkeypatches instead.

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu

Copy link
Copy Markdown
Author

CI residual

Root cause was PR-caused style/type, not the product wait fix:

  • Job: Test Suite / Python 3.8 → scripts/check → mypy
  • Errors: tests/_async/test_http2.py:423 and sync twin — Unused "type: ignore" comment [unused-ignore]
  • Other matrix jobs cancelled after first fail (fail-fast)

Fix: drop the unused ignores and monkeypatch via setattr in the #1082 regression only. Product while flow <= 0 unchanged.

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.

HTTP/2 large request body can fail with negative flow-control window

1 participant