comlink icon indicating copy to clipboard operation
comlink copied to clipboard

Unable to `Function.apply` on proxied object

Open andylizi opened this issue 3 years ago • 9 comments
trafficstars

worker.js

importScripts("https://unpkg.com/comlink/dist/umd/comlink.js");
// importScripts("../../../dist/umd/comlink.js");

const obj = {
  foo(...args) {
    console.log(this);
    const list = this.bar();
    console.log(list);
    list.push(...args);
    return list;
  },
  bar() {
    return [1, 2];
  }
};

Comlink.expose(obj);

main.js

import * as Comlink from "https://unpkg.com/comlink/dist/esm/comlink.mjs";
// import * as Comlink from "../../../dist/esm/comlink.mjs";

async function init() {
  const obj = Comlink.wrap(new Worker("worker.js"));
  // const list = await obj.foo(3, 4);           // this is fine
  // const list = await obj.foo(...[3, 4]);      // this is fine
  const list = await obj.foo.apply(obj, [3, 4]); // this is not
  console.log(list);
}

init();

Running this code will result in:

Proxy {length: 0, name: 'target', prototype: {…}}
Promise {<pending>}
Uncaught (in promise) TypeError: list.push is not a function
    at Proxy.foo (worker.js:9:10)
    at callback (comlink.js:100:50)

While I can workaround this using the spread syntax, Babel and other transpiler will desugar it to obj.foo.apply(obj, [1, 2]) again.

andylizi avatar Feb 24 '22 12:02 andylizi

I encountered this same problem. I was attempting to pass an object as an argument to an exposed function but one of the object properties was function. I tried proxying the function to discover comlink will not peek into the object to look for proxied values (is this something comlink could do?). e.g. I had something like this

First in the iframe

import { expose, windowEndpoint } from "comlink";

const someFunc = (namedArgs: { foo: string; bar: Function }) => { ... };

expose(someFunc, windowEndpoint(self.parent));

In the parent frame

import { proxy, windowEndpoint, wrap } from "comlink";
import { isFunction, mapValues } from "lodash-es";

const api = wrap(windowEndpoint(frame.contentWindow));

api(mapValues(input, value => isFunction(value) ? proxy(value) : value));

and was met with

DOMException: Failed to execute 'postMessage' on 'Window': function () { ... } could not be cloned.

After peeking at the source code it looked like comlink was not opening up my argument to check for proxied values. As a workaround I figured I could pass the key value pairs as arguments and then reassemble the object on the other side.

in the iframe

expose((...args) => someFunc(fromPairs(chunk(args, 2))), windowEndpoint(self.parent));

back in the host

const args = Object.entries(input)
  .map(([prop, value]) => [prop, isFunction(value) ? proxy(value) : value])
  .flat();

api(...args);

Which is when I ran into this issue. In my case, as @andylizi pointed out, this is being transpiled to api.apply(api, ...). In order to get this to work I need to explicitly pass the key value pairs.

api("foo", input.foo, "bar", proxy(input.bar));

And then my code to reassemble the object on the other side works.

To sum things up there are 2 problems here.

  1. comlink does not check for proxied values in objects
  2. proxiedValue.apply does not work, which seems to be how Babel transpiles spread arguments

daniel-nagy avatar Mar 07 '22 00:03 daniel-nagy

The cause of the issue is the apply prototype method is being proxied in the get handler here https://github.com/GoogleChromeLabs/comlink/blob/36d7bfae84f4fcacf2e6d922701dae331eece071/src/comlink.ts#L406

This has the side-effect of calling the apply handler with all arguments to the apply method (including the this arg). If I add the following conditional to the get handler it works for both apply and call.

if (typeof target === "function" && prop in Function.prototype) {
  return Reflect.get(target, prop);
}

daniel-nagy avatar Mar 07 '22 02:03 daniel-nagy

import { expose, ProxyMarked } from 'comlink';
import { OmitProperties } from 'ts-essentials';

type AnyCallback = (...params: any[]) => any;

type ProxiedParam<T> = T extends AnyCallback
  ? ProxyMarked & T
  : T extends { [x: string]: any }
  ? OmitProperties<T, AnyCallback>
  : T;

type ProxiedParams<T extends any[]> = T extends [infer F, ...infer R]
  ? [ProxiedParam<F>, ...ProxiedParams<R>]
  : T extends [infer F]
  ? ProxiedParam<F>
  : [];

export type ProxiedCallback<T extends AnyCallback> = (
  ...params: ProxiedParams<Parameters<T>>
) => ReturnType<T>;

type ProxiedApi<T> = {
  [K in keyof T]: T[K] extends AnyCallback ? ProxiedCallback<T[K]> : T;
};

/**
 * This ensures that the parameters for each method on the given API
 * are proxied correctly so that they can be accessed within a Web Worker
 * (using the Comlink library).
 *
 * There are two requirements for callback arguments to work when passed to a worker:
 *   1. The callback must be wrapped with Comlink.proxy()
 *   2. The callback must not be within an object, rather passed as a top-level argument
 *      to the exposed API method.
 *
 * This helper function ensure both conditions hold by enforcing proper types.
 */
export function exposeSafeWorkerApi<T>(api: T) {
  expose(api);

  return api as ProxiedApi<T>;
}

In case someone wants to make sure a function cannot be passed inside an object.

forabi-cosuno avatar Sep 14 '22 13:09 forabi-cosuno

For what it is worth I ran into quite a few problems using Comlink. In an attempt to solve these problems I created @boulevard/transporter which snowballed into a server-client architecture. We were successfully using Transporter in React Native to bridge a Webview but that project was postponed and we are not currently using Transporter so I have stopped working on it. We may use it for another project but that work hasn't started yet.

I've also recently discovered tgrid which looks interesting though I haven't experimented with it yet. It is another server-client architecture with RPC capabilities. It looks like it has transports for Web Workers and Web sockets but I don't see anything for iframes.

A couple interesting features of Transporter is pub-sub using observables and the ability to pass functions as values. This does make Transporter connection-oriented and stateful which increases complexity.

daniel-nagy avatar Sep 14 '22 14:09 daniel-nagy

another example of the same issue in case it helps making things clearer

main.js

import * as Comlink from "https://unpkg.com/comlink/dist/esm/comlink.mjs";

const worker = Comlink.wrap(new Worker('worker.js'));
const invokeOnMain = (cb) => {
    // spoiler alert, we wont get here
    console.log('got to main thread with callback arg', cb);
};
worker.withInvokeOnMain(Comlink.proxy(invokeOnMain));

worker.js

importScripts("https://unpkg.com/comlink/dist/umd/comlink.js");

async function withInvokeOnMain(invokeOnMain) {
    const callbackArg = Comlink.proxy(() => { console.log('im callback') })
    // use Function.prototype.apply instead of invoking the function directly
    invokeOnMain.apply(null, [callbackArg])
}

Comlink.expose({ withInvokeOnMain });

i can confirm the issue is coming from Comlink's proxy get trap not taking into account function's prototype apply. as result when the fn.apply function is invoked the proxy apply trap is triggered with rawArguments as the apply function's arguments (meaning the thisArgument is first argument, and second argument is an array of the original function's arguments)

this eventually cause function arguments not to be serialized

lirancr avatar Nov 13 '22 07:11 lirancr

For anybody coming to this thread, I've created a fork with the fix suggested by @daniel-nagy in https://github.com/GoogleChromeLabs/comlink/issues/579#issuecomment-1060122519. The fork is available at https://github.com/batmilkyway/comlink and it has a prepare script so you can just point to the github repo in your package.json.

batmilkyway avatar Dec 03 '22 17:12 batmilkyway

@batmilkyway any chance of making a PR?

ivancuric avatar Mar 16 '23 15:03 ivancuric

Any update on this issue please? seems the problem is still affecting apply().

isorna avatar Aug 02 '23 13:08 isorna

Hi everyone, I hope you had a happy New Year! 🎊

If anyone is interested I recently made some updates to Transporter. If you want to learn more check out my blog post or head over to the GitHub Repo!

daniel-nagy avatar Jan 02 '24 20:01 daniel-nagy