samdenty / comlink-extension

Use comlink with chrome extensions
https://www.npmjs.com/package/comlink-extension
28 stars 4 forks source link

Issues when awaiting proxy methods simultaneously #2

Open arseneyr opened 4 years ago

arseneyr commented 4 years ago

I'm having issues when calling two methods on the same proxied object and waiting on the results simultaneously. I have a minimal repro here (run yarn start and load the extension in the build folder) but here is the gist of it:

//background script

import * as Comlink from "comlink";
import { createBackgroundEndpoint, isMessagePort } from "comlink-extension";
import { browser } from "webextension-polyfill-ts";

class BackgroundEndpoint {
  getSubProxy() {
    return Comlink.proxy({
      getB: () => {
        return "B";
      },
    });
  }

  getA() {
    return "A";
  }
}

browser.runtime.onConnect.addListener((port) => {
  if (isMessagePort(port)) {
    return;
  }
  Comlink.expose(new BackgroundEndpoint(), createBackgroundEndpoint(port));
});

//content script

import { createEndpoint } from "comlink-extension";
import * as Comlink from "comlink";
import { browser } from "webextension-polyfill-ts";

const obj = Comlink.wrap(createEndpoint(browser.runtime.connect()));

obj
  .getSubProxy()
  .then((s) => s.getB())
  // s.getB() never resolves and "B" is not logged
  .then(console.log);

obj.getA().then(console.log);

The promise returned by s.getB() never resolves. The interesting thing is that changing the order of the promises (i.e. calling getA() before getSubProxy()) works fine. Also this is not a problem when using Comlink and web workers directly. Am I doing something obviously wrong here?

samdenty commented 4 years ago

Should've patched all the port creations, but maybe I left something out

If you want to debug, I'd log the messages that are sent and that should reveal more

arseneyr commented 4 years ago

I believe this is a race condition in forward():

https://github.com/samdenty/comlink-extension/blob/c909591390659683f8bff105bfa9894a34964866/src/adapter.ts#L150-L152

Specifically, the port promise can resolve after a message has already been received and is lost. I'm not too familiar with Javascript internals but I assume it's an implementation detail whether the promise is resolved first or the message is processed.

I've opened #3 to make forward() sync and callers to use callbacks instead.