fix: Schlage sync loop and uncaught ReadTimeout#1089
Merged
Conversation
…adTimeout Two fixes: 1. Schlage async_set_usercode: handle "already exists" error from add_code by falling back to delete-then-add. Schlage's cloud API has eventual consistency — get_codes may not return a code that add_code knows about. This caused an infinite retry loop where the sync manager kept trying to add a code that already existed. 2. BaseLock async_call_service: catch OSError alongside HomeAssistantError. ReadTimeout (from requests/urllib3) extends OSError, not HomeAssistantError. When Schlage's cloud API timed out, the error leaked to the generic exception handler which suspended the lock instead of retrying. Fixes #1083 Entire-Checkpoint: 1028a19f8cac
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1089 +/- ##
==========================================
+ Coverage 95.70% 95.74% +0.03%
==========================================
Files 46 46
Lines 5286 5283 -3
Branches 503 503
==========================================
- Hits 5059 5058 -1
+ Misses 227 225 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Schlage-specific sync retry loops and prevents network timeouts from escaping provider service-call wrappers during sync operations.
Changes:
- Schlage provider: simplify set-usercode flow to delete-then-add when a code already exists, and recover from Schlage “already exists” eventual-consistency errors by deleting and retrying add.
- Base provider: treat raw
OSErrorfailures fromhass.services.async_call()as transient and wrap them asLockDisconnectedfor retry handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| custom_components/lock_code_manager/providers/schlage.py | Updates async_set_usercode to avoid Schlage add-code duplicate-name/consistency failures that previously caused repeated retries. |
| custom_components/lock_code_manager/providers/_base.py | Expands async_call_service exception handling to include OSError so timeouts/network errors are wrapped consistently. |
Comments suppressed due to low confidence (1)
custom_components/lock_code_manager/providers/_base.py:844
- async_call_service now wraps OSError into LockDisconnected, but the current base provider tests only cover HomeAssistantError wrapping and propagation of non-HA exceptions (e.g., TypeError). Please add a unit test that registers a service raising an OSError (e.g., TimeoutError/ConnectionError) and asserts it is converted into LockDisconnected, to lock in the intended behavior and prevent regressions.
except (HomeAssistantError, OSError) as err:
# HomeAssistantError covers ServiceValidationError and HA-wrapped
# failures. OSError covers transient network errors (ReadTimeout,
# ConnectionError) from integrations that don't wrap them in
# HomeAssistantError. CancelledError and programming bugs
# (TypeError, KeyError) deliberately propagate.
LOGGER.error(
"Error calling %s.%s service call: %s", domain, service, str(err)
)
raise LockDisconnected(
f"Service call {domain}.{service} failed: {err}"
) from err
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the two-codepath approach (try add → catch duplicate → delete → retry) with unconditional delete-before-add. This handles Schlage cloud API eventual consistency more cleanly by pre-emptively deleting both the existing tagged name and the new tagged name before adding. Uses a set to deduplicate when the name is unchanged (same code path for PIN-only updates and name+PIN updates). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e5ef38dbac33
…all_service Verifies that transient network errors (TimeoutError, ReadTimeout) from integrations that don't wrap them in HomeAssistantError are correctly routed through the retry/backoff path as LockDisconnected. Addresses Copilot review comment on PR #1089. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e3a77ed6db14
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
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.
Proposed change
Fixes two bugs causing Schlage locks to repeatedly re-set codes every sync cycle:
1. Schlage set_usercode sync loop from eventual consistency
Schlage's cloud API has eventual consistency —
get_codesmay not return a code thatadd_codeknows about. This causedasync_set_usercodeto attemptadd_codefor a code that already existed, getting "already exists" error →LockOperationFailed→ retry → same error → infinite loop.Fix: Simplified
async_set_usercodeto always delete-then-add. Before adding a code, both the existing tagged name and the new tagged name are speculatively deleted (deduplicated via set when names match).LockDisconnectedon delete is swallowed since the code may not exist. This eliminates the "already exists" error entirely rather than handling it reactively.2. Catch OSError in BaseLock async_call_service
ReadTimeoutfromrequests/urllib3extendsOSError, notHomeAssistantError. When Schlage's cloud API timed out during a set operation, the error leaked pastasync_call_serviceto the sync manager's genericexcept Exceptionhandler, which suspended the lock instead of retrying.Fix:
async_call_servicenow catches(HomeAssistantError, OSError)— covering both HA-wrapped errors and raw network errors from integrations that don't wrap them. Added test coverage for this behavior.Type of change
Additional information