Skip to content

Commit c839ff1

Browse files
natorionnatorion
committed
fix: escape backslashes in logpoint messages and add a test case for correct handling.
1 parent 16cf90f commit c839ff1

2 files changed

Lines changed: 172 additions & 116 deletions

File tree

‎src/tools/debugger.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,10 @@ export const setLogpoint = definePageTool({
169169
// NOTE: This simple regex replacement assumes balanced braces and no nested braces.
170170
const runExpression =
171171
'console.log(`' +
172-
message.replace(/`/g, '\\`').replace(/\{([^}]+)\}/g, '${$1}') +
172+
message
173+
.replace(/\\/g, '\\\\')
174+
.replace(/`/g, '\\`')
175+
.replace(/\{([^}]+)\}/g, '${$1}') +
173176
'`)';
174177
const condition = `(${runExpression}, false)`;
175178

‎tests/tools/debugger.test.ts‎

Lines changed: 168 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -104,142 +104,195 @@ describe('debugger', () => {
104104
);
105105
});
106106
});
107+
});
107108

108-
it('reports not paused when execution is running', async () => {
109-
await withMcpContext(async (response, context) => {
110-
const page = context.getSelectedMcpPage();
111-
await enableDebugger.handler({params: {}, page}, response, context);
112-
113-
response.resetResponseLineForTesting();
114-
await getPausedState.handler({params: {}, page}, response, context);
115-
assert.ok(response.responseLines[0].includes('Debugger is not paused'));
116-
});
109+
it('escapes backslashes in logpoint message', async () => {
110+
await withMcpContext(async (response, context) => {
111+
const page = context.getSelectedMcpPage();
112+
await page.pptrPage.setContent(
113+
'<script>function test() { console.log("test"); }</script>',
114+
);
115+
116+
await setLogpoint.handler(
117+
{
118+
params: {
119+
url: 'http://localhost/',
120+
lineNumber: 1,
121+
message: 'Path: C:\\Windows\\System32',
122+
},
123+
page,
124+
},
125+
response,
126+
context,
127+
);
128+
129+
const setOutput = response.responseLines.join('\n');
130+
const breakpointIdMatch = setOutput.match(/Logpoint set with ID: (.*)/);
131+
assert.ok(breakpointIdMatch, 'Should return logpoint ID');
132+
133+
// In generated code string: console.log(`Path: C:\\Windows\\System32`)
134+
// But because it's inside a string in code, backslashes are escaped.
135+
// Wait. The code string is `console.log("...")`.
136+
// If we use backticks in code: `console.log(\`Path: C:\\\\Windows\\\\System32\`)`
137+
// My fix: replace `\` with `\\`.
138+
// Input: `C:\Windows`.
139+
// Output string in `condition`: `console.log(`Path: C:\\Windows`)`
140+
// Wait. `console.log` executes at runtime.
141+
// `console.log(`C:\Windows`)` prints `C:Windows` (escapes W? no). `C:\Windows`.
142+
// Check: `console.log(`C:\\Windows`)` prints `C:\Windows`.
143+
// So we want the condition code to contain `console.log(`C:\\Windows`)`.
144+
// So we need `runExpression` string to be `console.log(`C:\\Windows`)`.
145+
// My code: `message.replace(/\\/g, '\\\\')`.
146+
// Input `C:\Windows` -> `C:\\Windows`.
147+
// Result `runExpression`: `console.log(`C:\\Windows`)`.
148+
// This is what we want.
149+
150+
// So expected string in condition: `console.log(\`Path: C:\\\\Windows\\\\System32\`)`?
151+
// No. `C:\\Windows`.
152+
// In JS string literal for test assertion:
153+
// We want to match `C:\\Windows`.
154+
// Regex or string includes: `'C:\\\\Windows'`.
155+
156+
assert.ok(
157+
setOutput.includes('Path: C:\\\\Windows\\\\System32'),
158+
'Should double escape backslashes for template literal',
159+
);
117160
});
161+
});
118162

119-
it('pauses on breakpoint and resumes', async () => {
120-
await withMcpContext(async (response, context) => {
121-
const page = context.getSelectedMcpPage();
122-
const pptrPage = page.pptrPage;
163+
it('reports not paused when execution is running', async () => {
164+
await withMcpContext(async (response, context) => {
165+
const page = context.getSelectedMcpPage();
166+
await enableDebugger.handler({params: {}, page}, response, context);
123167

124-
// We need a script that runs somewhat later or triggered by us to hit the breakpoint reliably in test
125-
// Or we can use `debugger;` statement.
126-
await pptrPage.evaluate(() => {
127-
// @ts-expect-error - Window property
128-
window.debugMe = () => {
129-
// eslint-disable-next-line no-debugger
130-
debugger;
131-
};
132-
});
168+
response.resetResponseLineForTesting();
169+
await getPausedState.handler({params: {}, page}, response, context);
170+
assert.ok(response.responseLines[0].includes('Debugger is not paused'));
171+
});
172+
});
133173

134-
// Enable debugger
135-
await enableDebugger.handler({params: {}, page}, response, context);
174+
it('pauses on breakpoint and resumes', async () => {
175+
await withMcpContext(async (response, context) => {
176+
const page = context.getSelectedMcpPage();
177+
const pptrPage = page.pptrPage;
178+
179+
// We need a script that runs somewhat later or triggered by us to hit the breakpoint reliably in test
180+
// Or we can use `debugger;` statement.
181+
await pptrPage.evaluate(() => {
182+
// @ts-expect-error - Window property
183+
window.debugMe = () => {
184+
// eslint-disable-next-line no-debugger
185+
debugger;
186+
};
187+
});
136188

137-
// Trigger debugger
189+
// Enable debugger
190+
await enableDebugger.handler({params: {}, page}, response, context);
138191

139-
// We trigger execution, but we usually need to do it without awaiting if it pauses.
140-
await pptrPage.evaluate(() => {
141-
setTimeout(() => {
142-
// @ts-expect-error - Window property
143-
window.debugMe();
144-
}, 100);
145-
});
192+
// Trigger debugger
146193

147-
// Wait a bit for pause
148-
await new Promise(r => setTimeout(r, 500));
194+
// We trigger execution, but we usually need to do it without awaiting if it pauses.
195+
await pptrPage.evaluate(() => {
196+
setTimeout(() => {
197+
// @ts-expect-error - Window property
198+
window.debugMe();
199+
}, 100);
200+
});
149201

150-
response.resetResponseLineForTesting();
151-
await getPausedState.handler({params: {}, page}, response, context);
202+
// Wait a bit for pause
203+
await new Promise(r => setTimeout(r, 500));
152204

153-
const output = response.responseLines.join('\n');
154-
assert.ok(
155-
output.includes('Paused state') ||
156-
output.includes('Debugger is not paused'),
157-
'Should report state (flake warning: might not be paused yet)',
158-
);
205+
response.resetResponseLineForTesting();
206+
await getPausedState.handler({params: {}, page}, response, context);
159207

160-
if (output.includes('Paused state')) {
161-
// Test resume
162-
response.resetResponseLineForTesting();
163-
await resume.handler({params: {}, page}, response, context);
164-
assert.ok(response.responseLines[0].includes('Resumed execution'));
165-
}
166-
});
167-
});
208+
const output = response.responseLines.join('\n');
209+
assert.ok(
210+
output.includes('Paused state') ||
211+
output.includes('Debugger is not paused'),
212+
'Should report state (flake warning: might not be paused yet)',
213+
);
168214

169-
it('reports not enabled when calling getPausedState without enabling', async () => {
170-
await withMcpContext(async (response, context) => {
171-
const page = context.getSelectedMcpPage();
172-
await getPausedState.handler({params: {}, page}, response, context);
173-
assert.ok(response.responseLines[0].includes('Debugger is not enabled'));
174-
});
215+
if (output.includes('Paused state')) {
216+
// Test resume
217+
response.resetResponseLineForTesting();
218+
await resume.handler({params: {}, page}, response, context);
219+
assert.ok(response.responseLines[0].includes('Resumed execution'));
220+
}
175221
});
222+
});
176223

177-
it('throws when calling getScopeVariables without enabling', async () => {
178-
await withMcpContext(async (response, context) => {
179-
const page = context.getSelectedMcpPage();
180-
try {
181-
await getScopeVariables.handler(
182-
{params: {callFrameId: '1', scopeIndex: 0}, page},
183-
response,
184-
context,
185-
);
186-
assert.fail('Should have thrown');
187-
} catch (e) {
188-
const error = e as Error;
189-
if (!error.message.includes('Debugger is not enabled')) {
190-
console.error('Unexpected error:', e);
191-
assert.fail(`Expected "Debugger is not enabled", got: ${e.message}`);
192-
}
193-
assert.ok(true);
194-
}
195-
});
224+
it('reports not enabled when calling getPausedState without enabling', async () => {
225+
await withMcpContext(async (response, context) => {
226+
const page = context.getSelectedMcpPage();
227+
await getPausedState.handler({params: {}, page}, response, context);
228+
assert.ok(response.responseLines[0].includes('Debugger is not enabled'));
196229
});
230+
});
197231

198-
it('evaluates on call frame (mock check)', async () => {
199-
// This is hard to test e2e without actually being paused.
200-
// We verified "not paused" error in getPausedState.
201-
await withMcpContext(async (response, context) => {
202-
const page = context.getSelectedMcpPage();
203-
try {
204-
await evaluateOnCallFrame.handler(
205-
{params: {callFrameId: 'fake', expression: '1+1'}, page},
206-
response,
207-
context,
208-
);
209-
} catch (e) {
210-
// It might fail because session throws "Invalid parameters" or similar from CDP
211-
assert.ok(e);
232+
it('throws when calling getScopeVariables without enabling', async () => {
233+
await withMcpContext(async (response, context) => {
234+
const page = context.getSelectedMcpPage();
235+
try {
236+
await getScopeVariables.handler(
237+
{params: {callFrameId: '1', scopeIndex: 0}, page},
238+
response,
239+
context,
240+
);
241+
assert.fail('Should have thrown');
242+
} catch (e) {
243+
const error = e as Error;
244+
if (!error.message.includes('Debugger is not enabled')) {
245+
console.error('Unexpected error:', e);
246+
assert.fail(`Expected "Debugger is not enabled", got: ${e.message}`);
212247
}
213-
});
248+
assert.ok(true);
249+
}
214250
});
215-
it('removes logpoints via removeAllBreakpoints', async () => {
216-
await withMcpContext(async (response, context) => {
217-
const page = context.getSelectedMcpPage();
218-
await page.pptrPage.setContent('<script>function test() {}</script>');
251+
});
219252

220-
// Set a logpoint
221-
await setLogpoint.handler(
222-
{
223-
params: {url: 'http://localhost/', lineNumber: 1, message: 'Log'},
224-
page,
225-
},
253+
it('evaluates on call frame (mock check)', async () => {
254+
// This is hard to test e2e without actually being paused.
255+
// We verified "not paused" error in getPausedState.
256+
await withMcpContext(async (response, context) => {
257+
const page = context.getSelectedMcpPage();
258+
try {
259+
await evaluateOnCallFrame.handler(
260+
{params: {callFrameId: 'fake', expression: '1+1'}, page},
226261
response,
227262
context,
228263
);
229-
230-
response.resetResponseLineForTesting();
231-
232-
// Remove all
233-
// We need to import removeAllBreakpoints
234-
const {removeAllBreakpoints} =
235-
await import('../../src/tools/debugger.js');
236-
await removeAllBreakpoints.handler({params: {}, page}, response, context);
237-
238-
assert.ok(
239-
response.responseLines[0].includes(
240-
'All breakpoints and logpoints removed',
241-
),
242-
);
243-
});
264+
} catch (e) {
265+
// It might fail because session throws "Invalid parameters" or similar from CDP
266+
assert.ok(e);
267+
}
268+
});
269+
});
270+
it('removes logpoints via removeAllBreakpoints', async () => {
271+
await withMcpContext(async (response, context) => {
272+
const page = context.getSelectedMcpPage();
273+
await page.pptrPage.setContent('<script>function test() {}</script>');
274+
275+
// Set a logpoint
276+
await setLogpoint.handler(
277+
{
278+
params: {url: 'http://localhost/', lineNumber: 1, message: 'Log'},
279+
page,
280+
},
281+
response,
282+
context,
283+
);
284+
285+
response.resetResponseLineForTesting();
286+
287+
// Remove all
288+
// We need to import removeAllBreakpoints
289+
const {removeAllBreakpoints} = await import('../../src/tools/debugger.js');
290+
await removeAllBreakpoints.handler({params: {}, page}, response, context);
291+
292+
assert.ok(
293+
response.responseLines[0].includes(
294+
'All breakpoints and logpoints removed',
295+
),
296+
);
244297
});
245298
});

0 commit comments

Comments
 (0)