Skip to content

Commit 1adba1d

Browse files
author
vijay
committed
Report Skills handler-output faults as INTERNAL_ERROR, not INVALID_PARAMS
A non-conformant result from a `list_skills`/`get_skill`/`read_directory` handler (or a `get_skill` URI mismatch) is a server-side bug, not a bad caller request, so -32603 is the correct code rather than -32602. Log the real cause server-side and return a generic message, mirroring the runner's existing handling of invalid handler results. Input validation (`_require_skill_md_uri`, `_require_directory_uri`, params) stays -32602.
1 parent 2f363d5 commit 1adba1d

2 files changed

Lines changed: 27 additions & 24 deletions

File tree

‎src/mcp/server/skills.py‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ async def get_skill(ctx, params):
2828

2929
from __future__ import annotations
3030

31+
import logging
3132
from collections.abc import Awaitable, Callable, Sequence
3233
from typing import Any
3334

34-
from mcp_types.jsonrpc import INVALID_PARAMS
35+
from mcp_types.jsonrpc import INTERNAL_ERROR, INVALID_PARAMS
3536
from mcp_types.version import MODERN_PROTOCOL_VERSIONS
3637

3738
from mcp.server.context import HandlerResult, ServerRequestContext
@@ -57,6 +58,8 @@ async def get_skill(ctx, params):
5758

5859
__all__ = ["Skills"]
5960

61+
logger = logging.getLogger(__name__)
62+
6063
ListSkillsHandler = Callable[[ServerRequestContext[Any, Any], ListSkillsParams], Awaitable[ListSkillsResult]]
6164
GetSkillHandler = Callable[[ServerRequestContext[Any, Any], GetSkillParams], Awaitable[GetSkillResult]]
6265
ReadDirectoryHandler = Callable[[ServerRequestContext[Any, Any], ReadDirectoryParams], Awaitable[ReadDirectoryResult]]
@@ -102,24 +105,22 @@ async def _handle_list(self, ctx: ServerRequestContext[Any, Any], params: ListSk
102105
result = await self._list_skills(ctx, params)
103106
try:
104107
validate_list_result(result)
105-
except ValueError as exc:
106-
raise MCPError(
107-
code=INVALID_PARAMS, message=f"list_skills handler returned an invalid result: {exc}"
108-
) from exc
108+
except ValueError:
109+
logger.exception("list_skills handler returned an invalid result")
110+
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
109111
return _finalize_cacheable(result, ctx.protocol_version)
110112

111113
async def _handle_get(self, ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> HandlerResult:
112114
_require_skill_md_uri(params.uri)
113115
result = await self._get_skill(ctx, params)
114116
if result.skill.uri != params.uri:
115-
raise MCPError(
116-
code=INVALID_PARAMS,
117-
message=f"get_skill handler returned {result.skill.uri!r} for requested {params.uri!r}",
118-
)
117+
logger.error("get_skill handler returned %r for requested %r", result.skill.uri, params.uri)
118+
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result")
119119
try:
120120
validate_skill(result.skill)
121-
except ValueError as exc:
122-
raise MCPError(code=INVALID_PARAMS, message=f"get_skill handler returned an invalid result: {exc}") from exc
121+
except ValueError:
122+
logger.exception("get_skill handler returned an invalid result")
123+
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
123124
return result
124125

125126
async def _handle_read_directory(
@@ -130,10 +131,9 @@ async def _handle_read_directory(
130131
result = await self._read_directory(ctx, params)
131132
try:
132133
validate_directory_result(params.uri, result)
133-
except ValueError as exc:
134-
raise MCPError(
135-
code=INVALID_PARAMS, message=f"read_directory handler returned an invalid result: {exc}"
136-
) from exc
134+
except ValueError:
135+
logger.exception("read_directory handler returned an invalid result")
136+
raise MCPError(code=INTERNAL_ERROR, message="Handler returned an invalid result") from None
137137
return result
138138

139139

‎tests/server/test_skills.py‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import pytest
1313
from inline_snapshot import snapshot
14-
from mcp_types import INVALID_PARAMS, METHOD_NOT_FOUND, Resource
14+
from mcp_types import INTERNAL_ERROR, INVALID_PARAMS, METHOD_NOT_FOUND, Resource
1515
from pydantic import BaseModel, ConfigDict
1616

1717
from mcp.client.client import Client
@@ -135,8 +135,8 @@ async def private_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsPa
135135

136136
async def test_skills_list_rejects_a_handler_result_with_an_invalid_skill() -> None:
137137
"""SDK-defined: a `list_skills` handler bug (a non-conformant skill entry) is caught
138-
before it reaches the client, as an Invalid params error rather than a silently
139-
non-conformant listing."""
138+
before it reaches the client, as an Internal error — the fault is the server's, not the
139+
caller's params — rather than a silently non-conformant listing."""
140140

141141
async def bad_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult:
142142
return ListSkillsResult(
@@ -147,7 +147,7 @@ async def bad_list(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams
147147
async with Client(server) as client:
148148
with pytest.raises(MCPError) as exc_info:
149149
await client.session.send_request(ListSkillsRequest(), ListSkillsResult)
150-
assert exc_info.value.code == INVALID_PARAMS
150+
assert exc_info.value.code == INTERNAL_ERROR
151151

152152

153153
async def test_skills_get_returns_the_matching_skill() -> None:
@@ -196,7 +196,7 @@ async def test_skills_get_rejects_a_uri_that_does_not_end_in_skill_md() -> None:
196196

197197
async def test_skills_get_rejects_a_handler_that_returns_a_mismatched_uri() -> None:
198198
"""SDK-defined: a `get_skill` handler bug (returning the wrong skill) is caught before
199-
it reaches the client, as an Invalid params error rather than a silently wrong answer."""
199+
it reaches the client, as an Internal error rather than a silently wrong answer."""
200200

201201
async def wrong_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult:
202202
return GetSkillResult(skill=_git_workflow_skill())
@@ -207,12 +207,12 @@ async def wrong_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParam
207207
await client.session.send_request(
208208
GetSkillRequest(params=GetSkillParams(uri="skill://other/SKILL.md")), GetSkillResult
209209
)
210-
assert exc_info.value.code == INVALID_PARAMS
210+
assert exc_info.value.code == INTERNAL_ERROR
211211

212212

213213
async def test_skills_get_rejects_a_matching_but_non_conformant_skill() -> None:
214214
"""SDK-defined: a `get_skill` handler bug (a non-conformant skill body, distinct from a
215-
URI mismatch) is caught the same way, as an Invalid params error."""
215+
URI mismatch) is caught the same way, as an Internal error."""
216216

217217
async def bad_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams) -> GetSkillResult:
218218
return GetSkillResult(
@@ -223,7 +223,7 @@ async def bad_skill(ctx: ServerRequestContext[Any, Any], params: GetSkillParams)
223223
async with Client(server) as client:
224224
with pytest.raises(MCPError) as exc_info:
225225
await client.session.send_request(GetSkillRequest(params=GetSkillParams(uri=_SKILL_URI)), GetSkillResult)
226-
assert exc_info.value.code == INVALID_PARAMS
226+
assert exc_info.value.code == INTERNAL_ERROR
227227

228228

229229
async def test_missing_uri_param_is_rejected_before_the_handler_runs() -> None:
@@ -277,6 +277,9 @@ async def test_directory_read_rejects_a_trailing_slash_uri() -> None:
277277

278278

279279
async def test_directory_read_rejects_a_handler_result_with_a_grandchild() -> None:
280+
"""SDK-defined: a `read_directory` handler bug (a non-direct child) is a server fault,
281+
surfaced as an Internal error, not the caller's Invalid params."""
282+
280283
async def bad_directory(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult:
281284
return ReadDirectoryResult(resources=[Resource(uri="skill://git-workflow/a/b/c.md", name="c.md")])
282285

@@ -288,4 +291,4 @@ async def bad_directory(ctx: ServerRequestContext[Any, Any], params: ReadDirecto
288291
await client.session.send_request(
289292
ReadDirectoryRequest(params=ReadDirectoryParams(uri="skill://git-workflow")), ReadDirectoryResult
290293
)
291-
assert exc_info.value.code == INVALID_PARAMS
294+
assert exc_info.value.code == INTERNAL_ERROR

0 commit comments

Comments
 (0)