v0.18.2-0043: report partial commit failures and bound 429 retries (#1009) - #1021
Merged
Merged
Conversation
…Dispatcharr 429 backoff POST /api/channel-pipeline/run/commit returned 502 with an empty completed_writes list whenever the first replayed write failed, which a caller could not tell apart from a gateway failure. The trigger was Dispatcharr rate limiting: no code path in the client handled 429 and every write called raise_for_status, so one 429 aborted the fail-fast replay while the collect-and-continue /run path shrugged it off. - dispatcharr_client: retry 429 in _request and _login with bounded exponential backoff (1s, 2s, 4s; cap 10s), honouring Retry-After; a spent budget surfaces as an HTTPStatusError carrying the 429. - pipeline_write_plan: PartialReplayError carries failed_write, not_applied and pre_mutation. pre_mutation is True only when nothing completed, compensation was clean, and the first write was provably rejected (4xx or connection refused) before upstream mutated. - routers/channel_pipeline: a partial replay returns 424 Failed Dependency with execution_id, failed_index, failed_write, pre_mutation, completed_writes, not_applied and compensation_errors. The commit stays synchronous; the 30s request-timeout exposure raised in #1009 is left for a separate maintainer decision. - tests: replay classification (429 first / 429 after a landed write / timeout), client retry and budget behaviour, router 424 contract. Closes #1009 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ng and 429 backoff (#1009) 1. Outcome classes are disjoint. The failed write carries failed_outcome "rejected" (4xx or refused connection: upstream provably did not mutate) or "unknown" (timeout, dropped connection, 5xx: it may have landed). not_applied lists only the writes AFTER the failed one, which were never attempted; a landed PATCH with a lost response is no longer reported as not applied. completed_writes is documented as forward-call history, not current upstream state. Exposed on the 424 body, the execution log entry and the snapshot evidence. 2. Descriptors name the resolved upstream id: an update recorded against temp id -1 that PATCHed channel 101 reports update_channel:101; a completed create reports the resource it produced (create_channel#0->101) so a failed compensation can be located; an unresolved future target renders as pending(-2) instead of failing rendering. 3. Descriptors never stringify argument dictionaries or URLs. A create_logo payload with a credentialed URL is identified by plan position (create_logo#0) in failed_write, not_applied, completed and str(exc). 4. Retry-After is validated (finite, non-negative) and every call's 429 waits are bounded by an explicit 30s total budget shared by login and API requests. A server-directed delay beyond the budget raises the 429 HTTPStatusError immediately instead of sleeping (an infinite login sleep previously held _auth_lock); the lock is released on that path. 5. Retry-After in HTTP-date form is parsed to its remaining non-negative interval under the same budget; garbage still falls back to backoff. Tests: lost-response vs confirmed-rejection, resolved ids after a dependent update failure, compensation-failure naming, pending targets, credential canary absent from every diagnostic field, mapping error before the call, Retry-After parser edge cases, over-budget seconds and dates, cumulative budget, invalid values, auth-lock release. Router test updated for failed_outcome and the narrowed not_applied. Docs and changelog aligned. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGuX7qVpTp99bzYZXgLJMm
…#1010 review (#1009) Strict-delta review on PR #1010 accepted items 4 and 5 and asked for retained regressions proving items 1-3 across the HTTP, persistence and MCP boundaries rather than at PartialReplayError. - New backend/tests/integration/test_gh1009_commit_partial_failure_evidence.py drives the real DispatcharrClient over a controlled httpx.MockTransport that plays Dispatcharr, the real replay_write_plan and compensation pass, the real POST /api/auto-creation/run/commit handler over ASGI, and the real _mark_execution_failed persistence. Item 1: a PATCH that lands upstream and then loses its response yields failed_outcome=unknown, the failed write absent from not_applied and the later delete never attempted, in the 424 body, the execution log entry and the snapshot recovery evidence, with a 429 confirmed-rejection control (rejected, pre_mutation=true) through the same boundary. Item 2: create -> dependent update rejected reports completed create_channel#0->101 and failed_write update_channel:101 with no temp id anywhere in the response or persisted evidence, and compensation DELETEs the real id; a compensation-failure variant (DELETE 503) keeps the orphan locatable in both the 424 and the persisted evidence. Item 3: a create_logo payload carrying a canary token in its URL, and a create_channel carrying it in tvg_id, leave no trace in the 424 body, the persisted partial-failure fields, the execution error message or the backend DEBUG log, while execution_id and the plan-position descriptors survive. - New mcp-server/tests/test_gh1009_partial_failure_presentation.py runs the sidecar's real ECMClient.post and the run_channel_pipeline tool over a mock transport answering the backend's 424 shape, asserting the operator text and the sidecar log carry execution_id, failed_write, failed_outcome and not_applied, plus a control showing the sidecar is a verbatim pass-through, which is why the payload-free guarantee is asserted at the producer. - Rebased onto origin/dev (#1006, #1008, #1017 landed; dev is 0036) and bumped the three version touchpoints and the changelog to 0.18.2-0039. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…redentials from the log redactor (#1009) Review round 3 on PR #1010, two current-head blockers: - CodeQL alert 2034 (py/partial-ssrf) fired on the 401 re-issue inside the new 429 loop, which duplicated the outbound request call. _request now has exactly one self._client.request call site: the 401 refresh sets a flag and loops back to it (at most one refresh per rate-limit attempt, the historical contract), and the 429 backoff increments the attempt counter and loops. Behaviour is unchanged for api-key mode (401 terminal), retry_on_401=False, and the retry budget; the auth, rate-limit, version-advisory and settings suites pin all of them. - DispatcharrClient.__init__ registers its credentials with the process-global log redactor, so the tests' one-character keys rewrote ordinary log text in a later test (seq***REDACTED***ence-1) and failed the persistent-log rotation test on CI. The new tests now use long, unique synthetic credentials, and tests/conftest.py gains an autouse fixture that snapshots the registered sensitive-value forms before each test and restores them afterwards, so no test can leak a registration into another. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| callers can tell a rate-limit rejection from every other failure. | ||
| """ | ||
| import pytest | ||
| from unittest.mock import AsyncMock, MagicMock, patch |
|
|
||
| import httpx | ||
|
|
||
| import dispatcharr_client |
| # --------------------------------------------------------------------------- | ||
|
|
||
| from email.utils import format_datetime | ||
| from datetime import datetime, timedelta, timezone |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Canonical integration replacement for #1010. Preserves the accepted #1009 remediation while merging current
devthrough build0.18.2-0042, resolving the replay conflict with #1014, and assigning build0.18.2-0043.Retry-Afterwaits across login and API requestsOriginal contributor PR: #1010
Accepted remediation head:
af6f6904e6fae70ea7430860cf61ecda7877fc5eIntegrated dev head:
ebae7a48a36338f4a0d44fec1340e86897947692Integration head:
88e5b156d5b73ed2ab99e591a4ea415aa0d23965Verification
ECM_GATE_COMPLETE ecm-gate-xwtik607git diff --check: passedCloses #1009.