kwolfy / webworker-promise Goto Github PK

Bug Description

If the eventName is in the set of properties defined in Object.prototype (constructor, hasOwnProperty, isPrototypeOf, propertyIsEnumerable, toLocaleString, toString, valueOf), there would be a TypeError on this line.

This happens because, in JavaScript, objects inherit from Object.prototype by default. This can create potential issues that, although may be unlikely to happen, still represent a risk.

Fix

One of the possible fixes for this is creating the object without enabling prototype inheritance. More specifically, this would affect the __listeners property defined in TinyEmitter. Applying the fix suggested below makes the application not to crash even if the eventName is a string in the set of properties listed above.

--- TinyEmitter constructor (original) ---

class TinyEmitter {
  constructor() {
    Object.defineProperty(this, '__listeners', {
      value: {},
      ...
    });
  }
...
}

--- Tiny Emitter constructor (fixed) ---

class TinyEmitter {
  constructor() {
    Object.defineProperty(this, '__listeners', {
      value: Object.create(null),
      ...
    });
  }
...
}

Current behaviour

There is currently no validation for the maxThreads and maxConcurrentPerWorker parameters in the Worker pool. This raises to inconsistencies, such as:

• A worker being created even if maxThreads is 0
• maxThreads or maxConcurrency being assigned floating point values. If this happens, certain properties are not met. For instance, we could have maxThreads assigned to 2.5 and 3 workers created. This means that, for these particular cases, the actual number of threads could be greater than maxThreads. Maybe it would make sense to forbid this by requiring both maxThreads and maxConcurrentPerWorker to be integers greater or equal to 1.

Possible fix

--- WorkerPool Constructor (Original) ---

class WorkerPool {
  constructor({create, maxThreads, terminateAfterDelay, maxConcurrentPerWorker}) {
    ...
    this._maxThreads = maxThreads;
    ...
    this._maxConcurrentPerWorker = maxConcurrentPerWorker;
    ...
  }
...
}

--- WorkerPool Constructor (Fixed) ---

class WorkerPool {
  constructor({create, maxThreads, terminateAfterDelay, maxConcurrentPerWorker}) {
    ...
    if(!Number.isInteger(maxThreads) || !Number.isInteger(maxConcurrentPerWorker) || maxThreads < 1 || maxConcurrentPerWorker < 1){
       throw new TypeError('Expected maxThreads and maxConcurrentPerWorker to be integers greater or equal to 1');
    }
    this._maxThreads = maxThreads;
    ...
    this._maxConcurrentPerWorker = maxConcurrentPerWorker;
    ...
  }
...
}

First of all I would like to thank you for this lovely library, it, together with the webpack 5 worker loader is the first time I've felt working with web workers to be a good dev experience.

I took the liberty of creating some Typescript definitions for the project, and I would like to submit them to the DefinitelyTyped repository so that they will be automatically included for anyone who installs the package. Here is the PR: Idicious/DefinitelyTyped#1. I'd first like your input on the matter before officially submitting it.

Alternatively I could actually open up a PR in this repo and add the TS definitions directly here. Doing either of these will provide autocompletion + docs in most editors.

Hello, first of all, I want to express my gratitude for your open source repository. It has greatly enhanced the experience of working with workers.

Currently, I'm exploring options for incorporating a library that can improve the developer experience of workers for users of our apps and libraries. After some research, I have narrowed down my choices to your repository and comlink. However, I couldn't find a section in the readme that explains the differences between the two.

Would it be possible for you to provide some clarification on the distinctions between your repository and comlink? I appreciate your help. Thank you very much.

Hi there,

Thanks for maintaining this project!

I am spawning many workers and trying to collect them with Promise.all, but the return value is not expected. Is there an example I could look at to use webworker-promise with Promise.all?

Thanks,
Matt

It would be useful to support a bidirectional promise between parent and worker and also worker and parent.

For example, the worker may need to execute and wait for an operation only available to the main thread, so having exec/operation work within the context of the main thread should be supported.

I noticed that the version with typescript types is published to npm, but the type files haven't been copied from the src to lib directory, which is the type root. Copying the type files is part of the prepublish script in the repo, but perhaps you use another command in the ci tooling.

In any case, to fix this, before publishing to npm the prepublish script should be run.

Hey!

Thanks for the great package. I was eagerly awaiting throwing away my custom typings, but noted that 0.5.0 isn't published to NPM yet (latest is 0.4.4)

Cheers,
Eddie

So I'm aware that IE11 doesn't support ES6 and therefore Promises.
I'm using webpack's worker-loader to require all my libraries I need and I'm passing the workers throughbabel-loader so I can write in ES6.

I was using promise-worker prior to this but I didn't like that it JSONifies my data without any options and I found your library through an issue.

Unfortunately in your register.js file you return a new Promise and IE11 is not happy about it.
promise-worker doesn't seem to have this problem and works fine in IE11

Is there something we can do to add this compatibility?

This lib does not work for Shared WebWorkers.

const worker = new WebworkerPromise(new SharedWorker('worker.ts'));

dex.js:67 Uncaught (in promise) TypeError: this._worker.postMessage is not a function
    at index.js:67:20
    at new Promise (<anonymous>)
    at Worker.postMessage (index.js:64:12)
 

The promise returned by postMessage seems to be rejected given a message with valuenull. I would expect the promise to be resolved, as the postMessage API allows sending messages with value null.

How to reproduce the issue?

I wrote a simple client of the library, which imports WebworkerPromise and posts a message with null value. The worker handler does nothing except returning the message received from the main thread. The promise is then rejected and we get an error in the main thread, captured with the catch clause.

main.js

const WebworkerPromise = require('webworker-promise');
const worker = new WebworkerPromise(new Worker('worker.js'));

worker
  .postMessage(null)
  .then((response) => {
  	console.log('MAIN: got response '+response);
    // handle response
  })
  .catch(error => {
  	console.log('MAIN: got error '+error);
    // handle error
  });

worker.js

const registerWebworker = require('webworker-promise/lib/register');

registerWebworker(
  (message, emit) => {
    return message;
  }
);

How to fix?

The error originates in the function below, taken from register.js. When o has value null (which is exactly the case of the example shown above), we cannot access the then property without checking if o has value null.

const isPromise = o => typeof o === 'object' && typeof o.then === 'function' && typeof o.catch === 'function';

Thus, checking if o is null before accessing the then property would solve the issue and avoid the promise to be rejected. We could then have the following instead:

const isPromise = o => typeof o === 'object' && o !== null && typeof o.then === 'function' && typeof o.catch === 'function';

Hi，First of all thank you for your contribution to this project! I used websocket in webworker-promise to fetch some files, and I hoped that main.js use postMessage to send some files name to woker.js and then main.js can get files in .then() from the worker. But a problem occurred: If I send file A name first, and then send file B name, file B arrives before file A during the socket acquisition process, then I can only get file B in .then() of the main thread (File A in the worker will be received after B is received, but File A will not be sent to main.js by the worker). This sounds a bit messy, here is part of my code：
worker.js
`import registerWebworker from 'webworker-promise/lib/register';
let ws;
registerWebworker(async function (wwMessage, emit) {
return new Promise((resolve, reject) => {

    if (wwMessage.init) {
        console.log('Creating socket');
        ws = new WebSocket('ws://localhost:8888/');
        ws.binaryType = 'arraybuffer';
        ws.onopen = function () {
            console.log('Socket open.');
        }
    }

    ws.onclose = function () {
        console.log('Socket close.');
    }

    ws.onerror = function (event) {
        console.error("WebSocket error observed:", event);
    };

    if (wwMessage.data) {
        console.log("Client socket message:", wwMessage.data);
        let wsMessage = JSON.stringify(wwMessage.data);
        ws.send(wsMessage);
    }

    ws.onmessage = function (wsMessage) {
        let arrayBuffer = wsMessage.data;
        console.log('Socket server message', arrayBuffer);
        resolve(new registerWebworker.TransferableResponse(arrayBuffer, [arrayBuffer]));
    };
})})`

main.js
this.wsWorker.postMessage({ data: { fileName: this.fileName + '_' + frameIndex } }) .then(res => { use res do something... })

I hope you can tell me the reason for this problem, thank you very much！！！！

If the network is disconnected, the webworker-promise will not be processed, neither resolve nor reject

Can you provide an unminified files on release page? It's necessary when using third party scripts in browser extensions. Thank you.