Skip to content

fix(subscriptions): refuse malformed list/events 200s and replaying cursors (#142) - #147

Merged
karlwaldman merged 1 commit into
mainfrom
fix/142-subscriptions-malformed-list-events
Sep 13, 2026
Merged

karlwaldman merged 1 commit into
mainfrom
fix/142-subscriptions-malformed-list-events

Conversation

@karlwaldman

@karlwaldman karlwaldman commented Sep 13, 2026

Copy link
Copy Markdown
Member

Summary

subscriptions.list() and subscriptions.events() (sync and async) unwrapped with data.get("subscriptions", []), data.get("events", []) and data.get("cursor"). A 200 without those keys read as "no subscriptions" / "no new events" with cursor=None. Fed back through the documented loop events(since=page.cursor), that None dropped since, and the poller replayed the account's whole event history.

  • Response. New unwrap_subscription_list and unwrap_events_page in oilpriceapi/_subscriptions_common.py are used by both clients. They follow the unwrap_subscription style and reuse the existing _malformed from _fuel_surcharge_common; no new error helper. Each raises OilPriceAPIError(code="MALFORMED_RESPONSE", raw_body=...) when:
    • data.subscriptions or data.events is missing or not a list;
    • a record is not an object or fails its model;
    • data.cursor is not a non-negative int (bool excluded);
    • data.has_more is not a bool;
    • the cursor is behind since, or behind an event's seq in the same page, since following it would replay events.
  • Request. New validate_since refuses a since the API would read as 0 or truncate, before any request is sent. Refused values: "abc", "", "41", -1, 1.5, True, [41]. It raises ValidationError(field="since", status_code=None). None (first poll) and 0 stay allowed.
  • A genuinely empty {"subscriptions": []} or {"events": [], "cursor": 41, "has_more": false} is still an empty success, and page.cursor is now always an int.

No pydantic validator was added and no raw ValueError is raised.

What the API actually does with the cursor

V1::SubscriptionsController on oilpriceapi-api origin/main:

  • #events sets since = params[:since].to_i, then cursor = events.last&.seq || since, and always renders {cursor, has_more, events}.
  • #index always renders {subscriptions: [...]}.

Live probe against api.oilpriceapi.com on 2026-09-13, using the test account, which has 101 events:

since sent HTTP first seq cursor has_more
omitted 200 1 100 true
0 200 1 100 true
abc 200 1 100 true
-1 200 1 100 true
empty string 200 1 100 true
1.5 200 2 101 true
999999999 200 none 999999999 false

A missing or invalid cursor is accepted with a 200 and silently restarts from event 1. The server never returns an error for it, so the SDK has to refuse it. The 999999999 row is a separate API-side hazard, filed as OilpriceAPI/oilpriceapi-api#8491 together with the silent replay: a cursor past the latest event is echoed back and the poller never sees new events.

Nothing was created. The probe only issued GETs against the events route.

Red (tests written first, run on unchanged origin/main a5304b3)

$ pytest tests/unit/test_subscriptions_list_events_strict.py -q --no-cov
...
======================== 62 failed, 10 passed in 1.17s =========================

Failure reasons, counted:

  38 E       Failed: DID NOT RAISE OilPriceAPIError
  14 E       Failed: DID NOT RAISE ValidationError
   2 E       TypeError: oilpriceapi.models.SubscriptionEvent() argument after ** must be a mapping, not int
   2 E       TypeError: oilpriceapi.models.Subscription() argument after ** must be a mapping, not str
   2 E       TypeError: 'NoneType' object is not iterable
   2 E       pydantic_core._pydantic_core.ValidationError: 1 validation error for Subscription
   1 E               TypeError: '<=' not supported between instances of 'int' and 'MagicMock'
   1 E               TypeError: '<=' not supported between instances of 'int' and 'AsyncMock'

The 10 that passed on main are the success paths (live list body, live events page, empty page, first poll without since), kept as regression guards.

Green

$ pytest tests/unit/test_subscriptions_list_events_strict.py tests/unit/test_subscriptions_resource.py tests/unit/test_async_subscriptions_resource.py tests/unit/test_subscriptions_lifecycle.py -q --no-cov
============================= 377 passed in 1.84s ==============================

Full suite. Baseline re-measured on origin/main a5304b3 with all extras installed:

passed failed skipped
origin/main 1884 3 54
this branch 1956 3 54

The 3 failures are the live tests/integration/test_demo_contract.py 429s on both runs.

  • ruff check oilpriceapi/: All checks passed!
  • mypy oilpriceapi/ --ignore-missing-imports, in a CI-equivalent .[dev] environment: Success, no issues found in 55 source files.
  • python scripts/validate_storefront_claims.py: validated 76 public surfaces.

Merge order

Both this PR and the #145 PR add entries under ## [Unreleased] / ### Fixed in CHANGELOG.md. Merge this one first. The #145 branch then takes a merge of origin/main (no rebase) to resolve the CHANGELOG hunk.

Closes #142

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

…ursors (#142)

subscriptions.list() and events(), sync and async, unwrapped with
data.get("subscriptions", []) / data.get("events", []) / data.get("cursor"),
so a 200 without those keys read as "no subscriptions" / "no new events"
with cursor=None. Fed back as events(since=page.cursor), None dropped since;
the API reads params[:since].to_i, so the poller replayed every event.

- unwrap_subscription_list / unwrap_events_page in _subscriptions_common,
  shared by both clients, raise OilPriceAPIError(MALFORMED_RESPONSE) with the
  raw body for a missing or mistyped collection, an invalid record, a
  non-integer or negative cursor, a non-boolean has_more, or a cursor behind
  since or behind an event in the page. They reuse _malformed from
  _fuel_surcharge_common. An empty list or page the API actually sent is
  still an empty success.
- validate_since refuses since values the API reads as 0 or truncates
  ("abc", "", "41", -1, 1.5, True) with ValidationError(field="since",
  status_code=None) before any request.

Closes #142

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 4877d4b4-edcc-4152-a591-2ea568508d65


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@karlwaldman
karlwaldman merged commit c6f76ec into main Sep 13, 2026
7 checks passed
@karlwaldman
karlwaldman deleted the fix/142-subscriptions-malformed-list-events branch September 13, 2026 20:57
karlwaldman added a commit that referenced this pull request Sep 13, 2026
Resolve the CHANGELOG ### Fixed conflict with #147 by keeping both entries.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2][bug] subscriptions.list() and events() turn a malformed 200 into an empty success

1 participant