Deno: Workers crash silently

[josephrocca]

// file: main.js
let worker = new Worker("./worker.js");
worker.postMessage("hello");
console.log("finished main thread");

// file: worker.js
onmessage = (e) => {
  console.log("worker recieved message", e.data);
  new DoesNotExist();
  console.log("end worker");
}

$ deno run main.js
finished main thread
worker recieved message hello

There should be a crash message here, right? Here's what Chrome says:

finished main thread
worker recieved message hello
Uncaught ReferenceError: DoesNotExist is not defined
    at onmessage (worker.js:3)

[afinch7]

We still need to implement onerror for workers.

[bartlomieju]

Looks like onerror and onmessageerror are not yet implemented for Workers. When this script is run with -D flag one can see exception is emitted from V8 but not handled.

EDIT: @afinch7 was quicker. Andy will you work on that or can I take a stab?

[afinch7]

If you want to take a stab that's fine. I took a quick look at this one here are a few notes:

1. My types in the typescript implementation could really use some DRYing up maybe something like this:

export interface ErrorEvent {
  message: string;
  filename: string;
  lineno: number;
  colno: number;
  // TODO find a way to pass the error value.
}

export interface MessageEvent {
  data: any;
  // TODO add other values from spec to this interface when/if we have a way to pass them
}

export interface Worker {
  onerror?: (event: ErrorEvent) => void;
  onmessage?: (event: MessageEvent) => void;
  onmessageerror?: (event: MessageEvent) => void;
  postMessage(data: any): void;
  closed: Promise<void>;
}

1. You could easily use some accessors and resolveables to prevent messages from being eaten before you have a listener:

export class WorkerImpl implements Worker {
  private readonly rid: number;
  private isClosing: boolean = false;
  private readonly isClosedPromise: Promise<void>;
  private onerrorSet: Resolvable<void> = createResolvable();
  private onmessageFn?: (event: MessageEvent) => void;

  constructor(specifier: string) {
    this.rid = createWorker(specifier);
    this.runOnmessage();
    this.isClosedPromise = hostGetWorkerClosed(this.rid);
    this.isClosedPromise.then(
      (): void => {
        this.isClosing = true;
      }
    );
  }

  set onmessage(fn: (event: MessageEvent) => void)  {
    this.onmessageFn = fn;
    this.onmessageSet.resolve();
  }

  private async runOnmessage(): Promise<void> {
    // Ensure we don't start accepting until we have a listener.
    await this.onmessageSet;
    while (!this.isClosing) {
      const data = await hostGetMessage(this.rid);
      if (data == null) {
        log("worker got null message. quitting.");
        break;
      }
      // Maybe a resolvable for the function might be a better option
      if (this.onmessageFn) {
        const event = { data };
        this.onmessageFn(event);
      }
    }
  }
}

1. In the standard onerror, onmessage, and onmessageerror and their respective triggers are all independent, but they could be combined and split at some point and sent through one channel to avoid needing to define multiple message channels. Maybe combine worker messages as union types in flatbuffers: WorkerPostEvent, WorkerGetEvent, HostPostEvent, and HostGetEvent.

[bartlomieju]

You could easily use some accessors and resolveables to prevent messages from being eaten before you have a listener:

I'm not sure it's desired behavior - I just checked implementation in chrome:

// main.js
let worker = new Worker("./worker.js");
worker.postMessage("hello");
console.log("finished main thread");
setTimeout(() => {
  worker.postMessage("hello after timeout");
}, 2000);

// worker.js
setTimeout(() => {
  onmessage = (e) => {
    console.log("worker recieved message", e.data);
    console.log("end worker");
  };
}, 1000);



md5-9e2a472b4ee6dfad4b5f026a18db0ef5



finished main thread
worker recieved message hello after timeout
end worker

Looks like Chrome implementation consumes messages if there is no handler at the moment

Didn't understand concept with resolvable properly - it's on the worker class not in the worker