fix: add HTTP error handling to federation_connect endpoint
Adds try/except around the outbound http_client.post() call to match the error-handling pattern established by the federation_sessions sibling endpoint: - httpx.HTTPStatusError → 502 with upstream status code in detail - Any other exception (ConnectError, etc.) → 503 with remote URL in detail Also adds raise_for_status() to surface non-2xx responses explicitly. Tests added: - test_federation_connect_returns_503_when_remote_unreachable - test_federation_connect_returns_502_when_remote_returns_error_status
This commit is contained in:
@@ -990,11 +990,23 @@ async def federation_connect(
|
|||||||
url = f"{remote_url}/api/sessions/{session_name}/connect"
|
url = f"{remote_url}/api/sessions/{session_name}/connect"
|
||||||
|
|
||||||
http_client: httpx.AsyncClient = request.app.state.federation_client
|
http_client: httpx.AsyncClient = request.app.state.federation_client
|
||||||
|
try:
|
||||||
resp = await http_client.post(
|
resp = await http_client.post(
|
||||||
url,
|
url,
|
||||||
headers={"Authorization": f"Bearer {remote_key}"},
|
headers={"Authorization": f"Bearer {remote_key}"},
|
||||||
)
|
)
|
||||||
|
resp.raise_for_status()
|
||||||
return resp.json()
|
return resp.json()
|
||||||
|
except httpx.HTTPStatusError as exc:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=502,
|
||||||
|
detail=f"Remote returned {exc.response.status_code}",
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=503,
|
||||||
|
detail=f"Remote unreachable: {remote_url}",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
+102
-3
@@ -1676,7 +1676,10 @@ def test_federation_connect_proxies_to_remote(client, monkeypatch, tmp_path):
|
|||||||
post_calls.append({"url": url, "kwargs": kwargs})
|
post_calls.append({"url": url, "kwargs": kwargs})
|
||||||
mock_resp = MagicMock()
|
mock_resp = MagicMock()
|
||||||
mock_resp.status_code = 200
|
mock_resp.status_code = 200
|
||||||
mock_resp.json.return_value = {"active_session": "my-session", "ttyd_port": 7682}
|
mock_resp.json.return_value = {
|
||||||
|
"active_session": "my-session",
|
||||||
|
"ttyd_port": 7682,
|
||||||
|
}
|
||||||
return mock_resp
|
return mock_resp
|
||||||
|
|
||||||
mock_fed_client = MagicMock()
|
mock_fed_client = MagicMock()
|
||||||
@@ -1705,7 +1708,9 @@ def test_federation_connect_proxies_to_remote(client, monkeypatch, tmp_path):
|
|||||||
assert data["ttyd_port"] == 7682
|
assert data["ttyd_port"] == 7682
|
||||||
|
|
||||||
|
|
||||||
def test_federation_connect_returns_404_for_invalid_remote_id(client, monkeypatch, tmp_path):
|
def test_federation_connect_returns_404_for_invalid_remote_id(
|
||||||
|
client, monkeypatch, tmp_path
|
||||||
|
):
|
||||||
"""POST /api/federation/{remote_id}/connect/{session_name} returns 404 when remote_id is out of range."""
|
"""POST /api/federation/{remote_id}/connect/{session_name} returns 404 when remote_id is out of range."""
|
||||||
import json
|
import json
|
||||||
|
|
||||||
@@ -1721,7 +1726,9 @@ def test_federation_connect_returns_404_for_invalid_remote_id(client, monkeypatc
|
|||||||
assert response.status_code == 404
|
assert response.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
def test_federation_connect_returns_404_for_non_integer_remote_id(client, monkeypatch, tmp_path):
|
def test_federation_connect_returns_404_for_non_integer_remote_id(
|
||||||
|
client, monkeypatch, tmp_path
|
||||||
|
):
|
||||||
"""POST /api/federation/{remote_id}/connect/{session_name} returns 404 when remote_id is not an integer."""
|
"""POST /api/federation/{remote_id}/connect/{session_name} returns 404 when remote_id is not an integer."""
|
||||||
import json
|
import json
|
||||||
|
|
||||||
@@ -1733,3 +1740,95 @@ def test_federation_connect_returns_404_for_non_integer_remote_id(client, monkey
|
|||||||
|
|
||||||
response = client.post("/api/federation/not-an-int/connect/my-session")
|
response = client.post("/api/federation/not-an-int/connect/my-session")
|
||||||
assert response.status_code == 404
|
assert response.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_federation_connect_returns_503_when_remote_unreachable(
|
||||||
|
client, monkeypatch, tmp_path
|
||||||
|
):
|
||||||
|
"""POST /api/federation/{remote_id}/connect/{session_name} returns 503 when remote is unreachable.
|
||||||
|
|
||||||
|
If the outbound http_client.post() raises a network-level exception (e.g. ConnectError),
|
||||||
|
the endpoint must return 503 rather than propagating a raw 500.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
|
||||||
|
import muxplex.settings as settings_mod
|
||||||
|
|
||||||
|
settings_path = tmp_path / "settings.json"
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||||
|
settings_path.write_text(
|
||||||
|
json.dumps(
|
||||||
|
{
|
||||||
|
"remote_instances": [
|
||||||
|
{
|
||||||
|
"url": "http://remote-host:8088",
|
||||||
|
"key": "secret-key-123",
|
||||||
|
"name": "remote-host",
|
||||||
|
"id": "remote-0",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
async def mock_post_unreachable(*args, **kwargs):
|
||||||
|
raise httpx.ConnectError("Connection refused")
|
||||||
|
|
||||||
|
mock_fed_client = MagicMock()
|
||||||
|
mock_fed_client.post = mock_post_unreachable
|
||||||
|
monkeypatch.setattr(client.app.state, "federation_client", mock_fed_client)
|
||||||
|
|
||||||
|
response = client.post("/api/federation/0/connect/my-session")
|
||||||
|
assert response.status_code == 503
|
||||||
|
|
||||||
|
|
||||||
|
def test_federation_connect_returns_502_when_remote_returns_error_status(
|
||||||
|
client, monkeypatch, tmp_path
|
||||||
|
):
|
||||||
|
"""POST /api/federation/{remote_id}/connect/{session_name} returns 502 when remote returns HTTP error.
|
||||||
|
|
||||||
|
If the outbound http_client.post() returns a non-2xx response that raises HTTPStatusError
|
||||||
|
(via raise_for_status), the endpoint must return 502 with the upstream status code in the detail.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
|
||||||
|
import muxplex.settings as settings_mod
|
||||||
|
|
||||||
|
settings_path = tmp_path / "settings.json"
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||||
|
settings_path.write_text(
|
||||||
|
json.dumps(
|
||||||
|
{
|
||||||
|
"remote_instances": [
|
||||||
|
{
|
||||||
|
"url": "http://remote-host:8088",
|
||||||
|
"key": "secret-key-123",
|
||||||
|
"name": "remote-host",
|
||||||
|
"id": "remote-0",
|
||||||
|
}
|
||||||
|
],
|
||||||
|
}
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
async def mock_post_error(*args, **kwargs):
|
||||||
|
mock_response = MagicMock(spec=httpx.Response)
|
||||||
|
mock_response.status_code = 502
|
||||||
|
raise httpx.HTTPStatusError(
|
||||||
|
"Bad Gateway",
|
||||||
|
request=MagicMock(),
|
||||||
|
response=mock_response,
|
||||||
|
)
|
||||||
|
|
||||||
|
mock_fed_client = MagicMock()
|
||||||
|
mock_fed_client.post = mock_post_error
|
||||||
|
monkeypatch.setattr(client.app.state, "federation_client", mock_fed_client)
|
||||||
|
|
||||||
|
response = client.post("/api/federation/0/connect/my-session")
|
||||||
|
assert response.status_code == 502
|
||||||
|
|||||||
Reference in New Issue
Block a user