mirror of
https://github.com/trustgraph-ai/trustgraph.git
synced 2026-04-25 00:16:23 +02:00
Fix websocket error responses in Mux dispatcher (#726)
Error responses from the websocket multiplexer were missing the
request ID and using a bare string format instead of the structured
error protocol. This caused clients to hang when a request failed
(e.g. unsupported service for a flow) because the error could not
be routed to the waiting caller.
Include request ID in all error paths, use structured error format
({message, type}) with complete flag, and extract the ID early in
receive() so even malformed requests get a routable error when
possible.
Updated tests - tests were coded against invalid protocol messages
This commit is contained in:
parent
ea33620fb2
commit
a634520509
2 changed files with 48 additions and 17 deletions
|
|
@ -121,7 +121,11 @@ class TestMux:
|
||||||
# Based on the code, it seems to catch exceptions
|
# Based on the code, it seems to catch exceptions
|
||||||
await mux.receive(mock_msg)
|
await mux.receive(mock_msg)
|
||||||
|
|
||||||
mock_ws.send_json.assert_called_once_with({"error": "Bad message"})
|
mock_ws.send_json.assert_called_once_with({
|
||||||
|
"error": {"message": "Bad message", "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
"id": "test-id-123",
|
||||||
|
})
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_mux_receive_message_without_id(self):
|
async def test_mux_receive_message_without_id(self):
|
||||||
|
|
@ -145,7 +149,10 @@ class TestMux:
|
||||||
# receive method should handle the RuntimeError internally
|
# receive method should handle the RuntimeError internally
|
||||||
await mux.receive(mock_msg)
|
await mux.receive(mock_msg)
|
||||||
|
|
||||||
mock_ws.send_json.assert_called_once_with({"error": "Bad message"})
|
mock_ws.send_json.assert_called_once_with({
|
||||||
|
"error": {"message": "Bad message", "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
})
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_mux_receive_invalid_json(self):
|
async def test_mux_receive_invalid_json(self):
|
||||||
|
|
@ -168,4 +175,7 @@ class TestMux:
|
||||||
await mux.receive(mock_msg)
|
await mux.receive(mock_msg)
|
||||||
|
|
||||||
mock_msg.json.assert_called_once()
|
mock_msg.json.assert_called_once()
|
||||||
mock_ws.send_json.assert_called_once_with({"error": "Invalid JSON"})
|
mock_ws.send_json.assert_called_once_with({
|
||||||
|
"error": {"message": "Invalid JSON", "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
})
|
||||||
|
|
@ -33,9 +33,12 @@ class Mux:
|
||||||
|
|
||||||
async def receive(self, msg):
|
async def receive(self, msg):
|
||||||
|
|
||||||
|
request_id = None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|
||||||
data = msg.json()
|
data = msg.json()
|
||||||
|
request_id = data.get("id")
|
||||||
|
|
||||||
if "request" not in data:
|
if "request" not in data:
|
||||||
raise RuntimeError("Bad message")
|
raise RuntimeError("Bad message")
|
||||||
|
|
@ -51,7 +54,13 @@ class Mux:
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Receive exception: {str(e)}", exc_info=True)
|
logger.error(f"Receive exception: {str(e)}", exc_info=True)
|
||||||
await self.ws.send_json({"error": str(e)})
|
error_resp = {
|
||||||
|
"error": {"message": str(e), "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
}
|
||||||
|
if request_id:
|
||||||
|
error_resp["id"] = request_id
|
||||||
|
await self.ws.send_json(error_resp)
|
||||||
|
|
||||||
async def maybe_tidy_workers(self, workers):
|
async def maybe_tidy_workers(self, workers):
|
||||||
|
|
||||||
|
|
@ -97,12 +106,12 @@ class Mux:
|
||||||
})
|
})
|
||||||
|
|
||||||
worker = asyncio.create_task(
|
worker = asyncio.create_task(
|
||||||
self.request_task(request, responder, flow, svc)
|
self.request_task(id, request, responder, flow, svc)
|
||||||
)
|
)
|
||||||
|
|
||||||
workers.append(worker)
|
workers.append(worker)
|
||||||
|
|
||||||
async def request_task(self, request, responder, flow, svc):
|
async def request_task(self, id, request, responder, flow, svc):
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|
||||||
|
|
@ -119,7 +128,11 @@ class Mux:
|
||||||
)
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
await self.ws.send_json({"error": str(e)})
|
await self.ws.send_json({
|
||||||
|
"id": id,
|
||||||
|
"error": {"message": str(e), "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
})
|
||||||
|
|
||||||
async def run(self):
|
async def run(self):
|
||||||
|
|
||||||
|
|
@ -143,7 +156,11 @@ class Mux:
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
# This is an internal working error, may not be recoverable
|
# This is an internal working error, may not be recoverable
|
||||||
logger.error(f"Run prepare exception: {e}", exc_info=True)
|
logger.error(f"Run prepare exception: {e}", exc_info=True)
|
||||||
await self.ws.send_json({"id": id, "error": str(e)})
|
await self.ws.send_json({
|
||||||
|
"id": id,
|
||||||
|
"error": {"message": str(e), "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
})
|
||||||
self.running.stop()
|
self.running.stop()
|
||||||
|
|
||||||
if self.ws:
|
if self.ws:
|
||||||
|
|
@ -160,7 +177,11 @@ class Mux:
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Exception in mux: {e}", exc_info=True)
|
logger.error(f"Exception in mux: {e}", exc_info=True)
|
||||||
await self.ws.send_json({"error": str(e)})
|
await self.ws.send_json({
|
||||||
|
"id": id,
|
||||||
|
"error": {"message": str(e), "type": "error"},
|
||||||
|
"complete": True,
|
||||||
|
})
|
||||||
|
|
||||||
self.running.stop()
|
self.running.stop()
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue