diff --git a/packages/client/README.md b/packages/client/README.md index 1337c8fe..bd770abf 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -628,8 +628,8 @@ HTTP 422 when it will not serve one. - **Not the empty case:** an environment with zero skills is served an empty payload that commits normally. - **Recovery:** once the cause is fixed, call `start()` on the same store. Only `close()` is - final: a restart clears `failed`, resets the backoff, and held content stays readable - throughout. Restarting the process also works. + final: a restart clears `failed`, resets the backoff and `connection_failures`, and held + content stays readable throughout. Restarting the process also works. **Nothing above the store changes.** The accessors, verification, and `write_skills` see raw objects through the `SkillStore` interface and cannot tell which store produced them. diff --git a/packages/client/agents.md b/packages/client/agents.md index 8e1eb533..a57c3389 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -264,7 +264,8 @@ payload assigned to it, and chose a non-400 4xx because LD SDKs treat those as t `_classify_status` returns a `_FatalTransportError` and the normal give-up path runs: `failed` and `last_error` are set, `wait_for_skills` returns `False` at once, and `connection_failures` is left untouched (it counts consecutive *recoverable* failures; an -existing count is kept, not zeroed). +existing count is kept, not zeroed, until `start()` runs delivery again and `_rearm_waiters` +resets it). **The 422 message matches the TypeScript SDK's word for word, and stays short.** It names the one cause a customer can fix — a view-scoped SDK key — and refers every other case to diff --git a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py index 25f42918..fa19aef6 100644 --- a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py +++ b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py @@ -1503,12 +1503,14 @@ def start(self) -> FDv2SkillStore: def _rearm_waiters(self) -> None: """ Resets per-run state for a store being started again after it gave up: - the ended-delivery flag, ``failed``, and the failure count. A payload - already held still answers ``wait_for_skills``. Call with the lock held. + the ended-delivery flag, ``failed``, and the failure count, including + the one ``diagnostics`` reports. A payload already held still answers + ``wait_for_skills``. Call with the lock held. """ self._delivery_ended.clear() self._failed_reason = None self._failures = 0 + self._reader.diagnostics.connection_failures = 0 self._backoff_attempts = 0 if not self._first_payload.is_set(): self._released.clear() @@ -1557,7 +1559,8 @@ def wait_for_skills(self, timeout: float = 10.0) -> bool: has skills; see ``diagnostics``. Returns ``False`` on timeout, or early if delivery ends first (``close``, - or a failure that will not be retried). + or a failure that will not be retried). A store that gave up waits again + once ``start()`` runs delivery again; only ``close`` is final. """ self._released.wait(timeout=timeout) return self._first_payload.is_set() diff --git a/packages/client/tests/test_skills_fdv2.py b/packages/client/tests/test_skills_fdv2.py index 1864229b..0fbe85b8 100644 --- a/packages/client/tests/test_skills_fdv2.py +++ b/packages/client/tests/test_skills_fdv2.py @@ -2168,6 +2168,53 @@ def test_a_restart_resets_the_failure_count(self) -> None: # One failure in the new run, not three. assert store.diagnostics.connection_failures == 1 + def test_a_restart_reports_no_failures_before_the_new_run_has_any(self) -> None: + """The reported count starts over in ``start()``, not at the next failure. + + Read only after the new run fails, a stale count is overwritten and looks + reset. Read while the new run's first request is still in flight, it + would show the old run's failures on a store whose ``failed`` is None. + """ + + class _HoldsWhenScriptEnds(_ScriptedRequester): + def __init__(self, *outcomes: Any) -> None: + super().__init__(*outcomes) + self.release = threading.Event() + + def poll(self, basis: str | None, etag: str | None) -> Any: + if not self.outcomes: + self.calls.append((basis, etag)) + self.release.wait(timeout=10) + raise _RecoverableTransportError("released") + return super().poll(basis, etag) + + requester = _HoldsWhenScriptEnds( + _RecoverableTransportError("x"), + _RecoverableTransportError("x"), + _FatalTransportError("401"), + ) + store = FDv2SkillStore( + SDK_KEY, + mode="poll", + initial_backoff=0.001, + max_backoff=0.002, + _requester=requester, + ) + try: + store.start() + assert wait_until(lambda: store.failed is not None) + assert store.diagnostics.connection_failures == 2 + + calls = len(requester.calls) + store.start() + assert store.diagnostics.connection_failures == 0 + assert wait_until(lambda: len(requester.calls) > calls) + assert store.failed is None + assert store.diagnostics.connection_failures == 0 + finally: + requester.release.set() + store.close() + def test_a_404_stops_delivery_immediately(self, endpoint: Any) -> None: """A 404 means the endpoint does not exist for this credential.