fix(proxy): report a failed DAP request's body.error.format instead of a bare "Request failed" (#663) - #666
Merged
Merged
Conversation
…f a bare "Request failed" (#663) js-debug sends its user-facing errors as a ProtocolError — {success:false, body:{error:{id, format, …}}} with no top-level message — and MinimalDapClient rejected a failed response with new Error(response.message || 'Request failed'). So a ReferenceError in evaluate_expression, a bad frame id, or any other js-debug user error reached the agent as "Request failed", with the real text visible only in the adapter's telemetry line in the proxy log. Per the DAP spec, ErrorResponse.message is the raw short form a client may leave unset and body.error is the user-facing Message ({name} placeholders filled from variables). dapResponseErrorMessage / dapResponseErrorText (src/proxy/dap-response-error.ts) read message, then the formatted body.error, then the generic fallback; the client's rejection, its response log line (issue #519's errorMessage field), and the expose_session mirror's forwarded error all use it. Child sessions share MinimalDapClient, so js-debug's adopted target is covered too. Closes #663 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DP1W22KJbkHZi4ueAtMFPF
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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
1 participant
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.
Summary
Closes #663. Found while dogfooding #658 with mcp-debugger attached to a live mcp-debugger server.
evaluate_expressionon an identifier not visible from the paused frame returned{"success":false,"error":"Request failed"}while the proxy log's telemetry line carried the real text,Uncaught ReferenceError: httpSessions is not defined.Why
js-debug sends its user-facing errors as a
ProtocolError:MinimalDapClientrejected a failed response withnew Error(response.message || 'Request failed'), so every js-debug user error (ReferenceError/SyntaxError inevaluate, a bad frame id,setVariableon a read-only binding, …) reached the agent as the generic fallback. Per the DAP spec,ErrorResponse.messageis the raw short form a client may leave unset;body.erroris the user-facingMessage(formatwith{name}placeholders filled fromvariables).Fix
src/proxy/dap-response-error.ts:dapResponseErrorText/dapResponseErrorMessagereadmessage, then the formattedbody.error, then the generic fallback;formatDapMessagesubstitutes placeholders and leaves unknown ones intact.MinimalDapClient: the rejection and theDAP message: responselog line (Proxy routing decisions are invisible in the per-session proxy log (module loggers write to a per-pid file; responses logged without success flag) #519'serrorMessage) use it. Child sessions share the class, so js-debug's adopted target is covered.DapMirrorServer: the forwarded error response toexpose_sessionIDE clients carries the same text.Tests
tests/unit/proxy/dap-response-error.test.ts: precedence, js-debug shape, empty/malformed fallbacks, placeholder substitution.minimal-dap.test.ts: rejection withbody.error.formatonly, placeholder filling, and the log line'serrorMessage.dap-mirror-server.test.ts: a forwarded failure with onlybody.errorreaches the mirror client asmessage.Full pre-push suite green (4935 tests); ratchet unchanged; fragment
changelog.d/663.fixed.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01DP1W22KJbkHZi4ueAtMFPF