Skip to content

P2-T4: Surface broker unavailability as JSON-RPC error instead of silent timeout#127

Merged
SoundBlaster merged 7 commits into
mainfrom
feature/P2-T4-broker-unavailable-error
Mar 1, 2026
Merged

P2-T4: Surface broker unavailability as JSON-RPC error instead of silent timeout#127
SoundBlaster merged 7 commits into
mainfrom
feature/P2-T4-broker-unavailable-error

Conversation

@SoundBlaster
Copy link
Copy Markdown
Owner

Description

When BrokerProxy cannot connect to the broker (timeout, spawn failure, connection refused), the client previously received no response and hung silently — showing "0 tools" or a generic connection error with no actionable message.

Fix: wrap the connect phase in run() with a try/except that catches connection failures and writes a JSON-RPC 2.0 error response (code: -32001, message: "Broker unavailable: <reason>") to stdout before returning. The client receives a meaningful error immediately instead of hanging until its own timeout fires.

Changes:

  • src/mcpbridge_wrapper/broker/proxy.py — added import json; added _send_broker_error() helper; wrapped connect phase in run() with try/except
  • tests/unit/test_broker_proxy.py — added TestBrokerProxyUnavailableError (5 tests); updated existing timeout test to verify new behaviour
  • tests/unit/test_broker_stubs.py — updated test_run_raises_timeout_when_no_sockettest_run_writes_error_when_no_socket to match new behaviour

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • CI/CD improvement

Quality Gates

  • make test - All tests pass with ≥90% coverage (732 passed, 91.59% coverage)
  • make lint - No linting errors (ruff check src/ passed)
  • make format - Code is properly formatted (ruff format --check passed)
  • make typecheck - Type checking passes
  • make doccheck - Documentation is synced with DocC (if docs changed)

Documentation Sync

  • Documentation changes are synced with DocC catalog (or N/A) — no docs changed

Testing

  • Added/updated tests for new functionality (5 new tests in TestBrokerProxyUnavailableError, 2 existing tests updated)
  • All tests pass locally (732 passed, 0 failures)
  • Manually tested the changes

Checklist

  • Code follows the project's style guidelines
  • Self-review completed
  • Comments added for complex code
  • Documentation updated (if needed)
  • No new warnings generated
  • PR title is descriptive

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@SoundBlaster SoundBlaster merged commit 45b106a into main Mar 1, 2026
10 checks passed
@SoundBlaster SoundBlaster deleted the feature/P2-T4-broker-unavailable-error branch March 1, 2026 09:05
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