File upload retried after a transient error is sent with an empty multipart body #3459

Description of the problem, including code/CLI snippet

Location (python-gitlab/python-gitlab @ default branch 8daa940):

• gitlab/client.py:714-716
• gitlab/client.py:727-739
• gitlab/_backends/requests_backend.py:118-119

# gitlab/client.py
        # We need to deal with json vs. data when uploading files
        send_data = self._backend.prepare_send_data(files, post_data, raw)
        opts["headers"]["Content-type"] = send_data.content_type
        ...
        while True:
            try:
                result = self._backend.http_request(
                    method=verb,
                    url=url,
                    json=send_data.json,
                    data=send_data.data,
                    params=params,
                    timeout=timeout,
                    verify=verify,
                    stream=streamed,
                    **opts,
                )

# gitlab/_backends/requests_backend.py
            data = MultipartEncoder(fields=post_data)
            return SendData(data=data, content_type=data.content_type)

Problem

With retry_transient_errors=True, a request that uploads files (http_post(..., files=...), e.g. project.upload(...), avatar uploads) is retried with an empty body after a retryable response such as HTTP 500. prepare_send_data() runs once, before the retry loop, and every iteration re-sends the same send_data.data. For multipart uploads that object is a single streaming MultipartEncoder: the first attempt consumes it, so the retry reads b''.

I have not observed what a real GitLab server does with the empty retry; depending on server/proxy behavior it may reject the request or fail on a length mismatch.

Reproduction

Minimal reproduction (uses responses to return one 500 followed by a 200):

import json
from unittest import mock

import responses
import gitlab


@responses.activate
def test_upload_retry_keeps_body():
    gl = gitlab.Gitlab("http://localhost", private_token="t", api_version="4",
                       retry_transient_errors=True)
    bodies = []

    def cb(request):
        body = request.body
        bodies.append(body.read() if hasattr(body, "read") else (body or b""))
        if len(bodies) == 1:
            return (500, {}, "Internal Server Error")
        return (200, {"Content-Type": "application/json"},
                json.dumps({"url": "/uploads/abc/test.txt"}))

    responses.add_callback(responses.POST,
                           "http://localhost/api/v4/projects/1/uploads", callback=cb)
    with mock.patch("time.sleep"):
        gl.http_post("/projects/1/uploads",
                     files={"file": ("test.txt", b"file content")})

    assert len(bodies) == 2
    assert b"file content" in bodies[0]
    assert b"file content" in bodies[1]   # fails: retry body is b''

python -m pytest -q test_upload_retry.py

Failing output (from an equivalent unit test in tests/unit/ using the gl_retry fixture):

tests/unit/test_upload_retry.py:63: in test_http_post_upload_retry_sends_valid_body
    assert len(request_bodies[1]) > 0, "Retry request body is empty"
E   AssertionError: Retry request body is empty
E   assert 0 > 0
E    +  where 0 = len(b'')

Environment:

• Commit: observed on 51eb17e6e67a; the same code path is unchanged on current main 8daa94041261 (version 8.6.0)
• Toolchain: Python 3.14.10

Expected Behavior

When the library decides to retry, the retried request carries the same multipart content (and a matching Content-Type boundary / length) as the first attempt, at least for replayable inputs such as bytes or seekable files. For non-replayable inputs (e.g. a one-shot stream), failing clearly instead of sending an empty body would also be acceptable.

This report does not ask to change which responses are retried or to make any new idempotency promise; only that a retry that already happens sends the original payload.

Actual Behavior

The second request body is empty (see the output above).

Already fixed in xanzy/go-gitlab

PR: xanzy/go-gitlab#1115 — "Make sure we set the content correctly" (merged 2021-03-29T10:02:24Z, merge commit 8d741103c90). It stops assigning the upload buffer directly to req.Body and hands it to the retry-aware request with req.SetBody(b) (bumping go-retryablehttp to v0.6.8 for this); the PR's TestUploadFile_Retry covers a 500 followed by a successful retry of the upload. go-gitlab is a separate client for the same GitLab REST API, so this is precedent, not an obligation.

How go-gitlab fixed it:

--- a/projects.go
+++ b/projects.go
@@ -1218,15 +1217,18 @@ func (s *ProjectsService) UploadFile(pid interface{}, file string, options ...Re
...
 	w.Close()
 
-	req, err := s.client.NewRequest("", u, nil, options)
+	req, err := s.client.NewRequest(http.MethodPost, u, nil, options)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	req.Body = ioutil.NopCloser(b)
-	req.ContentLength = int64(b.Len())
+	// Set the buffer as the request body.
+	if err = req.SetBody(b); err != nil {
+		return nil, nil, err
+	}
+
+	// Overwrite the default content type.
 	req.Header.Set("Content-Type", w.FormDataContentType())
-	req.Method = http.MethodPost
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/xanzy/go-gitlab
 require (
 	github.com/google/go-querystring v1.0.0
 	github.com/hashicorp/go-cleanhttp v0.5.1
-	github.com/hashicorp/go-retryablehttp v0.6.4
+	github.com/hashicorp/go-retryablehttp v0.6.8
 	github.com/stretchr/testify v1.4.0
 	golang.org/x/net v0.0.0-20181108082009-03003ca0c849 // indirect
 	golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288

Suggested fix

Build the request body inside the retry loop, so every attempt gets a fresh MultipartEncoder (with its matching Content-type boundary) instead of re-sending the stream the previous attempt consumed; this is the analogous change to making the go-gitlab upload body replayable:

diff --git a/gitlab/client.py b/gitlab/client.py
--- a/gitlab/client.py
+++ b/gitlab/client.py
@@ -711,13 +711,6 @@ class Gitlab:
         if retry_transient_errors is None:
             retry_transient_errors = self.retry_transient_errors
 
-        # We need to deal with json vs. data when uploading files
-        send_data = self._backend.prepare_send_data(files, post_data, raw)
-        opts["headers"]["Content-type"] = send_data.content_type
-
-        if extra_headers is not None:
-            opts["headers"].update(extra_headers)
-
         retry = utils.Retry(
             max_retries=max_retries,
             obey_rate_limit=obey_rate_limit,
@@ -725,6 +718,15 @@ class Gitlab:
         )
 
         while True:
+            # We need to deal with json vs. data when uploading files.
+            # Prepare the body for every attempt: a multipart body is a stream
+            # that a previous attempt has already consumed.
+            send_data = self._backend.prepare_send_data(files, post_data, raw)
+            opts["headers"]["Content-type"] = send_data.content_type
+
+            if extra_headers is not None:
+                opts["headers"].update(extra_headers)
+
             try:
                 result = self._backend.http_request(
                     method=verb,

Verified on 51eb17e (Python 3.14.10, pytest 9.1.1): the test above fails without this patch and passes with it; no other test results change.

I can submit a PR for this.

Checked: #2421 (moving MultipartEncoder into the backend) and #1565 (global retry_transient_errors) are related refactors/features, not this bug; searches for retry upload, MultipartEncoder, retry_transient_errors files found no report of this.

Specifications

• python-gitlab version: observed on commit 51eb17e6e67a; the same code path is unchanged on current main 8daa94041261 (version 8.6.0)
• Gitlab server version (or gitlab.com): n/a (reproduced with mocked HTTP responses)
• Python 3.14.10