Skip to content

narrow bare except Exception in provider token status and model fetch#13

Open
HrachShah wants to merge 14 commits into
mainfrom
fix/provider-token-and-model-exception-narrow
Open

narrow bare except Exception in provider token status and model fetch#13
HrachShah wants to merge 14 commits into
mainfrom
fix/provider-token-and-model-exception-narrow

Conversation

@HrachShah

@HrachShah HrachShah commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Summary

Narrows the bare except Exception blocks in two FreeRelay provider modules to the specific exception types their inner operations raise.

Changes

freerelay/providers/codex.pyget_codex_token_status

The function tries to decode the ChatGPT OAuth JWT to surface

Summary by Sourcery

Tighten exception handling across Redis, HTTP, parsing, cryptography, routing, and middleware paths to avoid masking unexpected errors while preserving existing fallbacks and logging behavior.

Enhancements:

  • Replace broad except Exception clauses with specific Redis, HTTPX, JSON/parsing, and cryptography-related exception types throughout control-plane learners, data-plane ingress, and shared services.
  • Constrain error handling in routing, execution, observability, and middleware components to expected validation, I/O, and value errors to make failures more predictable and diagnosable.

Zo Bot added 14 commits May 26, 2026 19:29
…to specific types — WebSocket.send_text raises (RuntimeError, ConnectionResetError) when disconnected, get_metrics raises (RuntimeError, ValueError) for asyncio/data issues; catching all Exception masked these specific failures
…onseError — redis.xrange and xdel raise ResponseError on failures; catching all Exception masked this specific redis failure
… specific types — base64.b64decode raises (ValueError, TypeError) for invalid base64 input, AESGCM.decrypt raises ValueError when key is wrong or tag verification fails; catching all Exception masked these specific failures
…ponseError — redis.xadd raises ResponseError for stream errors, and falling back to in-memory on that specific failure is more targeted than masking all exceptions
…er_info catches (OSError, KeyError, TypeError, IndexError) from dict/getattr access on the Supabase response, and the idempotency cache handler catches (ValueError, TypeError) from json.loads; catching all Exception masked these specific failures
…fic types — xreadgroup/xpending/xclaim raise ResponseError, OutcomeRecord.from_stream raises (ValueError, TypeError), xack operates on Redis int response — catching all Exception masked these specific failures
…SON decode errors — xack returns an integer count, so only Redis-level failures apply
…fic types — publish/rollback/snapshot/delete/subscribe use Redis ops (ResponseError), load_current and history use (json.JSONDecodeError, ResponseError), and callback uses (json.JSONDecodeError, ValueError, TypeError) — catching all Exception masked these specific failures
…rror, TypeError, ValueError) — eval raises SyntaxError for malformed expressions, NameError for undefined variables, TypeError for type mismatches, and ValueError for invalid operations; catching all Exception masked these specific evaluation failures
…aml.YAMLError, ValueError) — open raises OSError for file I/O failures, yaml.safe_load raises YAMLError for malformed YAML, and model_validate raises ValueError for invalid schema; catching all Exception masked these specific file and parsing failures
…ogger — model_validate_json raises (ValueError, TypeError), LSH query raises (TypeError, ValueError, KeyError), execute_hedged catches (ProviderError, ValueError, TypeError), Supabase execute raises (OSError, ValueError, TypeError); catching all Exception masked these specific failures
…se import raises ImportError, request.body() raises OSError, model_validate_json raises (ValueError, TypeError); catching all Exception masked these specific failures
… key lookup and rate limit checks catch redis.asyncio.RedisError, idempotency check and store catch RedisError, httpx request cancellation catches (OSError, httpx.RequestError)
…etch

get_codex_token_status tries to decode the ChatGPT OAuth JWT to surface
expiry and plan_type, then falls back to "Token found (unable to decode
JWT)" if anything goes wrong. The body only does operations on a string
JWT: token.split
@sourcery-ai

sourcery-ai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Reviewer's Guide

Narrows previously broad except Exception handlers across Redis, HTTP, JSON/parsing, crypto, routing, auth, and provider code to concrete, expected exception types, improving error hygiene while keeping existing control flow and fallbacks intact.

Flow diagram for Redis idempotency check with narrowed RedisError handling

flowchart TD
    A[check request_id] --> B{_redis is not None?}
    B -- no --> C[_check_memory]
    B -- yes --> D[_check_redis]
    D --> E{redis.asyncio.RedisError?}
    E -- yes --> F[log 'Redis idempotency check failed' and _check_memory]
    E -- no --> G[return Redis result]
Loading

File-Level Changes

Change Details Files
Constrain Redis-related exception handling in control-plane learners and shared tenancy/audit code to Redis-specific errors while preserving logging and fallback behavior.
  • Replace broad except Exception in policy publishing, loading, history, rollback, snapshot, subscription, and delete paths with redis.asyncio.ResponseError, leaving return values and logging unchanged.
  • Update outcome consumer methods (ensure_group, consume_batch, acknowledge, pending_info, claim_stale) to catch redis.asyncio.ResponseError for Redis operations and explicit JSON/parse exceptions for record decoding.
  • Adjust audit purging, persisting, and retrieval helpers to catch redis.asyncio.ResponseError instead of generic exceptions, keeping memory fallbacks and error logs the same.
freerelay/control_plane/learner/policy_publisher.py
freerelay/control_plane/learner/outcome_consumer.py
freerelay/shared/tenancy/audit.py
Tighten Redis error handling in data-plane ingress paths (idempotency, auth, rate limiting) to only treat Redis failures as non-fatal and fall back to in-memory alternatives.
  • Import redis.asyncio in idempotency, auth, and rate_limit modules and narrow generic except to redis.asyncio.RedisError around Redis check/store and rate limit logic.
  • Keep existing log messages and in-memory fallback behavior identical so functional semantics stay the same.
freerelay/data_plane/ingress/idempotency.py
freerelay/data_plane/ingress/auth.py
freerelay/data_plane/ingress/rate_limit.py
Refine HTTP, network, and websocket-related exception handling to concrete error types for cancellation, provider model fetch, dashboard websockets, and Supabase logging.
  • In the cancellation executor, restrict the catch around httpx_client.cancel_request to (OSError, httpx.RequestError), add the exception to the debug log message, and import httpx.
  • In the OpenCode provider, replace a single bare except with separate handlers for httpx.RequestError, (ValueError, TypeError), and AttributeError, all of which silently fall through to the existing fallback model list.
  • In the dashboard websocket manager, narrow send/broadcast exception handling to (RuntimeError, ConnectionResetError) for client sends and (RuntimeError, ValueError) for metrics production, keeping disconnect cleanup and logging unchanged.
  • In the Supabase logger, change the catch to (OSError, ValueError, TypeError) so only expected client/serialization errors are swallowed with an error log.
freerelay/data_plane/execution/cancellation.py
freerelay/providers/opencode.py
freerelay/dashboard/ws.py
freerelay/core/observability/supabase_logger.py
Narrow JSON/model parsing and cache-related exception handling to explicit validation and type errors in caching, execution, routing, and middleware.
  • In the intelligence cache lookup, catch (ValueError, TypeError) when validating cached responses and (TypeError, ValueError, KeyError) when iterating multi-candidate entries, pruning invalid cache entries but otherwise preserving behavior.
  • In the main FastAPI app, replace a generic import failure catch with ImportError for ORJSON and narrow request body read exceptions to OSError and request validation to (ValueError, TypeError), still returning 400s on invalid input.
  • In the executor’s hedged execution path, restrict caught failures to ProviderError and simple data errors (ValueError, TypeError), maintaining circuit-breaker failure recording and metric emission for these cases.
  • In the routing engine capability matrix loader, constrain failure cases to (OSError, yaml.YAMLError, ValueError) when reading/parsing the YAML file.
  • In routing policy evaluation, catch only (SyntaxError, NameError, TypeError, ValueError) from eval and continue to log a warning and return False.
  • In middleware auth and idempotency, narrow Supabase auth failures to (OSError, KeyError, TypeError, IndexError) and idempotency response caching failures to (ValueError, TypeError) without changing outward behavior.
freerelay/core/intelligence/cache.py
freerelay/main.py
freerelay/core/execution/executor.py
freerelay/core/routing/engine.py
freerelay/core/routing/policy.py
freerelay/middleware/auth.py
freerelay/middleware/idempotency.py
Tighten cryptographic, JWT, and token-related exception handling to explicit parse/decrypt errors.
  • In the crypto helpers, replace broad exception handling on base64 decoding with (ValueError, TypeError) and on AES-GCM decrypt with ValueError, both re-raised as ValueError with clearer messages.
  • In the Codex provider’s get_codex_token_status, narrow token decoding errors to (TypeError, ValueError) so only expected decode/parse issues are silently ignored while returning the default status.
freerelay/shared/security/crypto.py
freerelay/providers/codex.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@HrachShah, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 8 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 54d535bd-e0bd-4e82-92d4-454f99e973bc

📥 Commits

Reviewing files that changed from the base of the PR and between ec9d17f and 53e8976.

📒 Files selected for processing (19)
  • freerelay/control_plane/learner/outcome_consumer.py
  • freerelay/control_plane/learner/policy_publisher.py
  • freerelay/core/execution/executor.py
  • freerelay/core/intelligence/cache.py
  • freerelay/core/observability/supabase_logger.py
  • freerelay/core/routing/engine.py
  • freerelay/core/routing/policy.py
  • freerelay/dashboard/ws.py
  • freerelay/data_plane/execution/cancellation.py
  • freerelay/data_plane/ingress/auth.py
  • freerelay/data_plane/ingress/idempotency.py
  • freerelay/data_plane/ingress/rate_limit.py
  • freerelay/main.py
  • freerelay/middleware/auth.py
  • freerelay/middleware/idempotency.py
  • freerelay/providers/codex.py
  • freerelay/providers/opencode.py
  • freerelay/shared/security/crypto.py
  • freerelay/shared/tenancy/audit.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/provider-token-and-model-exception-narrow

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 and usage tips.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In the Redis-backed components (e.g. policy_publisher, outcome_consumer, audit), consider catching the broader redis.asyncio.RedisError rather than only ResponseError so connection/timeouts and other operational Redis failures are still handled by these error paths.
  • In providers/opencode.fetch_opencode_models, the three separate except blocks that all just pass can be combined into a single except (httpx.RequestError, ValueError, TypeError, AttributeError): for clarity, and you may want to add minimal logging there to make silent failures easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the Redis-backed components (e.g. policy_publisher, outcome_consumer, audit), consider catching the broader redis.asyncio.RedisError rather than only ResponseError so connection/timeouts and other operational Redis failures are still handled by these error paths.
- In providers/opencode.fetch_opencode_models, the three separate except blocks that all just `pass` can be combined into a single `except (httpx.RequestError, ValueError, TypeError, AttributeError):` for clarity, and you may want to add minimal logging there to make silent failures easier to diagnose.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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.

1 participant