Repository navigation
Recover registration after outages and stop on hard failures - #19
Merged
Merged
Conversation
A failed registration refresh scheduled a reconnect that called SIPRegistrationUserAgent.Start() on an agent that was still running, which threw "already running" and ended the client's retry chain after one attempt. OnRegistrationTemporaryFailure also never cleared IsRegistered, so the client reported registered for the whole outage. Recovery came only from SIPSorcery's own retry, 300 s after the failure. SipClient now: - clears IsRegistered and raises RegistrationStatusChanged on any failure, with or without auto-reconnect; - stops the current registration agent (no unregister) and starts a fresh one on every start, so "already running" cannot happen and late events from an old agent are ignored; StartRegistration() is safe to call again; - keeps the default retry schedule (5 attempts at 2 s x n, then the agent's own retry stays armed), and adds optional extended retry (RegistrationRetryOptions.Extended: 30 s doubling to 5 min, never stops); - classifies hard failures by SIP status (401/407 after authentication, 402, 403, 404) and stops the agent, so rejected credentials are not sent again by SIPSorcery's repeating timer; the health check does not restart registration after a hard failure or while an attempt is in flight; - exposes RegistrationState and LastRegistrationError; - treats a loopback registrar given as host:port as loopback (no STUN). sipbot serve gains --extended-retry / SIP_EXTENDED_RETRY and --sip-trace / SIP_TRACE (both off, read by sipbot only) and reports both on its startup line. JSONL event and field names are unchanged; status.registered is now false while registration is failing. Tests: a scripted loopback registrar covers hard failures (first and authenticated REGISTER, both retry modes), re-registering after a hard failure, SIPSorcery's repeating-timer case, the default schedule, extended backoff and its reset, recovery with and without auto-reconnect, the health check, and Dispose with a reconnect pending. Closes #13 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found in a live outage test against the lab PBX with --extended-retry: with REGISTERs being dropped, the health check fired 3 s into a reconnect attempt and scheduled the next one. In extended mode that stopped the in-flight agent, so its own failure was never reported and the backoff skipped a step; in default mode the scheduled attempt cut the in-flight one short. The health check now does nothing while an attempt is in flight (RegistrationState.Registering), in both modes. New test: every REGISTER dropped, 2 s per attempt, health check every 100 ms; without the fix attempts were cut short after 176-600 ms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ck test On the CI runner a REGISTER can leave late (SIPSorcery sends it via a blocking call on a thread-pool thread) while its attempt timeout is already running, so gaps between REGISTERs came out short even with the fix. Assert the bug's own signature instead: every reconnect attempt must follow a reported temporary failure, taken from the client's metrics. Without the fix both cases still fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
The first CI run failed only in Commit 02d9d4d now checks the bug's own signature instead: every reconnect attempt must follow a reported temporary failure, counted from 🤖 Generated with Claude Code |
calebtt
added this pull request to stack #21
September 26, 2026 01:23
This was referenced Sep 26, 2026
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.
Summary
Closes #13.
A failed registration refresh scheduled a reconnect that called
SIPRegistrationUserAgent.Start()on an agent that was still running. That threw "already running" and ended the client's retry chain after one attempt.OnRegistrationTemporaryFailurealso never clearedIsRegistered, sostatussaid registered for the whole outage. Recovery came only from SIPSorcery's own retry, 300 s after the failure. The issue has the reproduction timeline.Changes
SipClientIsRegisteredand raisesRegistrationStatusChanged, with or without auto-reconnect.Stop(sendZeroExpiryRegister: false)), detaches it, and starts a fresh one. "Already running" can't happen, and late events from an old agent are ignored.StartRegistration()is safe to call again at any time, including after a hard failure.SipConfig.RegistrationRetry.Extended, for always-on hosts. The client owns the retries: the delay doubles from 30 s to at most 5 min, and retries never stop.StartRegistration()is called. Without this stop, a rejected password on the first REGISTER keeps being re-sent byStart()'s repeating timer every (Expires − 5) s.RegistrationState(NotStarted,Registering,Registered,TemporaryFailure,HardFailure) andLastRegistrationError.host:portis treated as loopback, so no STUN runs.sipbot serve--extended-retry/SIP_EXTENDED_RETRY=1turns extended retry on.--sip-trace/SIP_TRACE=1logs every SIP message sent and received to stderr. It is for lab use only, because the messages include headers such as digestAuthorization.sipbotonly, not by the library's settings loader. The startup line reportsextendedRetry=andsipTrace=.status.registeredis nowfalsewhile registration is failing.AGENTS.mddocuments all of this.Tests
A scripted loopback registrar (
FakeRegistrar) answers REGISTER with 200, 503, 402/403/404, a 401 challenge, or nothing. It counts transactions, not retransmissions. 26 new tests:LastRegistrationErroris set, and no REGISTER followsStartRegistration()after a hard failure sends one REGISTER while the 403 persists, then recovers once the account is accepted (both modes)dotnet test: 86 passed (60 existing + 26 new). Four full runs were all green. Build warnings unchanged (17).Live check against the lab PBX (VitalPBX / Asterisk 20.14)
sipbot serve --extended-retryregistered as the subject extension. Then, on the PBX, SIP UDP to and from that client's address and port only was dropped for 4 minutes.statusreportsregistered: falsefrom here on; attempt 1 is scheduled in 30 sNo "already running" appeared.
statussaidregistered: falsein all 12 polls during the outage. For comparison,masterstayedtruefor 43 polls and recovered only through SIPSorcery's 300 s retry.An earlier run of the first commit found the health check starting the next attempt 3 s into the current one. Commit 7a784ab fixes that and adds a test. In the run above, each attempt reports its own failure.
Compatibility
StartRegistration()once. They keep the same retry schedule and recovery, with two differences.IsRegisteredis now false during an outage. And a rejected account (for example a changed password) now stops registering instead of re-sending the rejected credentials, so restart after fixing it.StartRegistration().RegistrationState,LastRegistrationError, andSipConfig.RegistrationRetry. The previous constants became option defaults.🤖 Generated with Claude Code