elm327: leave a trace when we swallow an exception (#20) - #69
Open
ThinkOffApp wants to merge 1 commit into
Open
ThinkOffApp wants to merge 1 commit into
ThinkOffApp wants to merge 1 commit into
Conversation
hermes's review, 31 Aug: four bare `except Exception:` with no logging.
The handlers are right to continue - an OBD dongle vanishing mid-drive is
normal, not an error - but with nothing recorded, a decoder bug and a missing
dongle look identical in the field. The two decode sites are the sharp ones:
a decoder raising makes a reading disappear from the dash with no sign that
anything went wrong, which is the same shape as the other faults found today.
_swallowed(site, exc) now records all four:
close os.close on a vanished fd
decode pid 0x.. basic PID decoder raised -> reading dropped
decode ext pid 0x.. extended decoder raised -> tile stays blank
scan_supported_quiet capability scan failed -> falls back to every PID,
so the sweep looks merely unlucky
Deduped deliberately. "Dongles vanish as a rule" means a plain log would
flood the journal on exactly the drive being diagnosed, so the first
occurrence of each (site, exception type) prints and then only the 10th,
100th and 1000th. Behaviour is unchanged: every site still swallows and
returns what it returned before.
Writes to stderr, never stdout - this module's CLI prints JSON and a stray
line would corrupt it. A test asserts that, because it is the kind of thing a
later edit breaks silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
2 participants
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.
Fixes #20 (hermes's review, 31 Aug).
Four bare
except Exception:with no logging. The handlers are right to continue — an OBD dongle vanishing mid-drive is normal, not an error — but with nothing recorded, a decoder bug and a missing dongle look identical in the field.The sharp ones
The two decode sites: a decoder raising makes a reading disappear from the dashboard with no sign anything went wrong. Same shape as the other faults found today — a failure that is indistinguishable from normal operation.
closeos.closeon a vanished fddecode pid 0x..decode ext pid 0x..scan_supported_quietDeduped on purpose
"Dongles vanish as a rule" means a plain log would flood the journal on exactly the drive you are trying to diagnose. First occurrence of each
(site, exception type)prints, then only the 10th / 100th / 1000th.Behaviour is unchanged — every site still swallows and returns exactly what it returned before.
stderr, never stdout
This module's CLI prints JSON; a stray line would corrupt it. A test asserts that, because it is the kind of thing a later edit breaks silently.
Tests
tests/test_elm327_traces.py, 6 cases: first occurrence reported with type and message, repeats summarised rather than printed, the x10 milestone still surfaces, distinct sites and exception types tracked separately, nothing on stdout, and all four call sites still present.Suite failure on this branch is
test_dash_loop's missing pytest — pre-existing onmain, fixed separately in #64.Not verified on the car
Logic verified in tests; the car is unreachable, so no field trace has actually been captured from a real dongle.
🤖 Generated with Claude Code