fix: raise on 401/403 instead of retrying, and fix README's undocumented async-with usage - #40
Merged
Merged
Conversation
…ted async-with usage TeslemetryStream never implemented __aenter__/__aexit__ - the connection lifecycle is deliberately listener-driven (async_add_listener connects on the first listener, disconnects on the last removed), with connect()/ close()/listen() as the manual alternative. The README's async-with examples didn't match either path and raised TypeError on first use, so they're rewritten to the listener-driven pattern the class actually supports. Separately, aiohttp.ClientResponseError is a subtype of ClientError, so a 401/403 from a bad access token was being retried forever as if it were a transient network blip - the caller saw no events and no error. __anext__ now treats 401/403 as terminal and raises TeslemetryStreamAuthenticationError instead of retrying; every other ClientError (including other response statuses) keeps the existing backoff-and-reconnect behavior unchanged.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Intent
async with TeslemetryStream(...) as stream:, but the class has never implemented__aenter__/__aexit__- following the README raisesTypeError: ... does not support the asynchronous context manager protocolon the very first line.AGENTS.mdalready documents theasync_add_listener/auto-close machinery in detail, with dedicated lifecycle tests), and a context manager whose__aenter__does nothing but returnselfwould just be a second, redundant way to spell the same thing while implying a connect step that isn't actually needed. Rewrote the two affected examples to plain instantiation +async_add_listener, and reworded theUsagesection to describe the listener-driven default plus the manualconnect()/close()/listen()alternative that was already correctly documented elsewhere in the README.aiohttp.ClientResponseError(raised byconnect()'sraise_for_status=True) is a subtype ofaiohttp.ClientError, so a 401/403 from a bad or revoked access token was falling into__anext__'s generic client-error handler and being retried forever with backoff - the caller saw no events and no error, indistinguishable from a quiet vehicle.__anext__now special-cases a 401/403ClientResponseError: it stops the stream and raises a newTeslemetryStreamAuthenticationError(chaining the original error), instead of retrying. Every otherClientError, including other response statuses, keeps the existing backoff-and-reconnect behavior unchanged - verified bytests/test_auth_failure.py, which covers both the 401/403 case and a genuine transientClientErrorstill retrying and reconnecting.Ran
uv run --with ruff ruff check .anduv run --with mypy mypy teslemetry_stream(both clean save for pre-existing lint findings in the example scripts, unrelated to this change), and everytests/test_*.pyscript directly.