fix(agent): classify TypeError('NoneType ... not iterable') as retryable provider shape error#33399
Merged
Conversation
…ble provider shape error Salvages the intent of #33136 (@Brixyy) onto current main. The original PR was written against the pre-refactor monolithic run_agent.py and added a top-level _is_nonretryable_local_validation_error() helper. Both target functions have since been extracted to agent/conversation_loop.py:2869, so the salvage applies the equivalent guard inline at that canonical location rather than reintroducing the helper. ## Why After #33042 made our own Codex consumer structurally immune to NoneType crashes, third-party shims, mocked clients, and any future code path that hasn't migrated could still surface TypeError: 'NoneType' object is not iterable as a wire-shape mismatch. The agent loop's classifier currently treats ALL TypeError as a local programming bug and aborts non-retryable — users on stale Telegram/gateway turns saw bare "Non-retryable error (HTTP None)" with no recovery. This is a provider/SDK shape mismatch, not a local programming bug. The retry/fallback path should run, not be short-circuited. ## What agent/conversation_loop.py: extend is_local_validation_error to exclude TypeErrors whose message matches the NoneType-not-iterable shape (case- insensitive, both "NoneType" and "not iterable" must appear). tests/run_agent/test_jsondecodeerror_retryable.py: - update the mirror predicate to match the production check - add TestNoneTypeNotIterableIsRetryable class with 3 tests (the basic shape, message variants, unrelated TypeErrors still abort) - add TestAgentLoopSourceHasNoneTypeCarveOut to enforce the source-level invariant matches the test mirror ## Validation tests/run_agent/test_jsondecodeerror_retryable.py + tests/run_agent/test_31273_402_not_retried.py → 14/14 passing Co-authored-by: Brixyy <subrtt@gmail.com>
Contributor
🔎 Lint report:
|
1 task
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
3 participants
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.
Salvage of #33136 (@Brixyy) onto current main.
Why
After #33042 made our own Codex consumer structurally immune to NoneType crashes, third-party shims, mocked clients, and any future code path that hasn't migrated can still surface
TypeError: 'NoneType' object is not iterableas a wire-shape mismatch. The agent loop's classifier currently treats ALLTypeErroras a local programming bug and aborts non-retryable — users on stale Telegram/gateway turns saw bare "Non-retryable error (HTTP None)" with no recovery.This is a provider/SDK shape mismatch, not a local programming bug. The retry/fallback path should run, not be short-circuited.
Changes
agent/conversation_loop.py: extend theis_local_validation_errorpredicate to excludeTypeErrors whose message matches the NoneType-not-iterable shape (case-insensitive, both "NoneType" and "not iterable" must appear).tests/run_agent/test_jsondecodeerror_retryable.py:TestNoneTypeNotIterableIsRetryableclass (3 tests: shape match, message variants, unrelated TypeErrors still abort)TestAgentLoopSourceHasNoneTypeCarveOutenforces the source-level invariantHow this fits the cluster
The defensive-classifier layer that pairs with the structural fixes:
responses.stream()from our own paths (structural immunity)final.output = Noneto[]in the auxiliary adapter (defensive)Validation
tests/run_agent/test_jsondecodeerror_retryable.py+tests/run_agent/test_31273_402_not_retried.py→ 14/14 passingWhy not pure cherry-pick
@Brixyy's original PR was written against the pre-refactor monolithic
run_agent.pyand added a top-level helper function. Both target functions have since moved toagent/conversation_loop.py:2869, so the salvage applies the equivalent guard inline at the canonical extracted location.Attribution
Co-authored by @Brixyy. AUTHOR_MAP updated in follow-up commit.