[BUGFIX] Fix incorrect cancellation propagation#10
Conversation
82bc0d2 to
d540b8d
Compare
Fix issue where CancellationException was being incorrectly propagated
d540b8d to
694d51b
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the cancellation handling in the RequestDeduplication Ktor client plugin. The previous implementation used a SupervisorJob and CoroutineScope with scope.launch to isolate the first caller's HTTP request from cancellation propagation. The new implementation removes the separate scope entirely, running the request inline in the first caller's coroutine, and introduces a retry mechanism for non-first callers that catch a CancellationException from the shared deferred when their own coroutine is still active.
Changes:
- Removed
SupervisorJob,CoroutineScope,Job, andlaunchin favor of running the HTTP request inline in the first caller's coroutine, with a newexecuteextension function onSend.Sender. - Added
CancellationExceptionhandling for non-first callers: if the shared deferred fails with cancellation but the waiter's coroutine is still active, it retries with a fresh HTTP call. - Simplified
InFlightEntryby removing thejobfield and changing fromdata classtoclass(since identity comparison===is used).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../commonMain/kotlin/io/github/tiper/ktor/client/plugins/deduplication/RequestDeduplication.kt
Outdated
Show resolved
Hide resolved
.../commonMain/kotlin/io/github/tiper/ktor/client/plugins/deduplication/RequestDeduplication.kt
Outdated
Show resolved
Hide resolved
…ters to retry as a deduplicated group
abb4853 to
7824263
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...monTest/kotlin/io/github/tiper/ktor/client/plugins/deduplication/RequestDeduplicationTest.kt
Outdated
Show resolved
Hide resolved
37a7618 to
e026f37
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../commonMain/kotlin/io/github/tiper/ktor/client/plugins/deduplication/RequestDeduplication.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../commonMain/kotlin/io/github/tiper/ktor/client/plugins/deduplication/RequestDeduplication.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.