Repository navigation
fix(api): preserve zero JSON-RPC request id in MCP error responses - #42997
Conversation
`create_mcp_error_response` used `request_id or 1`, which rewrites a legal JSON-RPC 2.0 numeric id of 0 to 1. Per the JSON-RPC 2.0 spec the response id MUST echo the request id, so a client that sent id=0 can never match the error response. Only `None` (notification-style request) should fall back to the default id of 1. Adds a regression test for the zero-id case; all existing `TestCreateMCPErrorResponse` cases still pass.
github-actions
Bot
added
the
size:S
ghfind-review
Bot
added
the
review: low
|
Thank you for your contribution! I’ve approved the pending CI workflows so the automated checks can validate this update. Please keep an eye on the results and address any failures that need changes. We appreciate your help improving Dify. |
Pyrefly Type Coverage
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42997 +/- ##
==========================================
- Coverage 88.05% 88.05% -0.01%
==========================================
Files 5151 5151
Lines 323364 323364
Branches 65169 65169
==========================================
- Hits 284729 284722 -7
- Misses 33251 33259 +8
+ Partials 5384 5383 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for approving the CI workflows! All required checks now pass: API unit/integration/E2E/VDB tests, pyrefly type checks, style checks, and codecov/patch are all green. The change keeps the MCP error response from omitting the request_id when it is explicitly zero (0), which was causing downstream clients to fail correlation. Ready for review whenever you have a chance. |
create_mcp_error_response(api/core/mcp/utils.py) built the error response id withrequest_id or 1. Under JSON-RPC 2.0 a numeric id of0is perfectly legal — onlynullis reserved for notifications — and the response object MUST echo the same id as the request so the client can correlate it. Withor 1, a request carryingid: 0gets an error response withid: 1, which the client can never match.This keeps the existing default of
1forNone(notification-style) and preserves a genuine0.api/core/mcp/utils.py:request_id if request_id is not None else 1api/tests/unit_tests/core/mcp/test_utils.py: regression test for the zero-id caseVerified locally: with the patch,
create_mcp_error_response(0, ...)yieldsid == 0;Nonestill yields1; string and non-zero numeric ids are unchanged.