Skip to content

fix(telemetry): log an unreachable venue as a warning, not an ERROR traceback - #169

Merged
eaitbrahim merged 1 commit into
mainfrom
fix/transient-venue-unreachable-logging
Aug 6, 2026
Merged

fix(telemetry): log an unreachable venue as a warning, not an ERROR traceback#169
eaitbrahim merged 1 commit into
mainfrom
fix/transient-venue-unreachable-logging

Conversation

@eaitbrahim

Copy link
Copy Markdown
Contributor

An unreachable venue is now logged as a one-line warning instead of a full ERROR traceback, so grep ERROR in 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 by cb_client.get_accounts, then again by executor._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 Unauthorized in 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:

condition level body
unreachable, no cycle bound (a dashboard balance refresh) WARNING one line, unreachable=true + truncated error summary
unreachable, inside a trade cycle ERROR no traceback — rail 13 fails closed on a missing balance, so an order did not go out
anything else (auth, malformed, a bug) ERROR full traceback, byte-for-byte what log_exception emitted

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 adapter's HTTP stack. SSLError is deliberately excluded — a failed handshake can mean interception, which an operator must see at ERROR.

Control flow is untouched. get_accounts still raises; _fetch_available_quote still returns None so rail 13 still fails closed. Only the severity of the record changes.

Verification

  • 1902 tests pass, ruff check keel tests packages clean, mypy clean across 94 files.
  • 25 new tests, written failing first.
  • Validated against a real requests failure rather than only the name-matched fakes: ConnectionError → MaxRetryError → NameResolutionError → gaierror, all four in the match set.
  • Replaying all 124 recorded failures from keel-live.log through the classifier leaves 2 at ERROR — exactly the 401.

🤖 Generated with Claude Code

…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
eaitbrahim merged commit e25cc73 into main Aug 6, 2026
1 check passed
@eaitbrahim
eaitbrahim deleted the fix/transient-venue-unreachable-logging branch August 6, 2026 15:45
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>
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.

1 participant