Skip to content

mcp: report an error when a sampling or elicitation handler returns nil - #1304

Merged
guglielmo-san merged 7 commits into
modelcontextprotocol:mainfrom
po-et:fix/client-nil-handler-results
Oct 16, 2026
Merged

guglielmo-san merged 7 commits into
modelcontextprotocol:mainfrom
po-et:fix/client-nil-handler-results

Conversation

@po-et

@po-et po-et commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

A sampling or elicitation handler returning (nil, nil) could panic in the client or send a null result back to the server. These handler paths report -32603 by wrapping jsonrpc2.ErrInternal, while preserving errors returned by the handler.

Tests cover both sampling handlers and form/URL elicitation. Four direct tests check the error code and sentinel; eight multi-round-trip cases check propagation on 2026-07-28 and 2025-11-25 sessions.

The branch now includes upstream main 7c486efa8223d5f66af8cf7b2ac9bb04a1ee6b91 through a normal merge. The test-file conflict was resolved by retaining both the nil-handler regression and the upstream resource-cache tests. The original production fix and nil-handler test block are unchanged; the contribution remains two files, +113/-3 relative to that base.

Validation for head 56129e92521223e712dc1e68d01678f374443117: full go test -race ./..., go vet ./..., CI-pinned staticcheck v0.6.1, formatting, and diff checks pass locally on Go 1.25.6 / macOS arm64. An independent automated review found no blocking issue and reran the nil-handler and resource-cache tests successfully. All nine public checks have now passed on this exact head, including Go 1.25/1.26 tests, race, client/server conformance, lint, and CodeQL. Local conformance scripts and other-platform local tests were not run in this update.

AI assistance was used for the merge-conflict resolution, validation, description, and independent automated review.

po-et and others added 2 commits September 27, 2026 08:28
A CreateMessageHandler, or an ElicitationHandler for a form elicitation,
that returned nil, nil made the client dereference the nil result and
crash. CreateMessageWithToolsHandler and URL elicitation sent the nil to
the server instead. Report an error naming the handler in all four
cases, as readResource does for a resource handler.
Comment thread mcp/client.go Outdated
@guglielmo-san
guglielmo-san merged commit f286395 into modelcontextprotocol:main Oct 16, 2026
9 checks passed
@guglielmo-san

Copy link
Copy Markdown
Contributor

@po-et thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants