fix(proxy): report a failed DAP request's body.error.format instead of a bare "Request failed" (#663)

changelog.d/663.fixed.md

@@ -0,0 +1 @@
+**A failed DAP request reports the adapter's reason instead of "Request failed"** — js-debug sends its user-facing errors as `{success: false, body: {error: {format: …}}}` with no top-level `message`, and the DAP client read only `message`, so a ReferenceError in `evaluate_expression`, a bad frame id, or any other js-debug `ProtocolError` reached the agent as a bare `Request failed` (the text was visible only in the adapter's telemetry line in the proxy log). The rejection, the proxy log's response line, and the `expose_session` mirror's forwarded error now fall back to `body.error.format` with its `{placeholders}` filled from `variables`, per the DAP spec's split between the raw short-form `message` and the user-facing `body.error` (#663)

src/proxy/dap-mirror-server.ts

@@ -17,6 +17,7 @@ import net from 'net';
 import crypto from 'crypto';
 import { DebugProtocol } from '@vscode/debugprotocol';
 import { DapFrameDecoder, encodeDapMessage } from './dap-framing.js';
+import { dapResponseErrorText } from './dap-response-error.js';
 import type { ILogger } from './dap-proxy-interfaces.js';
 
 // ===== Host interface (implemented by DapProxyWorker) =====
@@ -286,7 +287,12 @@ export class MirrorClientConnection {
     if (FORWARDED_REQUESTS.has(command)) {
       try {
         const response = await this.host.forwardRequest(command, request.arguments);
-        this.sendResponse(request, response?.body ?? {}, response?.success !== false, response?.message);
+        this.sendResponse(
+          request,
+          response?.body ?? {},
+          response?.success !== false,
+          response ? dapResponseErrorText(response) : undefined
+        );
       } catch (error) {
         const message = error instanceof Error ? error.message : String(error);
         this.sendErrorResponse(request, MIRROR_ERROR_READ_ONLY, message, false);

src/proxy/dap-response-error.ts

@@ -0,0 +1,43 @@
+/**
+ * The human-readable reason a DAP response failed (issue #663).
+ *
+ * DAP's ErrorResponse carries two texts: `message`, "the raw error in short
+ * form", and `body.error`, a structured Message whose `format` (with `{name}`
+ * placeholders filled from `variables`) is what the user is meant to see.
+ * Adapters are free to set either. js-debug sends a ProtocolError with only
+ * `body.error` — `{success:false, body:{error:{id, format:"Uncaught
+ * ReferenceError: x is not defined"}}}` — so reading `message` alone turned
+ * every one of its user-facing evaluate/variables/frame errors into a bare
+ * "Request failed".
+ */
+import type { DebugProtocol } from '@vscode/debugprotocol';
+
+export const DEFAULT_DAP_FAILURE_MESSAGE = 'Request failed';
+
+/** Fill `{name}` placeholders in a DAP Message format from its variables. */
+export function formatDapMessage(message: DebugProtocol.Message): string {
+  const variables = message.variables ?? {};
+  return message.format.replace(/\{([^{}]+)\}/g, (placeholder, name: string) =>
+    Object.prototype.hasOwnProperty.call(variables, name) ? variables[name] : placeholder
+  );
+}
+
+/**
+ * `message` when the adapter set it, else the formatted `body.error`, else
+ * undefined. Callers add their own fallback.
+ */
+export function dapResponseErrorText(response: DebugProtocol.Response): string | undefined {
+  if (response.message) {
+    return response.message;
+  }
+  const error = (response.body as DebugProtocol.ErrorResponse['body'] | undefined)?.error;
+  if (error && typeof error.format === 'string' && error.format !== '') {
+    return formatDapMessage(error);
+  }
+  return undefined;
+}
+
+/** The reason a failed response gives, or the generic fallback. */
+export function dapResponseErrorMessage(response: DebugProtocol.Response): string {
+  return dapResponseErrorText(response) ?? DEFAULT_DAP_FAILURE_MESSAGE;
+}

src/proxy/minimal-dap.ts

@@ -6,6 +6,7 @@
 import net, { Socket } from 'net';
 import { EventEmitter } from 'events';
 import { DebugProtocol } from '@vscode/debugprotocol';
+import { DEFAULT_DAP_FAILURE_MESSAGE, dapResponseErrorMessage, dapResponseErrorText } from './dap-response-error.js';
 import { createLogger } from '../utils/logger.js';
 import fs from 'fs';
 import path from 'path';
@@ -221,8 +222,11 @@ export class MinimalDapClient extends EventEmitter {
       const resp = message as DebugProtocol.Response;
       debugInfo.success = resp.success;
       debugInfo.request_seq = resp.request_seq;
-      if (resp.success === false && resp.message) {
-        debugInfo.errorMessage = resp.message;
+      if (resp.success === false) {
+        const errorMessage = dapResponseErrorText(resp);
+        if (errorMessage) {
+          debugInfo.errorMessage = errorMessage;
+        }
       }
     }
 
@@ -263,7 +267,7 @@ export class MinimalDapClient extends EventEmitter {
         if (response.success) {
           pending.resolve(response);
         } else {
-          pending.reject(new Error(response.message || 'Request failed'));
+          pending.reject(new Error(dapResponseErrorMessage(response)));
         }
       } else {
         if (this.isDisconnectingOrDisconnected) {
@@ -906,7 +910,7 @@ export class MinimalDapClient extends EventEmitter {
       request_seq: request.seq,
       command: request.command,
       success,
-      ...(success ? { body } : { message: errorMessage || 'Request failed' })
+      ...(success ? { body } : { message: errorMessage || DEFAULT_DAP_FAILURE_MESSAGE })
     };
     this.writeMessage(response);
   }

tests/proxy/dap-mirror-server.test.ts

@@ -448,6 +448,26 @@ describe('DapMirrorServer', () => {
       expect(response?.message).toContain('not available while running');
     });
 
+    it('forwards a failed response that carries only body.error.format with that text as message (issue #663)', async () => {
+      const h = await createHarness();
+      h.host.forwardRequest.mockResolvedValueOnce({
+        seq: 999,
+        type: 'response',
+        request_seq: 0,
+        command: 'evaluate',
+        success: false,
+        body: { error: { id: 2013, format: 'Uncaught ReferenceError: {name} is not defined', variables: { name: 'x' } } }
+      });
+      const socket = h.connect();
+      await join(h, socket);
+      send(socket, request('evaluate', { expression: 'x' }));
+      await flush();
+
+      const response = responseFor(socket, 'evaluate');
+      expect(response?.success).toBe(false);
+      expect(response?.message).toBe('Uncaught ReferenceError: x is not defined');
+    });
+
     it('correlates out-of-order completions to the right request_seq', async () => {
       const h = await createHarness();
       const deferred: Array<(r: DebugProtocol.Response) => void> = [];

tests/unit/proxy/dap-response-error.test.ts

@@ -0,0 +1,44 @@
+/**
+ * The reason a failed DAP response gives (issue #663): `message` when the
+ * adapter set it, else the formatted `body.error`, else the generic fallback.
+ */
+import { describe, expect, it } from 'vitest';
+import type { DebugProtocol } from '@vscode/debugprotocol';
+import {
+  DEFAULT_DAP_FAILURE_MESSAGE,
+  dapResponseErrorMessage,
+  dapResponseErrorText,
+  formatDapMessage
+} from '../../../src/proxy/dap-response-error.js';
+
+function failed(extra: Partial<DebugProtocol.Response>): DebugProtocol.Response {
+  return { seq: 1, type: 'response', request_seq: 1, command: 'evaluate', success: false, ...extra };
+}
+
+describe('dapResponseErrorMessage', () => {
+  it('prefers the raw message when the adapter set one', () => {
+    const response = failed({ message: 'short form', body: { error: { id: 1, format: 'long form' } } });
+    expect(dapResponseErrorMessage(response)).toBe('short form');
+  });
+
+  it('falls back to body.error.format (js-debug ProtocolError shape)', () => {
+    const response = failed({ body: { error: { id: 2013, format: 'Uncaught ReferenceError: x is not defined' } } });
+    expect(dapResponseErrorMessage(response)).toBe('Uncaught ReferenceError: x is not defined');
+  });
+
+  it('uses the generic fallback when neither text is present', () => {
+    expect(dapResponseErrorMessage(failed({}))).toBe(DEFAULT_DAP_FAILURE_MESSAGE);
+    expect(dapResponseErrorMessage(failed({ message: '' }))).toBe(DEFAULT_DAP_FAILURE_MESSAGE);
+    expect(dapResponseErrorMessage(failed({ body: { error: { id: 1, format: '' } } }))).toBe(DEFAULT_DAP_FAILURE_MESSAGE);
+    expect(dapResponseErrorMessage(failed({ body: { error: 'not a Message' } }))).toBe(DEFAULT_DAP_FAILURE_MESSAGE);
+    expect(dapResponseErrorText(failed({ body: {} }))).toBeUndefined();
+  });
+});
+
+describe('formatDapMessage', () => {
+  it('substitutes {name} placeholders from variables and leaves unknown ones alone', () => {
+    expect(formatDapMessage({ id: 1, format: 'Cannot set {name}: {reason} {unknown}', variables: { name: 'x', reason: 'read-only' } }))
+      .toBe('Cannot set x: read-only {unknown}');
+    expect(formatDapMessage({ id: 1, format: 'plain {text}' })).toBe('plain {text}');
+  });
+});

tests/unit/proxy/minimal-dap.test.ts

@@ -429,6 +429,44 @@ describe('MinimalDapClient', () => {
       await expect(requestPromise).rejects.toThrow('Failed to launch');
     });
 
+    it('rejects with body.error.format when the adapter sets no message (js-debug ProtocolError, issue #663)', async () => {
+      await client.connect();
+
+      const requestPromise = client.sendRequest('evaluate', { expression: 'httpSessions.size', frameId: 3 });
+
+      // js-debug: this._send({...response, success: false, body: { error: cause }}) — no `message`.
+      const errorResponse: DebugProtocol.Response = {
+        seq: 2,
+        type: 'response',
+        request_seq: 1,
+        command: 'evaluate',
+        success: false,
+        body: { error: { id: 2013, format: 'Uncaught ReferenceError: httpSessions is not defined', showUser: true } }
+      };
+
+      mockSocket.emit('data', createDapMessage(errorResponse));
+
+      await expect(requestPromise).rejects.toThrow('Uncaught ReferenceError: httpSessions is not defined');
+    });
+
+    it('fills {placeholders} in body.error.format from its variables (issue #663)', async () => {
+      await client.connect();
+
+      const requestPromise = client.sendRequest('setVariable', { name: 'x', value: '?' });
+      const errorResponse: DebugProtocol.Response = {
+        seq: 2,
+        type: 'response',
+        request_seq: 1,
+        command: 'setVariable',
+        success: false,
+        body: { error: { id: 1, format: 'Cannot set {name}: {reason}', variables: { name: 'x', reason: 'read-only' } } }
+      };
+
+      mockSocket.emit('data', createDapMessage(errorResponse));
+
+      await expect(requestPromise).rejects.toThrow('Cannot set x: read-only');
+    });
+
     it('should handle concurrent requests', async () => {
       await client.connect();
 
@@ -2027,6 +2065,32 @@ describe('MinimalDapClient', () => {
 
       c.shutdown('test done');
     });
+
+    it('logs body.error.format as errorMessage when the adapter sets no message (issue #663)', async () => {
+      const c = new MinimalDapClient('localhost', 5678);
+      loggerInstances.forEach((l) => l.info.mockClear());
+
+      await (c as any).handleProtocolMessage({
+        seq: 12,
+        type: 'response',
+        request_seq: 9,
+        command: 'evaluate',
+        success: false,
+        body: { error: { id: 2013, format: 'Uncaught ReferenceError: httpSessions is not defined' } }
+      } satisfies DebugProtocol.Response);
+
+      const responseLog = loggerInstances
+        .flatMap((l) => l.info.mock.calls)
+        .find(([msg]) => typeof msg === 'string' && msg.includes('DAP message: response'));
+      expect(responseLog?.[1]).toMatchObject({
+        command: 'evaluate',
+        success: false,
+        request_seq: 9,
+        errorMessage: 'Uncaught ReferenceError: httpSessions is not defined'
+      });
+
+      c.shutdown('test done');
+    });
   });
 
   describe('Trace connection labels (issue #518)', () => {