fix: retry upload after package registration instead of raising

[dimbleby]

After _register() succeeded, resp.raise_for_status() raised on the original 400 response, so the upload was never retried. Now retries the upload once after registration, with a guard against infinite recursion if the server keeps returning 400.

Another from copilot. Borderline bug or enhancement, but the retry surely makes sense.

Summary by Sourcery

Ensure package uploads are retried once after successful registration when the server initially responds with a 400 indicating the package was never registered.

Bug Fixes:

• Prevent failed uploads when registration succeeds but the initial 400 response would previously halt the process instead of retrying the upload.

Tests:

• Add coverage to verify that an upload is retried after registration and that multiple files are uploaded in the correct sequence.

[sourcery-ai]

Reviewer's Guide

Adds a guarded retry of file upload after successful package registration when encountering specific 400 responses, and introduces a test to verify the upload-register-upload flow.

Sequence diagram for guarded retry upload after package registration

sequenceDiagram
    actor User
    participant Uploader
    participant Server

    User->>Uploader: publish_package(file)
    Uploader->>Server: POST upload_file(file, registered=false)
    Server-->>Uploader: 400 Bad Request
    alt status 400 and contains was ever registered and registered is false
        Uploader->>Server: POST register_package()
        Server-->>Uploader: 200 OK
        Uploader->>Server: POST upload_file(file, registered=true)
        alt second upload succeeds
            Server-->>Uploader: 200 OK
            Uploader-->>User: publish succeeded
        else second upload still 400
            Server-->>Uploader: 400 Bad Request
            Uploader->>Uploader: resp.raise_for_status()
            Uploader-->>User: error raised
        end
    else status 400 and registered is true
        Uploader->>Uploader: resp.raise_for_status()
        Uploader-->>User: error raised
    else other status handling
        Uploader->>Uploader: handle other responses
    end

Loading

Class diagram for updated uploader retry logic

classDiagram
    class Uploader {
        +upload_file(file)
        -_upload_file(session, url, file, dry_run, skip_existing, registered)
        -_register(session, url)
        -_is_file_exists_error(resp) bool
    }

    class Session {
    }

    class Response {
        +status_code int
        +text str
        +raise_for_status()
    }

    Uploader --> Session : uses
    Uploader --> Response : receives from server

Loading

File-Level Changes

Change	Details	Files
Add a retry path so that, after a package is registered in response to a specific 400 error, the file upload is attempted again instead of failing.	Extend the _upload_file helper signature with a keyword-only 'registered' flag to track whether registration has already occurred in the current call chain. On receiving a 400 response containing 'was ever registered', conditionally call _register only if registration has not yet been performed. After successful registration, recursively re-invoke _upload_file with registered=True so the upload is retried once, and guard against infinite recursion by raising for subsequent 400s with the same condition. Preserve existing behavior for other HTTP status codes and for skip_existing handling.	src/poetry/publishing/uploader.py
Add a regression test to verify that the uploader retries the upload after registration and that the request ordering is upload → register → upload → upload.	Define test_uploader_retries_upload_after_register to simulate a 400 'was ever registered' response followed by successful registration and uploads using the responses mock. Assert that four HTTP calls are made and validate the :action field of each multipart body to confirm the correct sequence of operations.	tests/publishing/test_uploader.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The retry logic in _upload_file uses recursion with a boolean flag; consider replacing this with an explicit, non-recursive retry loop (e.g., a small retry counter) to make the control flow clearer and easier to extend if additional retry cases are added.
• The registered flag name and behavior could be clarified (e.g., has_retried_after_register or similar) to make it immediately obvious that it is guarding a single retry attempt rather than simply reflecting registration state.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The retry logic in `_upload_file` uses recursion with a boolean flag; consider replacing this with an explicit, non-recursive retry loop (e.g., a small retry counter) to make the control flow clearer and easier to extend if additional retry cases are added.
- The `registered` flag name and behavior could be clarified (e.g., `has_retried_after_register` or similar) to make it immediately obvious that it is guarding a single retry attempt rather than simply reflecting registration state.

## Individual Comments

### Comment 1
<location path="src/poetry/publishing/uploader.py" line_range="260-264" />
<code_context>
-                    self._register(session, url)
+                    if not registered:
+                        self._register(session, url)
+                        return self._upload_file(
+                            session, url, file, dry_run, skip_existing,
+                            registered=True,
+                        )
</code_context>
<issue_to_address>
**suggestion:** Forwarding all parameters positionally in the recursive call is somewhat brittle.

The retry call currently passes `dry_run` and `skip_existing` positionally. If `_upload_file`’s signature changes (new params or reordered ones), this can silently break behavior. Prefer keyword arguments for these flags and `registered` (e.g. `dry_run=dry_run`, `skip_existing=skip_existing`, `registered=True`) to make the intent explicit and avoid argument ordering issues.

```suggestion
                        self._register(session, url)
                        return self._upload_file(
                            session=session,
                            url=url,
                            file=file,
                            dry_run=dry_run,
                            skip_existing=skip_existing,
                            registered=True,
                        )
```
</issue_to_address>

### Comment 2
<location path="tests/publishing/test_uploader.py" line_range="233-234" />
<code_context>
+) -> None:
+    """After registering a package, the upload must be retried.
+
+    The server returns 400 "was ever registered" on the first upload,
+    then 200 on the registration and the retried upload.
+    """
+    http.post("https://foo.com", status=400, body="No package was ever registered")
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for repeated 400s after registration to exercise the recursion guard

The current test only covers the case where a 400 leads to registration and then a successful retry. Please add a second test that mocks responses as: 400 (first upload), 200 (register), 400 (retried upload). That test should verify that `upload()` terminates (no further retries/registration attempts) and surfaces the second 400 (e.g., via an exception) instead of attempting to register again.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[dosubot]

Related Documentation

1 document(s) may need updating based on files changed in this PR:

Python Poetry

CHANGELOG /poetry/blob/main/CHANGELOG.md — ⏳ Awaiting Merge

Note: You must be authenticated to accept/decline updates.

^{How did I do? Any feedback?}