Skip to content

Cancelling uploadMedia task should cancel underlying HTTP request#794

Merged
oguzkocer merged 5 commits intotrunkfrom
cancel-upload-media
Jul 7, 2025
Merged

Cancelling uploadMedia task should cancel underlying HTTP request#794
oguzkocer merged 5 commits intotrunkfrom
cancel-upload-media

Conversation

@crazytonyli
Copy link
Contributor

There are two main changes in this PR:

  1. Add RequestExecutionErrorReason::CancellationError. Currently, cancelling an HTTP API call results in RequestExecutionErrorReason::GenericError { message: "cancelled" }, which is not specific enough. Adding a CancellationError would help developers to distinguish errors due to the user cancelling operations from other unexpected errors.
  2. I initially thought the try await uploadTask.value call would handle cancellation, but turns out it does not. I have addressed this issue in this PR and added a couple of unit tests to cover it.

@crazytonyli crazytonyli requested a review from oguzkocer July 4, 2025 10:49
Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

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

I think we'll have to update the Kotlin wrapper to handle the new error type. I am surprised it didn't cause a failure. However, we can merge this as is and I can take a look at that separately.

@oguzkocer oguzkocer merged commit 0b14d47 into trunk Jul 7, 2025
20 checks passed
@oguzkocer oguzkocer deleted the cancel-upload-media branch July 7, 2025 15:37
@oguzkocer
Copy link
Contributor

I think we'll have to update the Kotlin wrapper to handle the new error type.

We are not mapping RequestExecutionErrorReason types in Kotlin, so this was incorrect.

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