Skip to content

fix(mint): avoid failing large HTTP/2 uploads#842

Draft
yordis wants to merge 8 commits into
masterfrom
yordis/fix-http2-upload-flow-control
Draft

fix(mint): avoid failing large HTTP/2 uploads#842
yordis wants to merge 8 commits into
masterfrom
yordis/fix-http2-upload-flow-control

Conversation

@yordis

@yordis yordis commented Apr 18, 2026

Copy link
Copy Markdown
Member
  • Large HTTP/2 uploads currently fail once the request body exceeds Mint's flow-control window.
  • Streamed HTTP/2 uploads can crash instead of recovering when window updates arrive mid-request.
  • Tesla should handle larger HTTP/2 request bodies without forcing callers to fall back to HTTP/1.

Copilot AI review requested due to automatic review settings April 18, 2026 18:50
@cursor

cursor Bot commented Apr 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core Mint adapter request/response coupling for HTTP/2 only, but behavior for HTTP/1 and small bodies stays on existing paths and is covered by new integration tests.

Overview
Fixes HTTP/2 POST/PUT uploads in Tesla.Adapter.Mint that failed when the request body exceeded Mint’s flow-control window or when the server answered before the upload finished.

Non-empty HTTP/2 bodies with known size are sent via the streaming request path, with a default content-length when callers omit it. Upload chunks are capped (16 KiB) and {:exceeds_window_size, …} is handled by waiting for window updates (and by retrying with the advertised window size) instead of erroring or crashing.

While uploading, the adapter merges inbound Mint response events into an accumulator passed into format_response, so plain, chunks, and stream response modes can return status/body when the server responds early during a large upload.

Tests cover large and streamed HTTP/2 bodies, automatic content-length, and a Cowboy handler that replies immediately during upload.

Reviewed by Cursor Bugbot for commit ee6d187. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Mint adapter to support large HTTP/2 request bodies by streaming uploads and respecting HTTP/2 flow-control windows, preventing failures/crashes on large or streamed uploads.

Changes:

  • Add HTTP/2-aware request-body streaming that waits for flow-control window updates and splits chunks accordingly.
  • Thread an accumulated “early response” accumulator through the request/response pipeline so window updates (and any early response frames) can be processed mid-upload.
  • Add regression tests for large HTTP/2 uploads (binary body and streamed body).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/tesla/adapter/mint.ex Streams HTTP/2 request bodies (including previously non-streaming iodata) and waits for flow-control window before writing chunks.
test/tesla/adapter/mint_test.exs Adds tests covering large HTTP/2 uploads and helpers to generate/verify posted payloads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/tesla/adapter/mint.ex Outdated
Comment thread lib/tesla/adapter/mint.ex Outdated
Comment thread test/tesla/adapter/mint_test.exs
Comment thread lib/tesla/adapter/mint.ex
@yordis yordis requested a review from Copilot April 18, 2026 21:05
@yordis yordis marked this pull request as draft April 18, 2026 21:06
Comment thread lib/tesla/adapter/mint.ex

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

lib/tesla/adapter/mint.ex:322

  • receive_headers_and_status/4 always calls receive_packet/4 before checking whether acc already contains :status and :headers. Since acc can now be pre-populated from request-body streaming (e.g., while waiting for HTTP/2 window updates), this can cause an unnecessary extra receive and potentially block/timeout even though the headers/status are already available. Add a fast-path that returns immediately when acc already has both keys.
    defp receive_headers_and_status(conn, ref, opts, acc) do
      with {:ok, conn, acc} <- receive_packet(conn, ref, opts, acc) do
        case acc do
          %{status: _status, headers: _headers} -> {:ok, conn, acc}
          # if we don't have status or headers we try to get them in next packet
          _ -> receive_headers_and_status(conn, ref, opts, acc)
        end

lib/tesla/adapter/mint.ex:300

  • receive_responses/4 now accepts an initial acc, but it still unconditionally calls receive_packet/4 first. If acc already contains :done (or all response data) from earlier receives during HTTP/2 upload flow-control handling, this will wait for an extra packet and may block/timeout. Consider checking acc[:done] (and running check_data_size/3) before attempting another receive_packet/4 call.
    defp receive_responses(conn, ref, opts, acc) do
      with {:ok, conn, acc} <- receive_packet(conn, ref, opts, acc),
           :ok <- check_data_size(acc, conn, opts) do
        if acc[:done] do
          if opts[:close_conn], do: {:ok, _conn} = close(conn)
          {:ok, acc}
        else
          receive_responses(conn, ref, opts, acc)
        end

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/tesla/adapter/mint.ex Outdated
@yordis yordis force-pushed the yordis/fix-http2-upload-flow-control branch from 55a397e to 0b0486c Compare April 22, 2026 21:42

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ff091ff. Configure here.

Comment thread lib/tesla/adapter/mint.ex
yordis added 8 commits June 4, 2026 12:36
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis force-pushed the yordis/fix-http2-upload-flow-control branch from e71334b to ee6d187 Compare June 4, 2026 16: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.

2 participants