Skip to content

fix(device-sync): tolerate a device deleted out-of-band mid-update (#84) - #156

Open
simons-plugins wants to merge 2 commits into
mainfrom
fix/84-keyerror-guard
Open

fix(device-sync): tolerate a device deleted out-of-band mid-update (#84)#156
simons-plugins wants to merge 2 commits into
mainfrom
fix/84-keyerror-guard

Conversation

@simons-plugins

Copy link
Copy Markdown
Owner

Closes #84.

What

A device deleted out-of-band while its Matter node keeps reporting raised an uncaught KeyError per event until the next reconcile (non-node-scoped _on_attribute path + apply_states — the write seam PR #80's meter forward-links added a new route to), and the node-scoped path logged the "bad update" warning for what is a routine deletion race.

How

All three sites guard only the dict lookup, with the file's own tolerate/debug/move-on idiom (_safe_unreachable/_clear_error):

  • non-node-scoped path: KeyError → debug, return — never reaches the "bad update" warning
  • node-scoped fan-out: KeyError → debug, continue — siblings on the same endpoint still update
  • apply_states: KeyError → debug, return; updateStatesOnServer failures deliberately NOT caught — today's propagation behaviour is pinned by test

The tight scoping is itself pinned: a handler that raises KeyError for an unrelated reason still gets the "bad update" warning.

Tests

5 new (2270 Python; TS untouched). Mutation-verified both ways: unguarding the non-node-scoped lookup fails its test; widening the node-scoped catch back to the broad try fails the deleted-vs-warning distinction test.

Out of scope, deliberately: the other indigo.devices[...] sites in the file (reconcile/menu contexts, per the issue).

Version 2026.8.10 → 2026.8.13 (.11/.12 ride on open PRs #153/#155 — merge order: #153, #155, then this).

🤖 Generated with Claude Code

https://claude.ai/code/session_018Unpt5UPRdLoZkahH6a4gL

If an Indigo device is deleted while its Matter node keeps reporting,
indigo.devices[dev_id] raised an uncaught KeyError per event until the
next reconcile (non-node-scoped path and apply_states — the seam PR
#80's meter forward-links added a new route to), and the node-scoped
path logged the scary "bad update" WARNING for what is a routine
deletion race.

All three sites now guard the dict lookup ONLY, with the file's own
tolerate/debug/move-on idiom (_safe_unreachable/_clear_error). The
KeyError catch is deliberately scoped tight: a handler that itself
raises KeyError for an unrelated reason still gets the "bad update"
warning — pinned by test. updateStatesOnServer failures keep today's
behaviour (propagate uncaught) — also pinned.

5 new tests (2270 Python). Mutations verified: unguarding site 1 fails
its test; widening site 3's catch back to the broad try fails the
deleted-vs-warning test.

Version 2026.8.10 -> 2026.8.13 (.11/.12 ride on open PRs #153/#155).

Closes #84

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Unpt5UPRdLoZkahH6a4gL
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@simons-plugins, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ea853003-f3c0-4aa0-a851-a7b6b93cf8f2

📥 Commits

Reviewing files that changed from the base of the PR and between 5ceabdf and e7decf5.

📒 Files selected for processing (5)
  • docs/HANDOVER.md
  • indigo-matter.indigoPlugin/Contents/Info.plist
  • indigo-matter.indigoPlugin/Contents/Server Plugin/device_sync.py
  • tests/test_device_sync.py
  • tests/test_generic_switch.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The review found the identical unguarded lookup in _on_node_event —
the path switch presses and lock operations arrive on — where a
deleted device produced a full traceback per event via the plugin's
_on_matter_event exception handler. Same four-line guard, same idiom;
mutation-verified (unguarding fails the new test).

Also from review: two factual slips in the node-scoped guard's comment
("above" for a path that is below; "this endpoint's devices" for a
fan-out that is node-wide outside the multi_power_source branch), and
HANDOVER's backlog line no longer lists issues that have open PRs.

Suites: 2271 Python (+1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Unpt5UPRdLoZkahH6a4gL
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.

_on_attribute/apply_states: unguarded indigo.devices[dev_id] can KeyError if a device is deleted out-of-band

1 participant