fix(no-ticket): do not retry a POST after a 500, 502 or 504 response

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - An API request no longer waits without a limit when the API host accepts the connection but does not reply. A request now fails after 30 seconds without data, and the CLI retries it 6 times by default (`--error-retry-max`). File uploads and downloads do not change.
 - A POST request to the API no longer waits without a limit when the API does not reply. A POST or PATCH request now fails after 120 seconds without data. The CLI does not retry a POST or PATCH request after a read timeout or a dropped connection, because the API can have processed it.
+- The CLI no longer sends a POST or PATCH request to the API again after a 500, 502 or 504 response. A gateway can send these codes after the API created the entry, so a retry could create a second entitlement, token or package. The CLI still retries a 429 or 503 response, and it ignores 500, 502 and 504 in `--error-retry-codes` for these requests. A file upload still retries these codes, a read timeout and a dropped connection, because a second upload of the same file is safe.
 
 ## [1.30.0] - 2026-10-22
 

cloudsmith_cli/cli/decorators.py

@@ -781,7 +781,9 @@ def initialise_api(f):
         default="429,500,502,503,504",
         help="The status codes that when received from the API will cause "
         "a retry (if --error-retry-max is > 0). By default this will be for "
-        "429, 500, 502, 503 and 504 error codes.",
+        "429, 500, 502, 503 and 504 error codes. The CLI does not retry a POST "
+        "or PATCH request after a 500, 502 or 504 code, even if you list it, "
+        "because the API can have processed the request.",
     )
     @click.pass_context
     @functools.wraps(f)

cloudsmith_cli/core/api/files.py

@@ -91,7 +91,8 @@ def upload_file(upload_url, upload_fields, filepath, callback=None):
     client = get_files_api()
     headers["user-agent"] = client.api_client.user_agent
 
-    session = create_requests_session()
+    # A second upload of the same file replaces the first one.
+    session = create_requests_session(repeat_non_idempotent=True)
     resp = _send_upload_request(
         session.post, upload_url, data=monitor, headers=headers, proxies=proxies
     )

cloudsmith_cli/core/session.py

@@ -3,6 +3,7 @@
 import ssl
 import sys
 import time
+from typing import Any
 
 import requests
 import urllib3
@@ -29,6 +30,8 @@ def get_default_proxy():
 
 
 NON_IDEMPOTENT_METHODS = frozenset({"POST", "PATCH"})
+# A gateway can send these after the API processed the request.
+PROCESSED_REQUEST_STATUS_CODES = frozenset({500, 502, 504})
 
 
 class ApiKeySafeSession(requests.Session):
@@ -52,14 +55,26 @@ def merge_environment_settings(self, url, proxies, stream, verify, cert):
 class RetryWithCallback(Retry):
     """A urllib3 Retry with a callback on retries."""
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
         self.error_retry_cb = kwargs.pop("error_retry_cb", None)
+        self.repeat_non_idempotent = bool(kwargs.pop("repeat_non_idempotent", False))
         super().__init__(*args, **kwargs)
 
-    def new(self, **kw):
+    def new(self, **kw: Any) -> "RetryWithCallback":
         kw["error_retry_cb"] = self.error_retry_cb
+        kw["repeat_non_idempotent"] = self.repeat_non_idempotent
         return super().new(**kw)
 
+    def is_retry(
+        self, method: str, status_code: int, has_retry_after: bool = False
+    ) -> bool:
+        if (
+            self._is_unsafe_to_repeat(method)
+            and status_code in PROCESSED_REQUEST_STATUS_CODES
+        ):
+            return False
+        return super().is_retry(method, status_code, has_retry_after)
+
     def increment(
         self,
         method=None,
@@ -80,10 +95,14 @@ def _can_repeat_processed_request(
         self, method: str | None, error: Exception | None
     ) -> bool:
         """Return True when a retry can send a request that the server processed."""
+        return self._is_unsafe_to_repeat(method) and self._is_read_error(error)
+
+    def _is_unsafe_to_repeat(self, method: str | None) -> bool:
+        """Return True when a second send of the request can repeat its effect."""
         return (
-            isinstance(method, str)
+            not self.repeat_non_idempotent
+            and isinstance(method, str)
             and method.upper() in NON_IDEMPOTENT_METHODS
-            and self._is_read_error(error)
         )
 
     def sleep_for_retry(self, response=None):
@@ -177,8 +196,14 @@ def create_requests_session(
     respect_retry_after_header=True,
     user_agent=None,
     headers=None,
+    repeat_non_idempotent: bool = False,
 ):
-    """Create a requests session that retries some errors."""
+    """Create a requests session that retries some errors.
+
+    A POST or PATCH request is not retried after a read error or a 500, 502
+    or 504 response, because the server can have processed it. Set
+    repeat_non_idempotent when a second send of the request is safe.
+    """
     # pylint: disable=too-many-branches
     config = _sdk_configuration()
 
@@ -232,6 +257,7 @@ def create_requests_session(
         total=retries,
         error_retry_cb=error_retry_cb,
         respect_retry_after_header=respect_retry_after_header,
+        repeat_non_idempotent=repeat_non_idempotent,
     )
 
     adapter = SSLContextAdapter(

cloudsmith_cli/core/tests/test_files.py

@@ -1,14 +1,18 @@
 """Tests for the files API upload helpers."""
 
+from functools import partial
 from unittest.mock import Mock, patch
 
+import httpretty
+import httpretty.core
 import pytest
 import requests
 from cloudsmith_api.rest import ApiException as SdkApiException
 
 from ..api.exceptions import ApiException
 from ..api.files import multi_part_upload_file, upload_file
 from ..credentials.models import CredentialResult
+from ..session import create_requests_session
 
 
 def upload_and_capture_headers(tmp_path, credential, api_key=None):
@@ -117,3 +121,38 @@ def test_failed_multi_part_completion(self, tmp_path):
             )
 
         assert raised.value.status == 0
+
+
+@httpretty.activate(allow_net_connect=False)
+def test_upload_retries_a_gateway_error(tmp_path, monkeypatch):
+    """A second upload of the same file is safe, so the CLI retries it."""
+    # urllib3 2.x calls shutdown() on a retry, and httpretty does not fake it.
+    monkeypatch.setattr(
+        httpretty.core.fakesock.socket,
+        "shutdown",
+        lambda self, how: None,
+        raising=False,
+    )
+    filepath = tmp_path / "package.raw"
+    filepath.write_bytes(b"payload")
+    upload_url = "http://upload.example.invalid/"
+    requests_seen = []
+
+    def respond(request, uri, response_headers):
+        requests_seen.append(uri)
+        return [504, response_headers, ""]
+
+    httpretty.register_uri(httpretty.POST, upload_url, body=respond)
+    files_api = Mock(**{"api_client.user_agent": "cloudsmith-cli"})
+
+    with (
+        patch(
+            "cloudsmith_cli.core.api.files.create_requests_session",
+            partial(create_requests_session, retries=2, backoff_factor=0),
+        ),
+        patch("cloudsmith_cli.core.api.files.get_files_api", return_value=files_api),
+        pytest.raises(ApiException),
+    ):
+        upload_file(upload_url, {}, str(filepath))
+
+    assert len(requests_seen) == 3

cloudsmith_cli/core/tests/test_rest.py

@@ -154,6 +154,25 @@ def test_retries_a_read_timeout_only_for_idempotent_methods(
 
         assert len(requests_seen) == expected_requests
 
+    @pytest.mark.parametrize(
+        "method, status, expected_requests",
+        [("POST", 504, 1), ("POST", 503, 3), ("GET", 504, 3)],
+    )
+    @httpretty.activate(allow_net_connect=False)
+    def test_retries_a_gateway_error_only_for_idempotent_methods(
+        self, method, status, expected_requests
+    ):
+        """A gateway error can replace the reply to a POST that the API processed."""
+        initialise_api()
+        client = RestClient(retries=2, backoff_factor=0)
+        url = "https://test.site"
+        httpretty.register_uri(method, url, status=status, body="")
+
+        with pytest.raises(ApiException):
+            client.request(method, url)
+
+        assert len(httpretty.latest_requests()) == expected_requests
+
 
 class TestCreateRequestsSession:
     @pytest.fixture(autouse=True)