Harden bus subscriber + readsb parser (PR #4/#5 review)#8
Conversation
Address the valid items from the Gemini reviews on the merged M1.2c and M2.1a slices, now that readsb is live on the bus (M2.1b). bus/client.apply_payload — guard the handle() (state apply) call. A bug or bad-state error while applying one record must not propagate and tear down the single subscriber ingest loop that feeds every source and client (PRD §37 failure isolation). Logged and dropped like a malformed payload. readsb parser robustness (PRD §17.2 "validate source responses"): - _num rejects NaN/Infinity. Python's json accepts them by default, but they are invalid to re-emit and would crash a record's serialization on publish. Non-finite numeric fields normalize to None. - _epoch_to_dt guards datetime.fromtimestamp against OverflowError/OSError/ ValueError on out-of-range epochs (returns None -> snapshot falls back to received_at) instead of crashing the whole snapshot on a bad top-level `now`. - absurd per-aircraft age no longer overflows timedelta; the track is kept, anchored at snapshot now. - defensive isinstance(data, dict) guard at the parser entry. Adds tests for each. scripts/check.sh green (67 passed). Rejected from the review: inferring on-ground from a `g` field — not a documented readsb aircraft.json field (guardrail: no invented API fields). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request enhances the robustness and failure isolation of the readsb adapter and the bus client. Key changes include rejecting non-finite floats, handling potential exceptions (such as OverflowError, OSError, and ValueError) during timestamp and age calculations, and isolating exceptions raised during state application in the subscriber loop. The review feedback suggests further improving robustness by catching OverflowError during float conversion of extremely large integers in _num, and logging the specific exception details when dropping malformed payloads to aid in troubleshooting.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if isinstance(value, (int, float)): | ||
| return float(value) | ||
| result = float(value) | ||
| return result if math.isfinite(result) else None |
There was a problem hiding this comment.
Converting an extremely large integer (e.g., 10**309 or larger) to a float in Python raises an OverflowError rather than returning infinity. Since JSON allows arbitrarily large integers, if a client sends a very large integer for a numeric field (such as the top-level now field), _num will raise an unhandled OverflowError. Because the top-level now parsing is not wrapped in a try-except block in parse_aircraft_snapshot, this will crash the entire snapshot parsing, violating the failure isolation principle (PRD §37). Wrapping the float conversion in a try-except block to catch OverflowError prevents this crash.
| if isinstance(value, (int, float)): | |
| return float(value) | |
| result = float(value) | |
| return result if math.isfinite(result) else None | |
| if isinstance(value, (int, float)): | |
| try: | |
| result = float(value) | |
| except OverflowError: | |
| return None | |
| return result if math.isfinite(result) else None |
| except (ValidationError, RecordTooLargeError, ValueError): | ||
| log.warning("dropping malformed record payload") | ||
| return False |
There was a problem hiding this comment.
When dropping a malformed record payload due to validation or size errors, the actual exception details are discarded. Logging the exception message is highly beneficial for troubleshooting and debugging schema mismatches or corrupted payloads in production.
| except (ValidationError, RecordTooLargeError, ValueError): | |
| log.warning("dropping malformed record payload") | |
| return False | |
| except (ValidationError, RecordTooLargeError, ValueError) as exc: | |
| log.warning("dropping malformed record payload: %s", exc) | |
| return False |
Addresses the valid items from the Gemini reviews on the already-merged M1.2c (bus) and M2.1a (readsb parser) slices — now relevant because M2.1b put the readsb parser live on the bus.
Changes
bus/client.apply_payload— guard thehandle()(state-apply) call. A bug or bad-state error while applying one record must not propagate and tear down the single subscriber ingest loop that feeds every source and client (PRD §37). It's now logged and dropped like a malformed payload. (This was the highest-value item in the review backlog.)readsbparser robustness (PRD §17.2 "validate source responses"):_numrejectsNaN/Infinity— Python'sjsonaccepts them by default, but they're invalid to re-emit and would crash a record's serialization on publish. Non-finite numeric fields normalize toNone._epoch_to_dtguardsdatetime.fromtimestampagainstOverflowError/OSError/ValueErroron out-of-range epochs, so a bad top-levelnowfalls back toreceived_atinstead of crashing the whole snapshot.timedelta; the track is kept, anchored at snapshotnow.isinstance(data, dict)guard at the parser entry.Verification
Adds a test for each path.
scripts/check.shgreen (ruff + mypy strict + pytest, 67 passed).Rejected from the review
Inferring on-ground from a
"g"field — not a documented readsbaircraft.jsonfield, so it stays out per the "no invented API fields" guardrail.🤖 Generated with Claude Code