fix: swallow the Okta SDK's None-response AttributeError as transient

[Jeff-Rowell]

Summary

The syncer's fan-out logs an unhandled AttributeError: 'NoneType' object has no attribute 'status' from the Okta SDK during upstream gateway blips. This routes that raised error through the existing transient-error handling so it is swallowed and retried on the next reconcile, matching how returned 5xx/timeout errors are already treated.
Urgency: 3 days
Expected review effort: LOW

Motivation

Production error monitoring flagged an unhandled AttributeError originating in the Okta SDK's list/get endpoints. Root cause: the SDK dereferences response.status before checking the error, so when a gateway failure returns no response object (execute() returns (None, None, error)), the SDK raises AttributeError from inside the coroutine instead of returning the error in its result tuple.

The prior change that broadened transient handling to 5xx (#544) only inspects the returned error tuple in _call, so it cannot catch a failure the SDK raises. The exception propagates out of _call, the syncer fan-out classifies it as a generic BaseException, and it is logged at ERROR level. This is the same class of transient upstream failure OktaTransientError already exists to absorb.

Description of Changes

• _call now wraps await coro and routes raised exceptions through the existing _is_transient_okta_error classifier, so a transient failure that arrives as a raised exception is surfaced as OktaTransientError (same as one returned in the tuple).
• _is_transient_okta_error recognizes the SDK's None-response AttributeError signature (narrowly matched by message, so unrelated AttributeErrors still propagate).
• One classifier drives both the returned-error and raised-error paths.

Validation of Changes

• Added a failing test reproducing the raised AttributeError, confirmed it propagated unhandled before the fix, and that it now surfaces as OktaTransientError.
• Added a guard test that an unrelated AttributeError still propagates.
• okta_service, transient-error, sync (membership/ownership/group/user), and fan-out suites pass; ruff and ty clean on changed files.

Guidance for Reviewers

• The narrow message match in _is_transient_okta_error is the main judgment call: a None-response dereference in these SDK endpoints only happens on the transport/error path, so treating it as transient is safe for the read-only listing the syncer reconciles later. Confirm you are comfortable matching on the exception message rather than something more structural.

[somethingnew2-0]

Working this out with Claude, this Attribute error is not Transient and shows up on every staging run since #543 was deployed to staging

[somethingnew2-0]

Sharing findings from investigating the same AttributeError in the syncer — it overlaps with this PR, so flagging for coordination.

Root cause confirmed, plus a transport-layer detail. Agreed the SDK dereferences response.status before checking error in the list_*/get_* endpoints, and it only fires when execute() returns a None response. Tracing HTTPClient.send_request, that None response comes exclusively from its except (aiohttp.ClientError, asyncio.TimeoutError) branch — a pure transport-layer failure (dropped/reset connection, or timeout), before any body is read. It reproduced for us on a group with zero members — a single request, no pagination — which rules out response size/content and confirms it's the transport layer, not the data.

Why it surfaces now — the amplifier. In our case it correlates with running the syncer on a pooled client. OktaClient.__aenter__ creates one aiohttp.ClientSession reused for the whole run; a long-lived pooled keep-alive gets idle-closed server-side (by the LB) and then reused stale → ServerDisconnectedError (an aiohttp.ClientError) → the None-response path → this AttributeError. A fresh session per call rarely hits it. So this is inherent to pooling against a load balancer, and this PR correctly makes the residual benign.

Complement — a bounded retry (#547). This PR swallows and defers to the next reconcile. Since these are idempotent reads and an immediate retry gets a fresh connection, a single retry usually recovers within the same run instead of skipping the resource for a cycle. The SDK won't do it (it bails immediately on the null response and only retries HTTP statuses, not connection-launch failures). I put this in _WrapperClient in #547 — it retries OktaTransientError once and composes directly on top of your reclassification (your change is what turns the None-response case into an OktaTransientError for the retry to catch). Happy to fold it into this PR instead if you'd rather keep it in one place.

Connector-level hygiene — evaluated, not worth it. I checked whether tuning the pooled connector could prevent the stale reuse: enable_cleanup_closed is deprecated in aiohttp 3.14 (a no-op on modern Python's fixed SSL shutdown), and lowering keepalive_timeout enough to avoid stale-idle reuse would drop connections across the syncer's idle gaps, negating the pooling benefit. So the retry is the right lever, not connector settings.

Net: this PR (swallow) + #547 (retry) covers it well — nice work getting the reclassification in.

🤖 Posted by Claude Code

[somethingnew2-0]

Upstream fix submitted okta/okta-sdk-python#564