fix: handle outdated message in channel queue by jedlikowski · Pull Request #184 · un-ts/synckit (original) (raw)

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

What's the issue here? Can you provide more information? Reproduction or failing test for example.

Hi, sorry, it was meant to be a draft PR ;) I've linked in the description to the issue this PR means to fix.

Hi, I've added one more change to avoid sending the worker task result to parent if parent isn't waiting for the result anymore. Let me know what you think of this PR and I'll be happy to carry it to the end :)

I also wondered how we could pass some sort of EventEmitter to the task function itself, to let it know that the task has been aborted. This would allow it for example to cancel a HTTP request, but I couldn't figure out a way to do it without changing the library API.

This chart aims visualize the current behavior (dotted line means Atomics.wait() timeout on parent process):

While this one shows the updated behavior:

Hi, @JounQin, what do you think of this PR? Would you be interested in such a contribution?

Hi @JounQin, could you please take a look at this PR?

Interesting, is that possible to add a related test case?

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (35a89ea) to head (44881df).
Report is 1 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@ ## main #184 +/- ##

Coverage 100.00% 100.00%

Files 1 1
Lines 204 218 +14
Branches 101 105 +4

Hits 204 218 +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Could all lines except if (expectedId !== id) be test covered?

Could all lines except if (expectedId !== id) be test covered?

I've been trying to achieve that. AFAIK these are the only other lines not covered:

if (id < expectedId) { const waitingTime = Date.now() - start return receiveMessageWithId( port, expectedId, waitingTimeout ? waitingTimeout - waitingTime : undefined, ) }

Adding coverage for them in a stable manner would require us to mock receiveMessageOnPort so that it returns expectedId - 1 on first call and expectedId on second call. This is currently not possible in jest in ESM environment, because partial module mocking is not supported. Due to lack of support, it would be required to mock the whole node:worker_threads module 😞

Good news @JounQin, I've managed add tests for those lines 😄 Could you take a look?

Hi @JounQin, could you also add hacktoberfest label to this PR? 😄

Thanks for your contribution!

JounQin added a commit to fisker/synckit that referenced this pull request

May 30, 2025