Transfer handlers broke after upgrading to v4

[guanzo]

I'm encountering a bug that appeared after migrating from v3 to v4. It's related to babel's transpilation and proxying callback functions with Comlink.proxy, (Comlink.proxyValue in v3).

Some context:

I declared a transfer handler for Error

export function setTransferHandlers () {
    const errorTransferHandler = {
        canHandle (obj) {
            return obj instanceof Error
        },
        serialize (obj) {
            return [cloneErrorObject(obj), []]
        },
        deserialize (obj) {
            return Object.assign(Error(), obj)
        }
    }

    Comlink.transferHandlers.set('ERROR', errorTransferHandler)
}

Here's a block of my source code where the bug occurs.

const resolved = Promise.resolve()
function nextTickUnthrottled (fn, ...args) {
	resolved.then(() => {
		fn(...args)
	})
}

By this point, fn has been passed thru Comlink.proxy. In my debugging tests, arg is a single Error object. After the fn is called with the Error arg, I expect my error transfer handler to handle it, which it does if my source code is running in the browser. That is, I expect the below func to return true.

canHandle (obj) {
    return obj instanceof Error
},

However, since i'm using babel, this is the code that runs in the browser.

function nextTickUnthrottled (fn) {
        for (var _len = arguments.length, args = new Array(_len > 1 ? _len - 1 : 0), _key = 1; _key < _len; _key++) {
        	args[_key - 1] = arguments[_key];
        }
        resolved.then(function () {
        	fn.apply(void 0, args)
        })
    }

The important bit is fn(...args) -> fn.apply(void 0, args). This results in a different code path for the proxy. Instead of the apply trap being called (which happens with my source), it results in the get trap being called, and THEN the apply trap.

The bug: After upgrading to v4, the obj param in canHandle(obj) is a single element array with the error obj, instead of the error obj itself.

Here's a snippet of comlink's source with my comments to illustrate the differing code path:

function createProxy(ep, path = []) {
    const proxy = new Proxy(new Function(), {
	// BABEL CODE calls 'get' first which returns a new proxy,
        // then the `apply` trap is called on the new proxy, 
        // but my error object has been inserted into an array
        get(_target, prop) {
            ...
            return createProxy(ep, [...path, prop]);
        },
		// SOURCE CODE calls 'apply' first
        apply(_target, _thisArg, rawArgumentList) {
            ...
        },
    });
    return proxy;
}

Let me know if you need more info.
Is this something comlink can work around?

[surma]

Hm, I might be able to fix that on Comlink’s side.

Can you give me a minimal setup in a gist or something that reproduces this bug? Then I can turn it into a test.

[guanzo]

Here: http://next.plnkr.co/edit/jKM1kjMrb43XdbWM

Check the console for errors.

[surma]

Looking at this now

[surma]

After digging into this a little bit, I don’t think I will fix this from Comlink’s side.

The biggest change from v3 to v4 was to remove object traversals that were pretty costly, especially on low-end phones. To fix this, I’d have to either re-introduce traversals or give some special handling to apply (and maybe even call). A quick prototype for that showed that this requires a good amount of code for something that I think is a rather niché problem.

That being said, you can work around this by giving your transfer handle the capability to traverse objects:

Comlink.transferHandlers.set("ERROR",  {
  canHandle (obj) {
      if(Array.isArray(obj)) {
        return obj.some(el => this.canHandle(el));
      }
      return obj instanceof Error
  },
  serialize (obj) {
      if(Array.isArray(obj)) {
        return [obj.map(el => this.serialize(el)), []];
      }
      const message = obj && obj.message
      const stack = obj && obj.stack
      obj = Object.assign({}, obj, { message, stack })
      return [obj, []]
  },
  deserialize (obj) {
      if(Array.isArray(obj)) {
        return obj.map(el => this.deserialize(el));
      }

      return Object.assign(Error(), obj)
  }
});

(Demo & Code: https://comlink-issue-284.glitch.me/)

I’m closing this, but please re-open if you disagree with my decision/reasoning :)