Feature request: Automatically reject pending requests when proxy is released

[sampbarrow]

As it stands (as far as I can tell) comlink does not reject previous requests when closed, so if a proxy is released while you are waiting on a response to a previous call, there is no error, the promise just never resolves. There's no real way to deal with this outside of comlink except to track the state of the worker manually and add something to every function call to reject pending requests when the worker is closed.

Would be fairly simple to add by doing the following:

• When postMessage is called, add the reject(e) callback from the promise to an array.
• Remove this callback from the array on completion.
• When the proxy is released, loop over the array and call all of the rejection callbacks with an error message.

function requestResponseMessage(
  ep: Endpoint,
  msg: Message,
  transfers?: Transferable[],
  rejectors: ((e: unknown) => void)[], **<-- ADD**
): Promise<WireValue> {
  return new Promise((resolve, reject) => { **<-- ADD**
    rejectors.push(reject) **<-- ADD**
    const id = generateUUID();
    ep.addEventListener("message", function l(ev: MessageEvent) {
      if (!ev.data || !ev.data.id || ev.data.id !== id) {
        return;
      }
      ep.removeEventListener("message", l as any);
      resolve(ev.data);
      rejectors.splice(rejectors.indexOf(reject), 1) **<-- ADD**
    } as any);
    if (ep.start) {
      ep.start();
    }
    ep.postMessage({ id, ...msg }, transfers);
  });
}

function createProxy<T>(
  ep: Endpoint,
  path: (string | number | symbol)[] = [],
  target: object = function () {}
): Remote<T> {
  let isProxyReleased = false;
  let rejectors = new Array<(e: unknown) => void>() **<-- ADD**
  const proxy = new Proxy(target, {
    get(_target, prop) {
      throwIfProxyReleased(isProxyReleased);
      if (prop === releaseProxy) {
        return () => {
          rejectors.forEach(rejector => rejector("This proxy has been released.")) **<-- ADD**
          return requestResponseMessage(ep, {
            type: MessageType.RELEASE,
            path: path.map((p) => p.toString()),
          }).then(() => {
            closeEndPoint(ep);
            isProxyReleased = true;
          });
        };
      }

Of course anytime requestResponseMessage is called, the rejectors array would need to be passed (didnt bother to include).

[sampbarrow]

I went ahead and did this but I realized there was another similar issue. The proxy is not marked as released until after a response is received to the release message. For whatever reason, this is not happening for me - but in any case it also represents another possibility for a similar issue (promises never resolving since this message is asynchronous and theoretically you could have another pending request come in before it's received). To fix, I added a variable isProxyPendingRelease, set to true before the release message is sent, along with calling all the rejection callbacks, and replaced all throwIfProxyReleased(isProxyReleased) call with throwIfProxyReleased(isProxyReleased || isProxyPendingRelease)

get(_target, prop) {
      throwIfProxyReleased(isProxyReleased || isProxyPendingRelease);
      if (prop === releaseProxy) {
        return () => {
          isProxyPendingRelease = true;
          rejectors.forEach(_ => _("This proxy is pending release."))
          return requestResponseMessage(ep, {
            type: MessageType.RELEASE,
            path: path.map((p) => p.toString()),
          }, rejectors).then(() => {
            closeEndPoint(ep);
            isProxyReleased = true;
          });
        };
      }

[benjamind]

Would you be interested in making a PR for this? Seems like a useful edge case to close.

[sampbarrow]

Would you be interested in making a PR for this? Seems like a useful edge case to close.

I did make a fork but I can't seem to find the code now, let me try to dig it up.