Possible bugs in the code.

[iSuslov]

While I'm rewriting the code to Typescript, I found two things that I would rather ask here since it looks like bugs.

1. Lets take a look here:

https://github.com/josdejong/workerpool/blob/8219f7deaf535310cb8c3f273cda8e047959502c/src/WorkerHandler.js#L483-L494

On the line 490 there is a assignment resolver.promise.timeout = timeout;. Doc of the method says timeout param is a number, but Promise object has a method called timeout. Seems like an attempt to override a method with a number. Task object however has a property timeout:number. Luckily it seems nowhere in the code this method WorkerHandler.prototype.terminateAndNotify gets called with the second argument.

Is it a bug in abandoned code?

2. Second one. Lets take a look at how TERMINATE_METHOD_ID is used.

https://github.com/josdejong/workerpool/blob/8219f7deaf535310cb8c3f273cda8e047959502c/src/WorkerHandler.js#L11

https://github.com/josdejong/workerpool/blob/8219f7deaf535310cb8c3f273cda8e047959502c/src/WorkerHandler.js#L450-L454

There is no problem when it gets directly sent to a worker, however when it gets added to the requestQueue there is a problem, because this is how requestQueue is being processed:

https://github.com/josdejong/workerpool/blob/8219f7deaf535310cb8c3f273cda8e047959502c/src/WorkerHandler.js#L272-L278

Looks like instead of TERMINATE_METHOD_ID undefined gets sent. Correct me if I'm wrong please.

[josdejong]

Good finds Ivan!

1. I think this is indeed a bug. The method terminateAndNotify is called with the second timeout argument only from the public Pool.terminate method. I commented the line resolver.promise.timeout = timeout;, but still all tests pass. I expect that the terminate timeout simply doesn't work and isn't tested. The solution most likely is to change that line to resolver.promise.timeout(timeout).

2. I think you're right, it should be something like this.requestQueue.push({ message: TERMINATE_METHOD_ID });.

I think it's best to address these bugs separately from the refactor and make sure they are properly tested.

[Michsior14]

Both bugs have been resolved :)

[josdejong]

Both issues are fixed now in v6.4.2