fix(webdav): publish circuit state before failure - #56
Conversation
📝 WalkthroughWalkthroughThe HTTP 405 path now drains queued WebDAV requests before failing the current request, ensuring circuit-transition failures are published before caller completion. ChangesWebDAV circuit transition
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
media-source/src/main/kotlin/com/miruplay/tv/mediasource/WebDavRequestCoordinator.kt (1)
207-208: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for the completion ordering.
The implementation matches the intended ordering, but the existing 405 test only submits requests sequentially and does not prove that queued work has failed before the current caller future completes. Add a deterministic concurrent test using latches/barriers to enqueue work during the 405 request, then assert the queued future is already failed when the current future completes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@media-source/src/main/kotlin/com/miruplay/tv/mediasource/WebDavRequestCoordinator.kt` around lines 207 - 208, Add a deterministic concurrent regression test for the 405 handling in WebDavRequestCoordinator, using latches or barriers to enqueue a request while the 405 transition is in progress. Assert that the queued request’s future has failed before the current caller’s future completes, while preserving the existing sequential 405 coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@media-source/src/main/kotlin/com/miruplay/tv/mediasource/WebDavRequestCoordinator.kt`:
- Around line 207-208: Add a deterministic concurrent regression test for the
405 handling in WebDavRequestCoordinator, using latches or barriers to enqueue a
request while the 405 transition is in progress. Assert that the queued
request’s future has failed before the current caller’s future completes, while
preserving the existing sequential 405 coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d5952dd3-4ef0-41f2-9cc8-dd7d6b75dcec
📒 Files selected for processing (1)
media-source/src/main/kotlin/com/miruplay/tv/mediasource/WebDavRequestCoordinator.kt
|
CodeRabbit nitpick reviewed: I am not adding a second internal-completion-order test. WebDavEndpointConsumer exposes only synchronous submit(), so precisely observing queued-work CompletableFuture completion before the current internal future would require a production-only test hook or exposing private queue/work futures. The existing |
Summary
Release context
Master run 30343087558 failed in media-source:testReleaseUnitTest at WebDavRequestCoordinatorTest.kt:150. Lint passed; build-release was skipped. This one-file fix unblocks the pending Android 2.4 release without changing baseAppVersionName.
Validation
The unrelated untracked Windows NUL entry remains excluded.
Summary by CodeRabbit