diff --git a/changelog.d/865.changed.md b/changelog.d/865.changed.md new file mode 100644 index 00000000..53080365 --- /dev/null +++ b/changelog.d/865.changed.md @@ -0,0 +1 @@ +**A launch whose only armed instrument is a logpoint no longer says "no breakpoints"** — `start_debugging` (and `wait_for_stop`) answered a still-running program with "nothing is armed to stop it (no breakpoints, no entry stop, no caught-exception filter)" even when a logpoint was set and firing; both halves were true, since a logpoint never stops the program, but "no breakpoints" reads as "your logpoint was not registered" — the first thing a caller checks when a logpoint seems silent. Logpoints the adapter runs on are now named apart from what is armed to stop the program: "1 logpoint(s) that log without stopping is armed (read get_output for the messages), and nothing is armed to stop it (no pausing breakpoints, …)"; with breakpoints as well, both are named. Logpoints downgraded to pausing breakpoints (adapters without `supportsLogPoints`) are still counted as armed, as before (#865) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index f6678874..5be38fcb 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -402,7 +402,7 @@ Starts debugging a script. - **The program stops** — `state: "paused"` with `reason` (the example above). A breakpoint reached as the program starts is answered here, in the one call. - **The program ends** — `state: "stopped"` with the run-to-completion summary and `exitCode` below. -- **The hold elapses** — `state: "running"` with **`pending: true`** (top level and in `data`, like `step_over`) and a `message` that says what is armed: `…has not reached 2 breakpoint(s) yet. Nothing was cancelled: what is armed stays armed…`; or, with nothing armed (the `"uncaught"` default does not count — a crash is not a stop you are waiting for), `…nothing is armed to stop it (no breakpoints, no entry stop, no caught-exception filter)…`; or, for a `noDebug` launch, that the debugger is off. What is armed is read when the answer is built, so a breakpoint set while the launch was in flight is named. +- **The hold elapses** — `state: "running"` with **`pending: true`** (top level and in `data`, like `step_over`) and a `message` that says what is armed: `…has not reached 2 breakpoint(s) yet. Nothing was cancelled: what is armed stays armed…`; or, with nothing armed (the `"uncaught"` default does not count — a crash is not a stop you are waiting for), `…nothing is armed to stop it (no breakpoints, no entry stop, no caught-exception filter)…`; or, for a `noDebug` launch, that the debugger is off. Logpoints the adapter runs on never stop the program, so they are named apart: `…1 logpoint(s) that log without stopping is armed (read get_output for the messages), and nothing is armed to stop it (no pausing breakpoints, …)…`. What is armed is read when the answer is built, so a breakpoint set while the launch was in flight is named. The hold is not a limit on the program. Nothing is cancelled when it elapses: breakpoints and exception filters stay armed for the life of the process, and the program pauses when it reaches one — in a second or in a day. The hold only decides whether this call reports the first stop itself. It is deliberately short (about a second, sized from the measured time each adapter takes from launch to a first breakpoint; the number is a tunable, not part of this contract), so that a launch whose breakpoint needs an outside trigger — a request handler in a server — hands control back at once. To wait for the stop, call [`wait_for_stop`](#wait_for_stop), which blocks until the program stops or ends for as long as you say; `list_debug_sessions` reports the state without waiting. diff --git a/src/session/execution/execution-controller.ts b/src/session/execution/execution-controller.ts index 22ad16e7..6fa04b1a 100644 --- a/src/session/execution/execution-controller.ts +++ b/src/session/execution/execution-controller.ts @@ -853,6 +853,7 @@ export class ExecutionController { state === SessionState.INITIALIZING ? 'initializing' : 'running', { ...(arming.armed ? { armedSummary: arming.summary } : {}), + ...(arming.loggingSummary ? { loggingSummary: arming.loggingSummary } : {}), ...(debuggerOffWhy(session) ? { debuggerOffWhy: debuggerOffWhy(session) } : {}) } ), diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index e9ef4bba..bffa9b95 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -717,7 +717,8 @@ export class DebugLauncher { const arming = describeLaunchArming(finalSession, effectiveLaunchArgs?.stopOnEntry); stillRunning = ErrorMessages.launchStillRunning( arming.armed ? arming.summary : undefined, - debuggerOnAnyway ? undefined : debuggerOffWhy(finalSession) + debuggerOnAnyway ? undefined : debuggerOffWhy(finalSession), + arming.loggingSummary ); } diff --git a/src/session/launch/launch-arming.ts b/src/session/launch/launch-arming.ts index a68c7a6a..86228c13 100644 --- a/src/session/launch/launch-arming.ts +++ b/src/session/launch/launch-arming.ts @@ -32,6 +32,15 @@ export interface LaunchArming { pausesOnCaughtExceptions: boolean; /** The armed clauses in one phrase ("2 breakpoint(s) and an entry stop"); '' when unarmed. */ summary: string; + /** + * Logpoints the adapter runs on (issue #865). They never stop the program, + * so they are not armed and not in `summary` — but an answer that says + * "no breakpoints" to a caller who set one reads as "it was not + * registered", so they are counted and worded apart. + */ + logpoints: number; + /** "1 logpoint(s) that log without stopping"; undefined when there are none. */ + loggingSummary?: string; } export function describeLaunchArming(session: ArmingSession, stopOnEntry: boolean | undefined): LaunchArming { @@ -41,10 +50,13 @@ export function describeLaunchArming(session: ArmingSession, stopOnEntry: boolea const logpointsRunOn = session.adapterCapabilities?.supportsLogPoints === true; let lineBreakpoints = 0; let logpointsThatPause = 0; + let logpoints = 0; for (const bp of session.breakpoints.values()) { if (bp.logMessage === undefined) { lineBreakpoints++; - } else if (!logpointsRunOn) { + } else if (logpointsRunOn) { + logpoints++; + } else { logpointsThatPause++; } } @@ -76,7 +88,9 @@ export function describeLaunchArming(session: ArmingSession, stopOnEntry: boolea functionBreakpoints, stopOnEntry: entry, pausesOnCaughtExceptions, - summary: joinClauses(clauses) + summary: joinClauses(clauses), + logpoints, + ...(logpoints > 0 ? { loggingSummary: `${logpoints} logpoint(s) that log without stopping` } : {}) }; } diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 3dc8e6f7..6b7b7434 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -44,6 +44,14 @@ const DEBUGGER_OFF_FOR_LAUNCH = 'the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; ' + 'drop noDebug and launch again to debug'; +/** + * The clause naming logpoints the adapter runs on (issue #865): they never + * stop the program, so a launch or wait_for_stop answer names them apart + * from what is armed to stop it, with where their messages go. + */ +const loggingClause = (loggingSummary: string): string => + `${loggingSummary} ${loggingSummary.startsWith('1 ') ? 'is' : 'are'} armed (read get_output for the messages)`; + export const ErrorMessages = { /** * Error message for DAP request timeouts @@ -179,7 +187,7 @@ export const ErrorMessages = { waitForStopPending: ( seconds: number, state: 'running' | 'initializing', - why: { armedSummary?: string; debuggerOffWhy?: string } = {} + why: { armedSummary?: string; debuggerOffWhy?: string; loggingSummary?: string } = {} ) => { if (state === 'initializing') { return `The session is still starting after ${seconds}s (its launch or attach has not completed). ` + @@ -189,15 +197,19 @@ export const ErrorMessages = { return `The program is still running after ${seconds}s — ${why.debuggerOffWhy}. ` + `Call wait_for_stop again to wait for it to end.`; } + // Logpoints the adapter runs on never stop the program, so they are + // named apart from what is armed to stop it (issue #865): a caller who + // set one reads "no breakpoint is armed" as "it was not registered". + const logging = why.loggingSummary ? `; ${loggingClause(why.loggingSummary)}` : ''; if (why.armedSummary) { - return `The program is still running after ${seconds}s without reaching ${why.armedSummary}. ` + + return `The program is still running after ${seconds}s without reaching ${why.armedSummary}${logging}. ` + `Nothing was cancelled: what is armed stays armed, and the session becomes 'paused' when the program ` + `gets there. Call wait_for_stop again to keep waiting, or pause_execution to interrupt it.`; } - return `The program is still running after ${seconds}s, and no breakpoint or caught-exception filter is armed ` + - `to stop it: unless a step or a pause is still in flight, it will stop only for an uncaught exception the ` + - `debugger catches by default, or report its exit. Call wait_for_stop again to wait for either, or ` + - `pause_execution to interrupt it.`; + return `The program is still running after ${seconds}s${logging}, and no ${why.loggingSummary ? 'pausing ' : ''}breakpoint ` + + `or caught-exception filter is armed to stop it: unless a step or a pause is still in flight, it will stop ` + + `only for an uncaught exception the debugger catches by default, or report its exit. Call wait_for_stop ` + + `again to wait for either, or pause_execution to interrupt it.`; }, /** @@ -228,20 +240,25 @@ export const ErrorMessages = { * Used in: src/session/launch/debug-launcher.ts * @param armedSummary - The armed clauses ("2 breakpoint(s) and an entry stop"); undefined when nothing is armed * @param debuggerOffWhy - The debugger-off sentence, when it applies + * @param loggingSummary - Logpoints the adapter runs on ("1 logpoint(s) that log without stopping"), named apart + * because they never stop the program (issue #865); undefined when there are none */ - launchStillRunning: (armedSummary: string | undefined, debuggerOffWhy?: string) => { + launchStillRunning: (armedSummary: string | undefined, debuggerOffWhy?: string, loggingSummary?: string) => { if (debuggerOffWhy) { return `The program is running — ${debuggerOffWhy}. ` + `Call wait_for_stop to wait for it to end, or read get_output.`; } + const logging = loggingSummary ? `; ${loggingClause(loggingSummary)}` : ''; if (armedSummary) { - return `The program is running and has not reached ${armedSummary} yet. ` + + return `The program is running and has not reached ${armedSummary} yet${logging}. ` + `Nothing was cancelled: what is armed stays armed, and the session will report 'paused' when the program ` + `gets there — call wait_for_stop to block until then, or pause_execution to interrupt.`; } - return `The program is running and nothing is armed to stop it ` + - `(no breakpoints, no entry stop, no caught-exception filter): it will stop only for an uncaught ` + - `exception the debugger catches by default, or report its exit. Call wait_for_stop to wait for ` + + // "no breakpoints" to a caller who set a logpoint reads as "it was not + // registered" — the first thing checked when a logpoint seems silent. + return `The program is running${logging}${loggingSummary ? ',' : ''} and nothing is armed to stop it ` + + `(no ${loggingSummary ? 'pausing ' : ''}breakpoints, no entry stop, no caught-exception filter): it will stop only ` + + `for an uncaught exception the debugger catches by default, or report its exit. Call wait_for_stop to wait for ` + `either, read get_output, or set breakpoints — they take effect on the running program.`; }, diff --git a/tests/core/unit/session/launch/launch-arming.test.ts b/tests/core/unit/session/launch/launch-arming.test.ts index 0dd2b1e6..25cd77f2 100644 --- a/tests/core/unit/session/launch/launch-arming.test.ts +++ b/tests/core/unit/session/launch/launch-arming.test.ts @@ -79,6 +79,36 @@ describe('describeLaunchArming', () => { expect(describeLaunchArming(unknown, false).armed).toBe(true); }); + it('names logpoints the adapter runs on as logging without stopping, apart from the armed summary (issue #865)', () => { + const s = session({ adapterCapabilities: { supportsLogPoints: true } }); + s.breakpoints.set('lp1', lineBp('lp1', 'x={x}')); + s.breakpoints.set('lp2', lineBp('lp2', 'y={y}')); + const arming = describeLaunchArming(s, false); + + // They do not stop the program: not armed, not in the armed summary... + expect(arming.armed).toBe(false); + expect(arming.summary).toBe(''); + // ...but counted and worded, so the answer can say what IS set. + expect(arming.logpoints).toBe(2); + expect(arming.loggingSummary).toBe('2 logpoint(s) that log without stopping'); + + // Alongside a pausing breakpoint, both summaries stand. + s.breakpoints.set('bp', lineBp('bp')); + const both = describeLaunchArming(s, false); + expect(both.armed).toBe(true); + expect(both.summary).toBe('1 breakpoint(s)'); + expect(both.loggingSummary).toBe('2 logpoint(s) that log without stopping'); + + // Nothing to say when there are none. + expect(describeLaunchArming(session(), false).logpoints).toBe(0); + expect(describeLaunchArming(session(), false).loggingSummary).toBeUndefined(); + // A downgraded logpoint is a pausing breakpoint, not a logging one. + const downgraded = session({ adapterCapabilities: { supportsLogPoints: false } }); + downgraded.breakpoints.set('lp', lineBp('lp', 'x={x}')); + expect(describeLaunchArming(downgraded, false).logpoints).toBe(0); + expect(describeLaunchArming(downgraded, false).loggingSummary).toBeUndefined(); + }); + it('lists every armed clause in one summary', () => { const s = session({ effectiveBreakOnExceptions: 'all' }); s.breakpoints.set('a', lineBp('a')); diff --git a/tests/core/unit/session/session-manager-launch-contract.test.ts b/tests/core/unit/session/session-manager-launch-contract.test.ts index 3902e4ae..402fa8a5 100644 --- a/tests/core/unit/session/session-manager-launch-contract.test.ts +++ b/tests/core/unit/session/session-manager-launch-contract.test.ts @@ -111,6 +111,24 @@ describe('SessionManager - launch contract (issues #823, #826, #851)', () => { expect(result?.data?.message).toContain('wait_for_stop'); }); + it('names an armed logpoint instead of saying "no breakpoints" when only logpoints are set (issue #865)', async () => { + const sessionId = await createSession(); + const started = launch(sessionId); + await vi.advanceTimersByTimeAsync(300); + // The handshake has said logpoints run on; a logpoint is then set. + proxy().simulateEvent('adapter-capabilities', { supportsLogPoints: true }); + await sessionManager.setBreakpoint(sessionId, { file: 'test.py', line: 5, logMessage: 'x={x}' }); + await vi.advanceTimersByTimeAsync(HOLD_MS); + + const result = started.result(); + expect(result).toMatchObject({ success: true, state: SessionState.RUNNING }); + expect(result?.data?.pending).toBe(true); + expect(result?.data?.message).toMatch(/1 logpoint\(s\) that log without stopping/); + expect(result?.data?.message).toContain('get_output'); + expect(result?.data?.message).toMatch(/nothing is armed to stop it/); + expect(result?.data?.message).not.toMatch(/\(no breakpoints,/); + }); + it('answers stopped with the exit code for a program that ends inside the hold', async () => { const sessionId = await createSession(); const started = launch(sessionId); diff --git a/tests/unit/utils/error-messages.test.ts b/tests/unit/utils/error-messages.test.ts index 70df8e6a..07a23325 100644 --- a/tests/unit/utils/error-messages.test.ts +++ b/tests/unit/utils/error-messages.test.ts @@ -187,6 +187,25 @@ describe('ErrorMessages', () => { expect(message).toContain('get_output'); }); + it('a launch still running with only logpoints names them and points at get_output (issue #865)', () => { + const message = ErrorMessages.launchStillRunning(undefined, undefined, '1 logpoint(s) that log without stopping'); + expect(message).toMatch(/1 logpoint\(s\) that log without stopping/); + expect(message).toMatch(/nothing is armed to stop it/i); + // "no breakpoints" would read as "your logpoint was not registered" + expect(message).not.toMatch(/\(no breakpoints,/); + expect(message).toMatch(/no pausing breakpoint/); + expect(message).toContain('get_output'); + expect(message).toContain('wait_for_stop'); + }); + + it('a launch still running with breakpoints and logpoints names both (issue #865)', () => { + const message = ErrorMessages.launchStillRunning('2 breakpoint(s)', undefined, '1 logpoint(s) that log without stopping'); + expect(message).toMatch(/has not reached 2 breakpoint\(s\) yet/); + expect(message).toMatch(/1 logpoint\(s\) that log without stopping/); + expect(message).toContain('get_output'); + expect(message).toMatch(/Nothing was cancelled/); + }); + it('a launch still running with the debugger off promises no stop', () => { const message = ErrorMessages.launchStillRunning('2 breakpoint(s)', ErrorMessages.debuggerOffForLaunch); expect(message).toContain(ErrorMessages.debuggerOffForLaunch); @@ -215,6 +234,16 @@ describe('ErrorMessages', () => { expect(message).toContain('wait_for_stop'); }); + it('wait_for_stop with only logpoints armed names them (issue #865)', () => { + const message = ErrorMessages.waitForStopPending(30, 'running', { loggingSummary: '1 logpoint(s) that log without stopping' }); + expect(message).toMatch(/still running after 30s/); + expect(message).toMatch(/1 logpoint\(s\) that log without stopping/); + expect(message).toContain('get_output'); + expect(message).toMatch(/no pausing breakpoint or caught-exception filter is armed/); + expect(message).not.toMatch(/stays? armed/); + expect(message).toContain('wait_for_stop'); + }); + it('wait_for_stop with the debugger off waits for the exit and promises no stop', () => { const message = ErrorMessages.waitForStopPending(30, 'running', { debuggerOffWhy: ErrorMessages.debuggerOffForLaunch }); expect(message).toContain(ErrorMessages.debuggerOffForLaunch);