Skip to content

chore: ads/sdk resilience fixes#1

Merged
Sytten merged 4 commits intocaido-community:mainfrom
GangGreenTemperTatum:ads/sdk-resilience-fixes
Apr 12, 2026
Merged

chore: ads/sdk resilience fixes#1
Sytten merged 4 commits intocaido-community:mainfrom
GangGreenTemperTatum:ads/sdk-resilience-fixes

Conversation

@GangGreenTemperTatum
Copy link
Copy Markdown
Contributor

<3 you guys big time for this

tldr of proposed changes

was writing some automation scripts against my caido instance and ran into a few issues:

  • token expired mid-script, graphql calls blew up with AuthorizationUserError while rest calls silently recovered — turns out the graphql client had no auth retry logic even though has_authorization_error() was already written and unused
  • script hung forever when my instance was offline — no default timeout on the graphql http/ws transports (rest client defaults to 5s but graphql was None)
  • updated my caido instance and started getting AttributeError: 'CloudUserError' object has no attribute 'reason' — the error match statements had no default branch so new server-side enum variants left error objects half-constructed
  • to_user_error() was returning None for recognized caido error codes with unknown reasons, silently dropping the error classification

changes

  • add case _: default branches to CloudUserError, PluginUserError, StoreUserError, ProjectUserError match statements
  • add auth error detection + token refresh + single retry to graphql _execute(), matching the existing rest client pattern
  • default graphql timeout to 30s for both http and websocket transports, also fixed int truncation so sub-second timeouts work (500ms was becoming 0s)
  • to_user_error() now returns a typed error instead of None when it sees a recognized code with an unknown reason

testing

added 42 unit tests across 4 test files — all passing:

tests/test_error_default_branches.py   - 10 tests (known + unknown enum variants for all 4 error classes)
tests/test_to_user_error.py            - 15 tests (all 3 caido codes × known/unknown/missing reasons + has_authorization_error)
tests/test_graphql_auth_retry.py       - 5 tests (retry success, no retry when can't refresh, no infinite loop, non-auth errors skip retry)
tests/test_graphql_timeout.py          - 7 tests (default 30s, custom timeout, sub-second preservation, transport propagation)

ran ruff format, ruff check --select I, and ty check — no new issues

tyia!

GangGreenTemperTatum and others added 4 commits March 15, 2026 15:40
- Add default branches to match statements in CloudUserError,
  PluginUserError, StoreUserError, and ProjectUserError so new
  server-side enum variants don't leave error objects half-constructed
- Add auth retry to GraphQL client: detect authorization errors and
  refresh token + retry once, matching existing REST client behavior
- Set default 30s timeout on GraphQL HTTP and WebSocket transports
  (previously None, causing indefinite hangs on unreachable instances)
- Fix to_user_error() to preserve error classification when server
  returns recognized codes with unknown reasons, instead of silently
  discarding the error type

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
42 tests covering all four fixes:
- Error default branches: known variants still work, unknown variants
  produce valid errors instead of half-constructed objects
- to_user_error() fallthrough: recognized codes with unknown reasons
  preserve error classification, unrecognized codes still return None
- GraphQL auth retry: retries once on auth error then raises, does not
  retry on non-auth errors, does not loop infinitely
- GraphQL timeouts: default 30s applied, sub-second values not truncated,
  timeout propagated to both HTTP and WebSocket transports

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Sytten Sytten merged commit f30f2c9 into caido-community:main Apr 12, 2026
4 checks passed
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.

2 participants