Address comments

Author: Natallia Harshunova

src/McpResponse.ts

@@ -319,10 +319,15 @@ export class McpResponse implements Response {
               }
               const markdownAst = Marked.Marked.lexer(rawMarkdown);
               const title = findTitleFromMarkdownAst(markdownAst);
+              if (!title) {
+                logger('cannot read issue title from ' + filename);
+                return null;
+              }
               return {
                 consoleMessageStableId,
                 type: 'issue',
-                message: `${title}`,
+                item,
+                message: title,
                 count,
                 args: [],
               };

src/PageCollector.ts

@@ -14,7 +14,7 @@ import {
 
 import {FakeIssuesManager} from './DevtoolsUtils.js';
 import {logger} from './logger.js';
-import type {ConsoleMessage} from './third_party/index.js';
+import type {ConsoleMessage, Protocol} from './third_party/index.js';
 import {
   type Browser,
   type Frame,
@@ -96,13 +96,13 @@ export class PageCollector<T> {
   }
 
   public addPage(page: Page) {
-    if (this.storage.has(page)) {
-      return;
-    }
     this.#initializePage(page);
   }
 
   #initializePage(page: Page) {
+    if (this.storage.has(page)) {
+      return;
+    }
     const idGenerator = createIdGenerator();
     const storedLists: Array<Array<WithSymbolId<T>>> = [[]];
     this.storage.set(page, storedLists);
@@ -215,17 +215,13 @@ export class ConsoleCollector extends PageCollector<
   #mockIssuesManagers = new WeakMap<Page, FakeIssuesManager>();
 
   override addPage(page: Page): void {
-    if (this.storage.has(page)) {
-      return;
-    }
     super.addPage(page);
     void this.subscribeForIssues(page);
   }
   async subscribeForIssues(page: Page) {
-    if (!this.#seenIssueKeys.has(page)) {
-      this.#seenIssueKeys.set(page, new Set());
-    }
+    if (this.#seenIssueKeys.has(page)) return;
 
+    this.#seenIssueKeys.set(page, new Set());
     const mockManager = new FakeIssuesManager();
     const aggregator = new IssueAggregator(mockManager);
     this.#mockIssuesManagers.set(page, mockManager);
@@ -242,36 +238,33 @@ export class ConsoleCollector extends PageCollector<
         page.emit('issue', event.data);
       },
     );
-
     try {
       const session = await page.createCDPSession();
       session.on('Audits.issueAdded', data => {
-        // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
-        const issue = createIssuesFromProtocolIssue(null, data.issue)[0];
-        if (!issue) {
-          return;
-        }
+        const inspectorIssue =
+          data.issue satisfies Protocol.Audits.InspectorIssue;
+        // @ts-expect-error Types of protocol from Puppeteer and CDP are incomparable for InspectorIssueCode, one is union, other is enum
+        const issue = createIssuesFromProtocolIssue(null, inspectorIssue)[0];
+        if (!issue) return;
+
         const seenKeys = this.#seenIssueKeys.get(page)!;
         const primaryKey = issue.primaryKey();
         if (seenKeys.has(primaryKey)) return;
         seenKeys.add(primaryKey);
 
         const mockManager = this.#mockIssuesManagers.get(page);
-        if (mockManager) {
-          mockManager.dispatchEventToListeners(
-            IssuesManagerEvents.ISSUE_ADDED,
-            {
-              issue,
-              // @ts-expect-error We don't care that issues model is null
-              issuesModel: null,
-            },
-          );
-        }
+        if (!mockManager) return;
+
+        mockManager.dispatchEventToListeners(IssuesManagerEvents.ISSUE_ADDED, {
+          issue,
+          // @ts-expect-error We don't care that issues model is null
+          issuesModel: null,
+        });
       });
+
       await session.send('Audits.enable');
     } catch (e) {
-      const errorText = e instanceof Error ? e.message : JSON.stringify(e);
-      logger(`Error subscribing to issues: ${errorText}`);
+      logger('Error subscribing to issues', e);
     }
   }
 

src/formatters/consoleFormatter.ts

@@ -7,6 +7,7 @@
 export interface ConsoleMessageData {
   consoleMessageStableId: number;
   type?: string;
+  item?: unknown;
   message?: string;
   count?: number;
   args?: string[];

tests/PageCollector.test.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import assert from 'node:assert';
-import {describe, it} from 'node:test';
+import {beforeEach, describe, it} from 'node:test';
 
 import type {
   Browser,
@@ -13,6 +13,7 @@ import type {
   Page,
   Target,
   CDPSession,
+  Protocol,
 } from 'puppeteer-core';
 import sinon from 'sinon';
 
@@ -343,6 +344,21 @@ describe('NetworkCollector', () => {
 });
 
 describe('ConsoleCollector', () => {
+  let issue: Protocol.Audits.InspectorIssue;
+
+  beforeEach(() => {
+    issue = {
+      code: 'MixedContentIssue',
+      details: {
+        mixedContentIssueDetails: {
+          insecureURL: 'test.url',
+          resolutionStatus: 'MixedContentBlocked',
+          mainResourceURL: '',
+        },
+      },
+    };
+  });
+
   it('emits issues on page', async () => {
     const browser = getMockBrowser();
     const page = (await browser.pages())[0];
@@ -359,17 +375,6 @@ describe('ConsoleCollector', () => {
       } as ListenerMap;
     });
     await collector.init();
-
-    const issue = {
-      code: 'MixedContentIssue' as const,
-      details: {
-        mixedContentIssueDetails: {
-          insecureURL: 'test.url',
-        },
-      },
-    };
-
-    // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
     cdpSession.emit('Audits.issueAdded', {issue});
     sinon.assert.calledOnce(onIssuesListener);
 
@@ -391,26 +396,18 @@ describe('ConsoleCollector', () => {
     });
     await collector.init();
 
-    const issue = {
-      code: 'MixedContentIssue' as const,
-      details: {
-        mixedContentIssueDetails: {
-          insecureURL: 'test.url',
-        },
-      },
-    };
     const issue2 = {
-      code: 'PropertyRuleIssue' as const,
+      code: 'ElementAccessibilityIssue' as const,
       details: {
-        propertyRuleIssueDetails: {
-          test: 'test',
+        elementAccessibilityIssueDetails: {
+          nodeId: 1,
+          elementAccessibilityIssueReason: 'DisallowedSelectChild',
+          hasDisallowedAttributes: true,
         },
       },
-    };
+    } satisfies Protocol.Audits.InspectorIssue;
 
-    // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
     cdpSession.emit('Audits.issueAdded', {issue});
-    // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
     cdpSession.emit('Audits.issueAdded', {issue: issue2});
     const data = collector.getData(page);
     assert.equal(data.length, 2);
@@ -430,20 +427,10 @@ describe('ConsoleCollector', () => {
     });
     await collector.init();
 
-    const issue = {
-      code: 'MixedContentIssue' as const,
-      details: {
-        mixedContentIssueDetails: {
-          insecureURL: 'test.url',
-        },
-      },
-    };
-
-    // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
     cdpSession.emit('Audits.issueAdded', {issue});
-    // @ts-expect-error Types of protocol from Puppeteer and CDP are incopatible for Issues but it's the same type
     cdpSession.emit('Audits.issueAdded', {issue});
-    const collectedIssue = collector.getData(page)[0] as AggregatedIssue;
+    const collectedIssue = collector.getData(page)[0];
+    assert(collectedIssue instanceof AggregatedIssue);
     assert.equal(collectedIssue.code(), 'MixedContentIssue');
     assert.equal(collectedIssue.getAggregatedIssuesCount(), 1);
   });

tests/tools/console.test.ts

@@ -4,8 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import assert from 'node:assert';
-import {describe, it} from 'node:test';
+import {before, describe, it} from 'node:test';
 
+import {loadIssueDescriptions} from '../../src/issue-descriptions.js';
 import {
   getConsoleMessage,
   listConsoleMessages,
@@ -14,6 +15,10 @@ import {withBrowser} from '../utils.js';
 
 describe('console', () => {
   describe('list_console_messages', () => {
+    before(async () => {
+      await loadIssueDescriptions();
+    });
+
     it('list messages', async () => {
       await withBrowser(async (response, context) => {
         await listConsoleMessages.handler({params: {}}, response, context);
@@ -36,6 +41,28 @@ describe('console', () => {
       });
     });
 
+    it('lists issues messages', async () => {
+      await withBrowser(async (response, context) => {
+        const page = await context.newPage();
+        const issuePromise = new Promise<void>(resolve => {
+          page.on('issue', () => {
+            resolve();
+          });
+        });
+
+        await page.setContent('<input type="text" name="username" />');
+        await issuePromise;
+        await listConsoleMessages.handler({params: {}}, response, context);
+        const formattedResponse = await response.handle('test', context);
+        const textContent = formattedResponse[0] as {text: string};
+        assert.ok(
+          textContent.text.includes(
+            `msgid=1 [issue] An element doesn't have an autocomplete attribute (count: 1)`,
+          ),
+        );
+      });
+    });
+
     it('work with primitive unhandled errors', async () => {
       await withBrowser(async (response, context) => {
         const page = await context.newPage();