Skip to content

[P2][Coverage review] Bound ActionCable setup and repair streaming cleanup/reconnect lifecycle #108

Closed

Description

@karlwaldman

Goal

Bound ActionCable setup and repair streaming cleanup/reconnect lifecycle

Evidence at frozen review baseline

oilpriceapi/streaming/client.py:130 applies open_timeout to websocket connect, then awaits welcome and subscription confirmation without a deadline. A fake connected socket whose recv never completes exceeded the configured open timeout; cancellation left that socket open. In _iterate, a failed reconnect inside the ConnectionClosed handler escapes instead of using remaining attempts: a local probe raised OSError after one attempt with max_reconnect_attempts=10.

Smallest change / deletion first

Keep the existing stream abstraction. Add one bounded setup lifecycle and reliable cleanup; remove duplicated or ineffective retry branches. Do not build a new streaming framework or background supervisor.

Acceptance and negative-path proof

  • Red tests cover upgraded socket with no welcome, no confirmation, rejected subscription, caller cancellation and failed aenter; all allocated sockets close.
  • Define/document whether open_timeout includes protocol setup, or use a clearly named bounded setup timeout without surprising unlimited waits.
  • Transient reconnect failures use the configured bounded consecutive budget; permanent auth/rejection stops with a useful error.
  • Closing/canceling prevents subsequent reconnects and leaves no pending task/socket.
  • Run streaming tests and a local ActionCable-compatible fixture smoke; no production websocket required.

Risk, delivery and rollback

MEDIUM — SDK connection lifecycle; preserve compatibility and bounded resource use.

Planning only. Reproduce against current main and check intervening/duplicate work before implementation. One focused reversible PR per admitted change, independent adversarial review, ordered merge and approved release/publish with artifact proof. If no change is needed, close with evidence. Rollback must preserve customer data and restore only a known-safe version; do not restore an unsafe public entrypoint to resolve an ownership question.

Parent plan: https://github.com/OilpriceAPI/oilpriceapi-api/issues/8015 . No application changes, remote abuse probes or production writes were made during this audit.

Activity

  1. karlwaldman commented on Sep 5, 2026

    @karlwaldman
    MemberAuthor

    Coverage-audit revision — current sprint admission

    Plan v2: oilpriceapi-api#8015 supersedes earlier scheduling comments. Rank 31/70; Monday, 2026-09-07, America/New_York. Low/medium-risk only; reclassify to a Friday/Saturday slot if boundary/compatibility risk is higher.

    Intended outcome: Bound ActionCable setup and repair streaming cleanup/reconnect lifecycle.

    Coverage ledger: #8056. Inventory/pattern screening is complete; exhaustive semantic/runtime sign-off is not. Before implementation, review the complete affected file set and callers, revalidate against current main, supply the missing negative-path proof, and retain prior scope corrections. Prefer deletion, then simplification; correct behavior needs no PR.

    No implementation is approved yet. After Karl approves: focused reversible PR → independent adversarial review → passing affected checks → merge in plan order → separately approved release/publish and artifact/customer-path verification. Maximum ten planned slots per night, not ten forced merges. If the dated window elapses before approval, re-anchor the calendar rather than backdate execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions