Skip to content

Commit 326c279

Browse files
nattalliusNatallia Harshunova
andauthored
chore: report console issues in get_console_messages (#594)
Co-authored-by: Natallia Harshunova <[email protected]>
1 parent 5b84e76 commit 326c279

File tree

10 files changed

+339
-34
lines changed

10 files changed

+339
-34
lines changed

src/DevtoolsUtils.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,17 @@
66

77
import {
88
type Issue,
9+
type AggregatedIssue,
910
type IssuesManagerEventTypes,
11+
MarkdownIssueDescription,
12+
Marked,
1013
Common,
1114
I18n,
1215
} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
1316

17+
import {ISSUE_UTILS} from './issue-descriptions.js';
18+
import {logger} from './logger.js';
19+
1420
export function extractUrlLikeFromDevToolsTitle(
1521
title: string,
1622
): string | undefined {
@@ -69,6 +75,48 @@ export class FakeIssuesManager extends Common.ObjectWrapper
6975
}
7076
}
7177

78+
export function mapIssueToMessageObject(issue: AggregatedIssue) {
79+
const count = issue.getAggregatedIssuesCount();
80+
const markdownDescription = issue.getDescription();
81+
const filename = markdownDescription?.file;
82+
if (!markdownDescription) {
83+
logger(`no description found for issue:` + issue.code);
84+
return null;
85+
}
86+
const rawMarkdown = filename
87+
? ISSUE_UTILS.getIssueDescription(filename)
88+
: null;
89+
if (!rawMarkdown) {
90+
logger(`no markdown ${filename} found for issue:` + issue.code);
91+
return null;
92+
}
93+
let processedMarkdown: string;
94+
let title: string | null;
95+
96+
try {
97+
processedMarkdown = MarkdownIssueDescription.substitutePlaceholders(
98+
rawMarkdown,
99+
markdownDescription.substitutions,
100+
);
101+
const markdownAst = Marked.Marked.lexer(processedMarkdown);
102+
title = MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst);
103+
} catch {
104+
logger('error parsing markdown for issue ' + issue.code());
105+
return null;
106+
}
107+
if (!title) {
108+
logger('cannot read issue title from ' + filename);
109+
return null;
110+
}
111+
return {
112+
type: 'issue',
113+
item: issue,
114+
message: title,
115+
count,
116+
description: processedMarkdown,
117+
};
118+
}
119+
72120
I18n.DevToolsLocale.DevToolsLocale.instance({
73121
create: true,
74122
data: {

src/McpContext.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,13 @@ export class McpContext implements Context {
199199
this.logger('no cdpBackendNodeId');
200200
return;
201201
}
202+
if (this.#textSnapshot === null)
203+
throw new Error(
204+
"The snapshot is not defined, can't resolve backendNodeId: " +
205+
cdpBackendNodeId,
206+
);
202207
// TODO: index by backendNodeId instead.
203-
const queue = [this.#textSnapshot?.root];
208+
const queue = [this.#textSnapshot.root];
204209
while (queue.length) {
205210
const current = queue.pop()!;
206211
if (current.backendNodeId === cdpBackendNodeId) {

src/McpResponse.ts

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import {
8-
AggregatedIssue,
9-
Marked,
10-
MarkdownIssueDescription,
11-
} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
7+
import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
128

9+
import {mapIssueToMessageObject} from './DevtoolsUtils.js';
1310
import type {ConsoleMessageData} from './formatters/consoleFormatter.js';
1411
import {
1512
formatConsoleEventShort,
@@ -23,8 +20,6 @@ import {
2320
getStatusFromRequest,
2421
} from './formatters/networkFormatter.js';
2522
import {formatSnapshotNode} from './formatters/snapshotFormatter.js';
26-
import {getIssueDescription} from './issue-descriptions.js';
27-
import {logger} from './logger.js';
2823
import type {McpContext} from './McpContext.js';
2924
import type {
3025
ConsoleMessage,
@@ -256,6 +251,16 @@ export class McpResponse implements Response {
256251
}),
257252
),
258253
};
254+
} else if (message instanceof AggregatedIssue) {
255+
const mappedIssueMessage = mapIssueToMessageObject(message);
256+
if (!mappedIssueMessage)
257+
throw new Error(
258+
"Can't prpovide detals for the msgid " + consoleMessageStableId,
259+
);
260+
consoleData = {
261+
consoleMessageStableId,
262+
...mappedIssueMessage,
263+
};
259264
} else {
260265
consoleData = {
261266
consoleMessageStableId,
@@ -309,29 +314,11 @@ export class McpResponse implements Response {
309314
};
310315
}
311316
if (item instanceof AggregatedIssue) {
312-
const count = item.getAggregatedIssuesCount();
313-
const filename = item.getDescription()?.file;
314-
const rawMarkdown = filename
315-
? getIssueDescription(filename)
316-
: null;
317-
if (!rawMarkdown) {
318-
logger(`no markdown ${filename} found for issue:` + item.code);
319-
return null;
320-
}
321-
const markdownAst = Marked.Marked.lexer(rawMarkdown);
322-
const title =
323-
MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst);
324-
if (!title) {
325-
logger('cannot read issue title from ' + filename);
326-
return null;
327-
}
317+
const mappedIssueMessage = mapIssueToMessageObject(item);
318+
if (!mappedIssueMessage) return null;
328319
return {
329320
consoleMessageStableId,
330-
type: 'issue',
331-
item,
332-
message: title,
333-
count,
334-
args: [],
321+
...mappedIssueMessage,
335322
};
336323
}
337324
return {

src/formatters/consoleFormatter.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import type {AggregatedIssue} from '../../node_modules/chrome-devtools-frontend/mcp/mcp.js';
8+
79
export interface ConsoleMessageData {
810
consoleMessageStableId: number;
911
type?: string;
10-
item?: unknown;
12+
item?: AggregatedIssue;
1113
message?: string;
1214
count?: number;
15+
description?: string;
1316
args?: string[];
1417
}
1518

@@ -34,12 +37,12 @@ function getArgs(msg: ConsoleMessageData) {
3437

3538
// The verbose format for a console message, including all details.
3639
export function formatConsoleEventVerbose(msg: ConsoleMessageData): string {
40+
const aggregatedIssue = msg.item;
3741
const result = [
3842
`ID: ${msg.consoleMessageStableId}`,
39-
`Message: ${msg.type}> ${msg.message}`,
40-
formatArgs(msg),
43+
`Message: ${msg.type}> ${aggregatedIssue ? formatIssue(aggregatedIssue, msg.description) : msg.message}`,
44+
aggregatedIssue ? undefined : formatArgs(msg),
4145
].filter(line => !!line);
42-
4346
return result.join('\n');
4447
}
4548

@@ -62,3 +65,29 @@ function formatArgs(consoleData: ConsoleMessageData): string {
6265

6366
return result.join('\n');
6467
}
68+
69+
export function formatIssue(
70+
issue: AggregatedIssue,
71+
description?: string,
72+
): string {
73+
const result: string[] = [];
74+
75+
let processedMarkdown = description?.trim();
76+
// Remove heading in order not to conflict with the whole console message response markdown
77+
if (processedMarkdown?.startsWith('# ')) {
78+
processedMarkdown = processedMarkdown.substring(2).trimStart();
79+
}
80+
if (processedMarkdown) result.push(processedMarkdown);
81+
82+
const links = issue.getDescription()?.links;
83+
if (links && links.length > 0) {
84+
result.push('Learn more:');
85+
for (const link of links) {
86+
result.push(`[${link.linkTitle}](${link.link})`);
87+
}
88+
}
89+
90+
if (result.length === 0)
91+
return 'No details provided for the issue ' + issue.code();
92+
return result.join('\n');
93+
}

src/issue-descriptions.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,8 @@ export async function loadIssueDescriptions(): Promise<void> {
4747
export function getIssueDescription(fileName: string): string | null {
4848
return issueDescriptions[fileName] ?? null;
4949
}
50+
51+
export const ISSUE_UTILS = {
52+
loadIssueDescriptions,
53+
getIssueDescription,
54+
};

tests/DevtoolsUtils.test.ts

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,17 @@
55
*/
66

77
import assert from 'node:assert';
8-
import {describe, it} from 'node:test';
8+
import {afterEach, describe, it} from 'node:test';
99

10+
import sinon from 'sinon';
11+
12+
import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
1013
import {
1114
extractUrlLikeFromDevToolsTitle,
1215
urlsEqual,
16+
mapIssueToMessageObject,
1317
} from '../src/DevtoolsUtils.js';
18+
import {ISSUE_UTILS} from '../src/issue-descriptions.js';
1419

1520
describe('extractUrlFromDevToolsTitle', () => {
1621
it('deals with no trailing /', () => {
@@ -70,3 +75,104 @@ describe('urlsEqual', () => {
7075
);
7176
});
7277
});
78+
79+
describe('mapIssueToMessageObject', () => {
80+
const mockDescription = {
81+
file: 'mock-issue.md',
82+
substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]),
83+
links: [
84+
{link: 'http://example.com/learnmore', linkTitle: 'Learn more'},
85+
{
86+
link: 'http://example.com/another-learnmore',
87+
linkTitle: 'Learn more 2',
88+
},
89+
],
90+
};
91+
92+
afterEach(() => {
93+
sinon.restore();
94+
});
95+
96+
it('maps aggregated issue with substituted description', () => {
97+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
98+
mockAggregatedIssue.getDescription.returns(mockDescription);
99+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
100+
101+
const getIssueDescriptionStub = sinon.stub(
102+
ISSUE_UTILS,
103+
'getIssueDescription',
104+
);
105+
106+
getIssueDescriptionStub
107+
.withArgs('mock-issue.md')
108+
.returns(
109+
'# Mock Issue Title\n\nThis is a mock issue description with a {PLACEHOLDER_VALUE}.',
110+
);
111+
112+
const result = mapIssueToMessageObject(mockAggregatedIssue);
113+
const expected = {
114+
type: 'issue',
115+
item: mockAggregatedIssue,
116+
message: 'Mock Issue Title',
117+
count: 1,
118+
description:
119+
'# Mock Issue Title\n\nThis is a mock issue description with a substitution value.',
120+
};
121+
assert.deepStrictEqual(result, expected);
122+
});
123+
124+
it('returns null for the issue with no description', () => {
125+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
126+
mockAggregatedIssue.getDescription.returns(null);
127+
128+
const result = mapIssueToMessageObject(mockAggregatedIssue);
129+
assert.equal(result, null);
130+
});
131+
132+
it('returns null if there is no desciption file', () => {
133+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
134+
mockAggregatedIssue.getDescription.returns(mockDescription);
135+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
136+
137+
const getIssueDescriptionStub = sinon.stub(
138+
ISSUE_UTILS,
139+
'getIssueDescription',
140+
);
141+
142+
getIssueDescriptionStub.withArgs('mock-issue.md').returns(null);
143+
const result = mapIssueToMessageObject(mockAggregatedIssue);
144+
assert.equal(result, null);
145+
});
146+
147+
it("returns null if can't parse the title", () => {
148+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
149+
mockAggregatedIssue.getDescription.returns(mockDescription);
150+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
151+
152+
const getIssueDescriptionStub = sinon.stub(
153+
ISSUE_UTILS,
154+
'getIssueDescription',
155+
);
156+
157+
getIssueDescriptionStub
158+
.withArgs('mock-issue.md')
159+
.returns('No title test {PLACEHOLDER_VALUE}');
160+
assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null);
161+
});
162+
163+
it('returns null if devtools utill function throws an error', () => {
164+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
165+
mockAggregatedIssue.getDescription.returns(mockDescription);
166+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
167+
168+
const getIssueDescriptionStub = sinon.stub(
169+
ISSUE_UTILS,
170+
'getIssueDescription',
171+
);
172+
// An error will be thrown if placeholder doesn't start from PLACEHOLDER_
173+
getIssueDescriptionStub
174+
.withArgs('mock-issue.md')
175+
.returns('No title test {WRONG_PLACEHOLDER}');
176+
assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null);
177+
});
178+
});

0 commit comments

Comments
 (0)