Fail-soft on unknown template ids + map template 358 (AccountRmsUpdates) - #67
Open
aktiwers wants to merge 1 commit into
Open
Fail-soft on unknown template ids + map template 358 (AccountRmsUpdates)#67aktiwers wants to merge 1 commit into
aktiwers wants to merge 1 commit into
Conversation
Two related changes to the order plant message handling:
1. Fail-soft on unmapped template ids. `_convert_bytes_to_response` used to
`raise Exception("Unknown template ID: ...")` for any template id missing
from `TEMPLATES_MAP`. In the read/process loop that exception is caught and
logged with a full traceback, so a single recurring unmapped message type
floods the logs with ERROR tracebacks (see rundef#66). It now warns once per
template id and returns None; the read loops skip a None response and keep
going. One unknown message type can no longer drown out real errors.
2. Map template id 358 to AccountRmsUpdates and dispatch it through a new
`on_account_rms_update` client event, mirroring how 351/352/353 are wired.
Adds tests covering: an unmapped template id yields a single warning and no
raise while the loop continues, and a 358 frame decodes to AccountRmsUpdates
and fires on_account_rms_update.
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.
Fixes #66.
The order plant floods the logs with ERROR tracebacks (
Unknown template ID: 358) when Rithmic pushes a message type the library does not map. This PR has two parts that stand independently.1. Fail-soft on unmapped template ids
BasePlant._convert_bytes_to_responseraisedException(f"Unknown template ID: {template_id}")for any template id missing fromTEMPLATES_MAP. That exception is caught by the read/process loop and logged vialogger.exception(...), so a single recurring unmapped message type produces a full traceback for every message and drowns out real errors.It now:
None,_process_loopinbackground_task_mixin.pyand the inline loop in_send_and_recv_immediate) skip aNoneresponse and continue.This half is safe regardless of what template 358 actually is — it just stops one unhandled message type from flooding the logs.
2. Map template 358 →
AccountRmsUpdatesaccount_rms_updates_pb2.AccountRmsUpdatesis already vendored inprotocol_buffers/but was never registered inTEMPLATES_MAP. This adds:358: pb.account_rms_updates_pb2.AccountRmsUpdatestoTEMPLATES_MAP,OrderPlant._process_responsefiring a newon_account_rms_updateclient event, mirroring howon_bracket_update(353) is wired.On the 358 == AccountRmsUpdates mapping: this is verified circumstantially, not from an official Rithmic spec. On a live Rithmic paper session the template-358 frames decode cleanly against
AccountRmsUpdates(the RMS fields — auto-liquidation thresholds, buying power, etc. — populate correctly), and 358 sits in the order-plant notification range right after the 350–353 notifications. If you'd rather not commit to the mapping without an official reference, the fail-soft half (part 1) resolves the log-flood on its own and I'm happy to split this into two PRs — just let me know your preference.Tests
Added
tests/test_unknown_template.py(matches the existing plant-mock idiom):Nonereturn, and the process loop keeps dispatching subsequent messages;AccountRmsUpdatesand fireson_account_rms_update.Full suite:
30 passedlocally (27 pre-existing + 3 new) viaPYTHONPATH=. pytest testson Python 3.12. No live credentials required — everything runs against the existing offline mocks.