Skip to content

fix(#3109): catch httpx.HTTPStatusError in Watsonx model discovery - #3111

Merged
tisnik merged 1 commit into
mainfrom
agent/3109-watsonx-iam-discovery-error
Nov 3, 2026
Merged

tisnik merged 1 commit into
mainfrom
agent/3109-watsonx-iam-discovery-error

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Description

Catch httpx.HTTPStatusError in WatsonxInferenceProvider._discover_models so that IBM IAM token exchange failures are wrapped in ModelDiscoveryError like all other provider discovery errors.

WatsonxIAMAuth._exchange_token() calls response.raise_for_status() on the IAM endpoint response. When IAM rejects the API key (HTTP 401/403) or is unreachable (HTTP 5xx), this raises httpx.HTTPStatusError, which is not a subclass of openai.APIError and was escaping the existing except APIError handler.

The fix widens the except clause to except (APIError, httpx.HTTPStatusError) as exc.

Type of change

  • Bug fix

Tools used to create PR

  • Generated by: fullsend-code

Related Tickets & Documents

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Testing

Added a parametrized httpx.HTTPStatusError test case to the existing test_discover_models_wraps_api_error test. The test mocks models.list to raise httpx.HTTPStatusError (simulating an IAM 401) and asserts that ModelDiscoveryError is raised with the original exception chained as __cause__.

All 13 tests in tests/unit/providers/inference/watsonx/test_provider.py pass.


Closes #3109

Post-script verification

  • Branch is not main/master (agent/3109-watsonx-iam-discovery-error)
  • Secret scan passed (gitleaks — 0b7d2138a96689f06ac4330cf28069f122ff1fbe..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Final PR description secret scan passed (gitleaks — no-git)

WatsonxIAMAuth.raise_for_status() raises httpx.HTTPStatusError on
IAM token exchange failures (401/403/5xx), which is not a subclass
of openai.APIError. This escaped the except clause in
_discover_models, propagating unwrapped instead of as
ModelDiscoveryError.

Widen the except tuple to (APIError, httpx.HTTPStatusError) so IAM
failures are wrapped consistently with other discovery errors.

Add a parametrized test case for httpx.HTTPStatusError to the
existing test_discover_models_wraps_api_error test.

Closes #3109
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Nov 2, 2026
@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@fullsend-ai-review

fullsend-ai-review Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:04 PM UTC · Completed 2:18 PM UTC

Commit: 6636d77 · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.76

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [missing-test] tests/unit/providers/inference/watsonx/test_provider.py:148 — The new httpx.HTTPStatusError test case injects the error only via models.list while stubbing _list_foundation_model_functions to succeed. The except clause covers both coroutines in asyncio.gather, but no test verifies the error path when _list_foundation_model_functions raises httpx.HTTPStatusError from the IAM auth layer.

  • [edge-case] src/providers/inference/watsonx/provider.py:67 — The except (APIError, httpx.HTTPStatusError) handler does not catch raw httpx transport errors (httpx.ConnectError, httpx.TimeoutException). However, these would only be unhandled if they escape the openai SDK's own error-wrapping layer, which normally converts them to APIConnectionError/APITimeoutError (both subclasses of APIError). The practical risk is low and unconfirmed.


Risk assessment missing — risk assessment is enabled but no score was
produced. A human reviewer must evaluate this PR.


Labels: Bug fix PR addressing an exception handling gap in the Watsonx provider.

@fullsend-ai-review fullsend-ai-review 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.

Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:

  • tests/unit/providers/inference/watsonx/test_provider.py (file-level): Line 148 ·
    [low] missing-test

The new httpx.HTTPStatusError test case injects the error only via models.list while stubbing _list_foundation_model_functions to succeed. The except clause covers both coroutines in asyncio.gather, but no test verifies the error path when _list_foundation_model_functions raises httpx.HTTPStatusError from the IAM auth layer.

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

self._list_foundation_model_functions(),
)
except APIError as exc:
except (APIError, httpx.HTTPStatusError) as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] edge-case

The except (APIError, httpx.HTTPStatusError) handler does not catch raw httpx transport errors (httpx.ConnectError, httpx.TimeoutException). However, these would only be unhandled if they escape the openai SDK's own error-wrapping layer, which normally converts them to APIConnectionError/APITimeoutError (both subclasses of APIError). The practical risk is low and unconfirmed.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment bug Something isn't working labels Nov 2, 2026
@tisnik
tisnik requested a review from asimurka November 2, 2026 15:52

@asimurka asimurka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tisnik tisnik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tisnik
tisnik merged commit 0537a8d into main Nov 3, 2026
138 of 144 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:07 AM UTC · Completed 9:14 AM UTC

Commit: 6636d77 · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.89

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #3111 — catch httpx.HTTPStatusError in Watsonx model discovery

Timeline

  1. Issue Watsonx IAM token refresh failures escape ModelDiscoveryError wrapping #3109 created by the retro agent analyzing PR LCORE-4710: Catch provider model discovery failures #3102 (2026-11-02 13:39 UTC). Identified that httpx.HTTPStatusError from WatsonxIAMAuth._exchange_token() escapes the except APIError handler in _discover_models.
  2. Triage confirmed the bug via code inspection in 4 minutes ($1.11). Recommended the specific fix.
  3. Code agent implemented the fix correctly on the first try in 11 minutes ($1.51) — 2 files, +8/−2 lines, with a parametrized test case.
  4. Review agent (run 41457315726) completed in 14 minutes ($3.76). Produced 2 low-severity, non-actionable findings.
  5. Human reviewers asimurka and tisnik both approved with "LGTM"; merged ~19 hours later.

What went well

  • Full pipeline success: retro agent identified a real bug in PR LCORE-4710: Catch provider model discovery failures #3102 → triage confirmed → code implemented → review validated → humans approved. Zero rework iterations.
  • Code quality: the code agent's fix was precise and correct, matching the triage agent's recommendation exactly.
  • Review findings were reasonable: the missing-test and edge-case observations were technically sound, even if non-actionable.

Model unavailability degraded review coverage

4 of 7 sonnet-tier review sub-agents (intent-coherence, style-conventions, docs-currency, risk-assessment) failed because both claude-sonnet-5-5 and fallback claude-sonnet-4-6 were not served on this project's Vertex AI endpoint (firefly-868139). The risk-assessment failure triggered the requires-manual-review label, forcing human intervention on a trivially correct 10-line bug fix. The review output noted the missing risk assessment but did not surface that three other review dimensions were also skipped.

This is a well-tracked problem across multiple open issues — this retro adds PR #3111 as another data point:

Autonomy assessment

Inconclusive. Both human reviewers approved without substantive findings, so the delta between agent and human review cannot be meaningfully computed for this PR.

No new proposals

All improvement opportunities identified in this retro are already tracked by existing open issues. No new proposals are warranted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Watsonx IAM token refresh failures escape ModelDiscoveryError wrapping

2 participants