fix(telemetry): log an unreachable venue as a warning, not an ERROR traceback - #169
Merged
Merged
Conversation
…raceback
A 35-minute offline window on 2026-08-06 (laptop asleep, DNS not up after
wake) wrote 124 ERROR records with full 20-frame tracebacks into
keel-live.log: the TUI polls the live balance every 30s, and every failed
poll was logged twice -- once by `cb_client.get_accounts`, then again by
`executor._fetch_available_quote`, which re-logs the exception the client
already logged and re-raised.
Nothing was broken. The venue was unreachable, every caller already fails
soft, and the condition cleared by itself. The cost was that a real
`401 Unauthorized` in the same log is one line among 124 -- `grep ERROR`
told the operator nothing.
Add `telemetry.log_venue_failure`, for any call that crosses the network to
a venue. It picks severity by what the failure actually cost:
- unreachable, no cycle bound (a dashboard balance refresh) -> WARNING,
one line, `unreachable=true` plus a truncated `error` summary
- unreachable, inside a trade cycle -> ERROR: rail 13 fails closed on a
missing balance, so an order did not go out. Still no traceback, the
cause is known
- anything else (auth, malformed, a bug) -> ERROR with the full
traceback, byte-for-byte what `log_exception` emitted before
Unreachability is matched on exception type NAME over the __cause__ chain,
so `keel-core` needs no `requests`/`urllib3` dependency to classify their
exceptions and the check holds for any broker's HTTP stack. Verified
against a real `requests` failure: ConnectionError -> MaxRetryError ->
NameResolutionError -> gaierror. `SSLError` is deliberately excluded -- a
failed handshake can mean interception, which an operator must see.
Control flow is untouched: `get_accounts` still raises, and
`_fetch_available_quote` still returns None so rail 13 fails closed.
Replaying the 124 recorded failures through the classifier leaves 2 at
ERROR -- exactly the 401.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 6, 2026
Patch: #169 changes log severity only. `cb_client.get_accounts` still raises and `executor._fetch_available_quote` still returns None, so rail 13 fails closed exactly as before -- no behaviour an operator or a rail can observe has changed, only whether a record lands at ERROR with a traceback or at WARNING on one line. uv.lock is relocked in the SAME commit, per 0.5.0. Leaving it behind is what made the 0.4.0 release fail its self-identification check: CI's `uv sync` rewrote a tracked file, so the artifact reported (DIRTY). Verified here with `uv sync --frozen`, which accepted the lock and rewrote nothing. The live deployment is on 0.5.0, whose TUI balance poll writes a 20-frame ERROR traceback every 30s whenever the machine is offline; cutting this release is what gets the fix there. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
eaitbrahim
added a commit
that referenced
this pull request
Aug 7, 2026
* chore: bump to 0.5.2 for the UTC detector retiming and its entry gate Patch: no schema change, no new capability. #172 retimed the live detector to 01:20 UTC (cutting ~12h of detection lag), #174 made that safe, and #169 downgraded an unreachable venue from an ERROR traceback to a warning. #174 is the reason this release exists rather than #172 alone. The retiming trades a 13-hour data-publication margin for 20 minutes, and nothing on the live entry path dedupes an order -- so the freshness gate in `run_once` that withholds ALL entries when any bar is unconfirmed has to ship in the SAME wheel as the schedule it protects. Installing this wheel is a prerequisite for copying the new keel-live-run.sh/com.keel.live.plist to the deployment; the shell files alone would be the unsafe half. uv.lock relocked in the SAME commit, per 0.5.0 and 0.5.1. Verified with `uv sync --frozen`, which accepted the lock and rewrote nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): skip the macOS-only real-script tests instead of erroring on Linux `main` went red the moment #174 landed. The new real-script tests execute the shipped `keel-live-run.sh` under `/usr/bin/sandbox-exec` (so a test run can never fire a real notification on a machine that also trades real money), with a `date` shim built on BSD `date -r` and stamp-failure cases built on `chflags uchg`. None of those exist on Linux, so all 45 invocations died with FileNotFoundError on the ubuntu runner. This is not a coverage question worth solving portably. The artifact under test IS a macOS deployment -- launchd plus `osascript` -- and the script only ever runs on the box that owns the launchd job. So on non-Darwin the real-script tests SKIP, and the schedule INVARIANT (exactly one cycle per UTC date, across both DST transitions) stays covered everywhere, because that is proven by the pure-Python model tests, which are platform-neutral. One runtime guard in `_run_script` covers all 45 call sites, since every invocation already funnels through it. The five tests that reach for `chflags` BEFORE running the script carry an explicit `@_macos_only` mark, where the runtime guard would come too late to stop a FileNotFoundError. Blocks the release otherwise: `release.yml` refuses to publish unless tests pass, so 0.5.2 could not have been cut with main in this state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
An unreachable venue is now logged as a one-line warning instead of a full ERROR traceback, so
grep ERRORin the live log means something again.What was happening
A 35-minute offline window on 2026-08-06 — laptop asleep, DNS not up after wake — wrote 124 ERROR records with 20-frame tracebacks into
keel-live.log. The TUI polls the live balance every 30s (_BALANCE_REFRESH_SEC), and every failed poll was logged twice: once bycb_client.get_accounts, then again byexecutor._fetch_available_quote, which re-logged the exception the client had already logged and re-raised.Nothing was broken. The venue was unreachable, every caller already fails soft, and the condition cleared by itself. The cost was that a real
401 Unauthorizedin the same log is one line among 124.What changed
telemetry.log_venue_failure, for any call that crosses the network to a venue. It picks severity by what the failure actually cost:unreachable=true+ truncatederrorsummarylog_exceptionemittedUnreachability is matched on exception type name over the
__cause__chain, sokeel-coreneeds norequests/urllib3dependency to classify their exceptions, and the check holds for any broker adapter's HTTP stack.SSLErroris deliberately excluded — a failed handshake can mean interception, which an operator must see at ERROR.Control flow is untouched.
get_accountsstill raises;_fetch_available_quotestill returnsNoneso rail 13 still fails closed. Only the severity of the record changes.Verification
ruff check keel tests packagesclean, mypy clean across 94 files.requestsfailure rather than only the name-matched fakes:ConnectionError → MaxRetryError → NameResolutionError → gaierror, all four in the match set.keel-live.logthrough the classifier leaves 2 at ERROR — exactly the 401.🤖 Generated with Claude Code