ruifigueira / playwright-crx

Playwright for chrome extensions
Apache License 2.0
81 stars 15 forks source link

Utilizing CDPSession - ERR: "background.js:135091 No tabId provided for Target.attachToBrowserTarget" #13

Closed ekingunoncu closed 6 months ago

ekingunoncu commented 6 months ago

Hello,

first of all, thank you very much for your efforts in creating this tool it will be a great fit for my project.

The issue is that I need to utilize CDPSession which gives me the error while creation, the error looks like:

background.js:135091 No tabId provided for Target.attachToBrowserTarget

I thought it might be related to https://github.com/ruifigueira/playwright-crx/issues/1 but then I tried the solution from there it didn't help.

chrome.action.onClicked.addListener(async ({ id: tabId }) => { await chrome.action.disable();

const crxApp = await crx.start({ slowMo: 500 }); const page = await crxApp.attach(tabId!).catch(() => crxApp.newPage()); try { await page.goto("https://www.example.com/", { waitUntil: "load" }); const context = page.context(); const client = await context.newCDPSession(page); console.log("CDP Session:", client); await createTodos(page); } finally { await crxApp.close(); await chrome.action.enable(); } });



About CDPSession: 
[https://playwright.dev/docs/api/class-cdpsession](https://playwright.dev/docs/api/class-cdpsession)

How can we fix this issue? I will continue inspecting to find some fix for this one. 

Thank you very much!

Best regards,
ruifigueira commented 6 months ago

Hi @ekingunoncu

currently creating a new CDP session is not supported, but you can run CDP commands direcly with chrome extension API: https://developer.chrome.com/docs/extensions/reference/api/debugger

I even use that exact same API internally (check crxTransport.ts).

ekingunoncu commented 6 months ago

Hello, Thank you very much Rui, for your quick response,

I will look to create a solution to reach CDP through playwright because I want my app to behave the same for both Nodejs and Chrome Extension environments.

Will creating CrxCDPSession and integrate it CrxTransport help me to achieve this?

If I won't understand the logic or if it doesn't work out for me I will create an interface for GDPSession and its send method to have different implementations for both NodeJS and Chrome Extension

ruifigueira commented 6 months ago

Sorry for not replying sooner. I was trying to provide support for context.newCDPSession without patching playwright but it's harder than I thought because playwright tries to create a new session.

Creating a CrxCDPSession would work but that most likely requires that we change playwright code directly, and that's something I'm trying to avoid.

Maybe you can abstract CDP session access on your app, and delegate to playwright CDP session when in nodejs mode and to chrome.debugger when in Chrome Extension mode.

ekingunoncu commented 6 months ago

Hello Rui,

I hope this message finds you well. I've been making some headway with the CDPSession issue and I wanted to share the progress with you, especially since your insights have been incredibly helpful.

After looking into the possibility of supporting the newCDPSession, I completely agree with you. It does seem like a hefty task to make it available through context.newCDPSession, and perhaps not the most efficient route.

Taking your suggestion to heart, I've gone ahead and abstracted CDP session access to work seamlessly across both NodeJS and Chrome Extension environments. Here's what I've put together:

export abstract class CDPS {
  abstract send(method: string, params?: Record<string, unknown>): Promise<any>;
}

export class BrowserCDPS extends CDPS {
  private static instances: Map<number, CDPS> = new Map();
  private tabId: number;

  private constructor(tabId: number) {
    super();
    this.tabId = tabId;
  }

  public static async getInstance(): Promise<CDPS> {
    const tabId = (await ChromeUtils.getCurrentTab()).id!;
    if (!this.instances.has(tabId)) {
      const instance = new BrowserCDPS(tabId);
      this.instances.set(tabId, instance);
    }
    return this.instances.get(tabId)!;
  }

  async send(method: string, params = {}): Promise<any> {
    return chrome.debugger.sendCommand({ tabId: this.tabId }, method, params);
  }
}

export class ChromeUtils {
  public static async getCurrentTab() {
    let queryOptions = { active: true, lastFocusedWindow: true };
    let [tab] = await chrome.tabs.query(queryOptions);
    return tab;
  }
}

export class NodeCDPS extends CDPS {
  private page?: Page;
  private cdpSession?: CDPSession;
  private static instance?: NodeCDPS;

  private constructor(page: Page) {
    super();
    this.page = page;
  }

  public static async getInstance(): Promise<CDPS> {
    if (!NodeCDPS.instance) {
      const nodeTestContext = await NodeContext.getInstance();
      NodeCDPS.instance = new NodeCDPS(nodeTestContext.getPage()!);
      NodeCDPS.instance.cdpSession = await NodeCDPS.instance
        .page!.context()
        .newCDPSession((NodeCDPS.instance.page! as any));
    }
    return NodeCDPS.instance;
  }

  async send(method: any, params = {}) {
    if (!this.cdpSession) {
      throw new Error("CDP Session not initialized. Call attach() first.");
    }
    return await this.cdpSession.send(method, params);
  }
}

I think we can close this 🎉

Best regards,

Ekin Gün Öncü

ruifigueira commented 6 months ago

That's great!

If you want to have the CDPS.send method typified exactly as CDPSession.send you can do the following trick:

import type { CDPSession } from 'playwright-crx/test';

type CDPSessionSendFunction = CDPSession['send'];

export abstract class CDPS {
  send: CDPSessionSendFunction;
}

and then, in your CDPS.send implementations, for instance BrowserCDPS) just do the following trick:

  // @ts-ignore
  send: CDPSessionSendFunction = async (method, params = {}) => {
    return chrome.debugger.sendCommand({ tabId: this.tabId }, method, params);
  };

With that, if can have proper autocompletion in vscode:

image