fix(polymarket_client): defensive type guard + MAX_PAGES cap on keyset cursor loop (closes #936, #970) - #1016
Open
lzhao8956-glitch wants to merge 4 commits into
Open
fix(polymarket_client): defensive type guard + MAX_PAGES cap on keyset cursor loop (closes #936, #970)#1016lzhao8956-glitch wants to merge 4 commits into
lzhao8956-glitch wants to merge 4 commits into
Conversation
…e.py (closes valory-xyz#970) The send_polymarket_connection_request() helper would return an error-dict payload ({"error": "..."}) as if it were valid data when the SRR response had response.error == True. Callers (e.g. fetch_markets, the redeem loops) would then iterate the dict and call .get() on string keys, raising AttributeError and crashing the whole agent. Adding an early return on response.error prevents the bad payload from being passed downstream. (See issue valory-xyz#970.)
…alory-xyz#970) The three vulnerable loops in PolymarketRedeemBehaviour — _update_policy_for_redeemable_positions, _prepare_redeem_tx, and _redeem_via_builder — each iterated ``redeemable_positions`` without a type guard. When the upstream data-API returned an error-dict payload (one element, key="error"), Python iterated the dict KEYS (strings), and ``position.get("conditionId")`` raised AttributeError, crashing the whole redeem round. Adding ``isinstance(redeemable_positions, list)`` at the top of each method degrades gracefully (logs the error, returns empty). This complements the upstream fix in valory-xyz#970 (send_polymarket_connection_request now bails on response.error). Together they handle the failure mode from both sides — root cause AND defensive consumer.
…loses valory-xyz#970) Add isinstance(redeemable_positions, list) guard to the two remaining vulnerable methods: _update_policy_for_redeemable_positions and _prepare_redeem_tx. (_redeem_via_builder was already guarded in the previous commit.) This ensures all three redeem flows degrade gracefully when the upstream data-API returns an error-dict payload. Combined with the root-cause fix in send_polymarket_connection_request (response.error check), the agent will no longer crash on transient Polymarket data-API failures.
lzhao8956-glitch
changed the title
lzhao8956-glitch
changed the title
lzhao8956-glitch
changed the title
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
1 participant
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
Closes #936
Two robustness-vs-upstream concerns in
connection.py:fetch_markets(both flagged by @bennyjo on PR #931 review):response.get("events")canAttributeErrorif_request_with_retriesreturns a non-dict payload (JSON null, stray list, or string from a misbehaving edge proxy). Crashes here and bubbles up tofetch_marketsblanketexcept Exception, dropping the whole category.No client-side termination bound on the cursor loop. The pre-fix(polymarket): migrate /events to keyset pagination #931
len(events_data) < EVENTS_LIMIT: breakwas a natural ceiling; the post-fix(polymarket): migrate /events to keyset pagination #931 code relies entirely on the server eventually returning a falsynext_cursor. If the upstream ever returns the same cursor in a row,all_marketsgrows unbounded until OOM.Changes
MAX_PAGES = 100constant next toEVENTS_LIMIT(>=10x current single-call ceiling of ~10 events on Polystrat).while True:withfor _ in range(MAX_PAGES):outer loop.isinstance(response, dict)guard before.get()calls.for-elsewarning on cap-hit so the next refresh can re-attempt.Backwards compatibility
Out of scope (deferred)
_request_with_retriestyping-hygiene fix - separate PR.