Skip to content

Conversation

@carlsverre
Copy link
Contributor

@carlsverre carlsverre commented Dec 17, 2025

Which issue does this PR close?

Closes #7044.

Rationale for this change

Initially, the rationale was that the CompleteWriter emits an erroneous warning when an error occurs before it's able to set self.inner = None.

However, upon inspection, it seems like CompleteWriter should enforce a more precise invariant: once the write completes without error, then close or abort must be called.

What changes are included in this PR?

Updates to CompleteWriter to enforce more precise invariants.

Are there any user-facing changes?

AI Usage Statement

No AI used other than in-IDE code completion.

@carlsverre
Copy link
Contributor Author

@Xuanwo throwing this up for discussion. It currently fails the test_retry_write_fail_on_close here:

        match w.close().await {
            Ok(_) => (),
            Err(_) => {
                w.abort().await.unwrap();
            }
        };

But it seems that close/abort is somewhat inconsistent throughout. It seems like most of the code expects that if close fails you don't need to call abort. I'd like to understand what the actual intention of close vs abort is, and when users are expected to call them. Then it may be worthwhile to enforce the correct invariants in the CompleteWriter.

@carlsverre
Copy link
Contributor Author

if write completes without error then close/abort must be called, but it's ok if they fail

@carlsverre carlsverre marked this pull request as ready for review December 23, 2025 17:22
@carlsverre carlsverre requested a review from Xuanwo as a code owner December 23, 2025 17:22
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Enhance the output while writer close with error

1 participant