fix(vehicle): single-flight config writes to stop the batch replay storm - #11
Conversation
update_config() logged and returned on an error-shaped response without clearing the merged pending config, so every already-scheduled listener task (HA's async_setup_stream fans out one add_field task per streaming entity) independently replayed the identical full PATCH - 80 requested fields observed replaying serially against a deterministic upstream rejection. TeslemetryStreamVehicle now funnels every caller through one per-vehicle flush: the first caller starts it, later callers merge into the same pending config and await that flush instead of starting their own. A body-shaped error is terminal for the batch - it is not replayed, but the desired config is kept for the next explicit update_config call (a new listener, reconnect, or manual retry). A transport-level failure gets one bounded retry inside the same flush before it is treated as failed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef5a76a19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| flight = asyncio.ensure_future(self._flush()) | ||
| self._flight = flight | ||
|
|
||
| await flight |
There was a problem hiding this comment.
Shield the shared flush from waiter cancellation
If any concurrent update_config caller is cancelled or wrapped in a timeout during the debounce or PATCH, cancellation propagates through this direct await and cancels the shared task. Every other caller awaiting the same flight then receives CancelledError, and the entire coalesced configuration can remain unapplied; await asyncio.shield(flight) so cancelling one waiter does not abort the per-vehicle single-flight operation.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
A waiter joined the shared per-vehicle flush with a plain await, so cancelling (or timing out) one caller threw CancelledError into the flush itself, aborting it for every other concurrent waiter and leaving the coalesced config unapplied. Join with asyncio.shield instead: the cancelled caller still observes CancelledError, but the flush it was waiting on keeps running to completion for the remaining waiters.
Intent
TeslemetryStreamVehicle.update_config()logged and returned on an error-shaped response ({"response": null, "error": ...}) without clearing the merged pending config. Every already-scheduled listener task then independently replayed the same full PATCH:async_setup_stream()schedules oneadd_field()task per streaming listener. Each call merges into the shared pending dict, waits out the 1s debounce, and then PATCHes.TeslemetryStreamVehiclenow funnels every caller through one per-vehicle flush (_flush):aiohttp.ClientError/timeout) gets one bounded retry inside the same flush before the batch is treated as failed, since that's not the server's deterministic rejection shape.tests/test_batch_retry_storm.pyreproduces the storm with 80 concurrent listener tasks against a failing mock and asserts exactly one PATCH per failed batch (confirmed this fails againstmain, sending 80 PATCHes, before this fix).tests/test_config_update.pycontinues to cover the response-shape handling this builds on.