fix: structured success:false envelope for session-lifecycle failures in all session-scoped tools - #205
Merged
Conversation
…n all session-scoped tools (#203) For an unknown or terminated sessionId, pause_execution and list_threads threw protocol-level McpError while sibling tools returned structured {success: false, error} results. Investigation showed the drift was wider than reported: validateSession threw a plain McpError, so the typed-error conversion branches in get_variables / get_stack_trace / get_scopes were dead code and those tools also threw; their unit tests masked it by normalizing thrown errors into the asserted shape. Align everything with the contract documented in docs/error-handling-guide.md (Pattern 2): - validateSession now throws the typed SessionNotFoundError / SessionTerminatedError (same codes and messages, both extend McpError), making the existing typed-conversion branches live. - handlePause: typed-error conversion moved ahead of the McpError re-throw (it was unreachable dead code behind it). - handleListThreads and handleGetSourceContext: same typed-conversion branch added ahead of the McpError re-throw. - Protocol-level McpError remains for invalid-parameter checks (missing sessionId etc.) and unexpected internal errors. Tests updated to pin the envelope (including removing the try/catch shims that tolerated either behavior) plus new terminated-session coverage for pause/list_threads. Docs: error-handling-guide wording tightened; tool-reference no longer lists session-not-found under -32603. Fixes #203 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #203.
For an unknown/stale
sessionId,pause_executionandlist_threadsthrew protocol-levelMcpErrorwhile sibling tools returned structured{success: false, error}results.Investigation showed the drift is wider than the issue reports:
validateSessionthrew a plainMcpError, never the typedSessionNotFoundError/SessionTerminatedError— so the typed-error conversion branches inget_variables/get_stack_trace/get_scopeswere dead code too, and those tools also threw protocol errors for unknown sessions. Their unit tests masked this with try/catch shims that normalized a thrown error into the asserted{success:false}shape.Changes
validateSessionthrows the typedSessionNotFoundError/SessionTerminatedError(same codes/messages, both extendMcpError) — the issue's suggested fix: the dead typed-error branches become live.handlePause: typed-error conversion moved ahead of theMcpErrorre-throw that made it unreachable.handleListThreads,handleGetSourceContext: same typed-conversion branch added.{success: false, error}for unknown/terminated sessions, matching the contract already documented indocs/error-handling-guide.md(Pattern 2). Protocol-levelMcpErrorremains for invalid parameters (missingsessionId) and unexpected internal errors.-32603.Verification
server-control-tools,server-coverage,server-inspection-tools): 126 passed.🤖 Generated with Claude Code