fix(mcp): report a clear error when the OAuth callback port is in use
_wait_for_callback catches OSError on bind with a comment claiming the port is held by a server build_oauth_auth started, and promising to fall back to polling it. build_oauth_auth never starts a callback server (this is the only listener), so there is nothing to poll: the branch just raised a misleading "OAuth callback timed out" when the real cause is a busy port (a concurrent login, a leftover listener, or a fixed oauth.redirect_port that collided). Fix the stale comment and raise an accurate, actionable message (names the port, suggests freeing it or setting a free oauth.redirect_port), chained from the original OSError. Behavior is otherwise unchanged. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bda8bd76a8
commit
454d553d34
2 changed files with 29 additions and 6 deletions
|
|
@ -1470,3 +1470,22 @@ class TestPoisonClientRegistration:
|
|||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
storage = HermesTokenStorage("srv")
|
||||
assert storage.poison_client_registration() is False
|
||||
|
||||
|
||||
def test_wait_for_callback_port_in_use_reports_clear_error(monkeypatch):
|
||||
"""A busy loopback callback port surfaces a clear 'already in use' error,
|
||||
not a misleading 'timed out'. Guards the stale-comment fix where the branch
|
||||
also wrongly claimed build_oauth_auth had started a server to poll."""
|
||||
import tools.mcp_oauth as mo
|
||||
|
||||
monkeypatch.setattr(mo, "_is_interactive", lambda: True)
|
||||
with patch.object(mo, "_oauth_port", 54321), patch.object(
|
||||
mo, "HTTPServer", side_effect=OSError("address already in use")
|
||||
):
|
||||
with pytest.raises(mo.OAuthNonInteractiveError) as excinfo:
|
||||
asyncio.run(mo._wait_for_callback())
|
||||
|
||||
msg = str(excinfo.value)
|
||||
assert "54321" in msg
|
||||
assert "already in use" in msg
|
||||
assert "timed out" not in msg
|
||||
|
|
|
|||
|
|
@ -736,13 +736,17 @@ async def _wait_for_callback() -> tuple[str, str | None]:
|
|||
server.allow_reuse_address = True
|
||||
server.server_bind()
|
||||
server.server_activate()
|
||||
except OSError:
|
||||
# Port already in use — the server from build_oauth_auth is running.
|
||||
# Fall back to polling the server started by build_oauth_auth.
|
||||
except OSError as exc:
|
||||
# The loopback callback port is genuinely in use: a concurrent OAuth
|
||||
# flow, a leftover listener, or a fixed `oauth.redirect_port` that
|
||||
# collided. build_oauth_auth does not start its own callback server,
|
||||
# so there is nothing to poll here; surface a clear, actionable error
|
||||
# instead of a misleading "timed out".
|
||||
raise OAuthNonInteractiveError(
|
||||
"OAuth callback timed out — could not bind callback port. "
|
||||
"Complete the authorization in a browser first, then retry."
|
||||
)
|
||||
f"OAuth callback port {_oauth_port} is already in use ({exc}). "
|
||||
"Close any other in-progress login, or set a free `oauth.redirect_port` "
|
||||
"in the server config, then retry."
|
||||
) from exc
|
||||
|
||||
server_thread = threading.Thread(target=server.handle_request, daemon=True)
|
||||
server_thread.start()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue