fix(python): call _history_error_envelope on the class - #7
Open
Dusk1e wants to merge 1 commit into
Open
Conversation
f5f3bf8 moved _history_error_envelope from module scope into BulkHttpClient as a staticmethod, but the call site in _get_history_page stayed a bare name. Class-body names are not in scope inside method bodies, so every non-2xx history response raised NameError instead of HistoryHttpError. This affects get_fills_page, get_positions_page, get_funding_page, get_orders_page, get_activity_page and get_risk_page. tests/test_history_http.py already covers the intended behaviour and fails on main.
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.
Summary
_get_history_pagecalls_history_error_envelope(...)as a bare name, but f5f3bf8 moved that helper from module scope intoBulkHttpClientas a@staticmethod. Class-body names are not in scope inside method bodies, so the call resolves against globals, finds nothing, and every non-2xx history response raisesinstead of the intended
HistoryHttpError. Callers that catchHistoryHttpError— the documented contract — never see it, and the status code and error envelope are lost.All six public history readers go through
_get_history_page, so all six are affected:get_fills_page,get_positions_page,get_funding_page,get_orders_page,get_activity_page,get_risk_page.tests/test_history_http.pyalready covers the intended behaviour and fails onmain:Changes
Qualify the call with
self. One line; no behaviour change on the success path, and the helper itself is unchanged.Verification
The existing tests already assert the parts that were unreachable — that the status code survives, that a well-formed
{"error": {code, message}}body is preserved, and that an oversized or malformed body falls back to theHISTORY_HTTP_ERRORenvelope.I also checked the rest of
bulk_apifor the same pattern — a call to a bare name that only exists as a method on the enclosing class — and this was the only occurrence.