Repository navigation
fix: recover runtime task function operations - #202
Conversation
Port poolifier/poolifier#3369 at 23357872, retaining synchronous Web lifecycle safeguards.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8768f49ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = await operation() | ||
| await this.retryFailedWorkerTaskFunctionBootstraps() |
There was a problem hiding this comment.
When bootstrap fails because a captured function cannot be installed (for example, its serialized source evaluates to null), calling removeTaskFunction() for that exact function cannot recover the pool: the worker never installed it, so it rejects the remove request, and await operation() throws before the pool deletes the captured registration or retries restoration. The worker therefore remains unavailable and every subsequent removal repeats the same failure; removal should be able to discard a pool-side registration that is absent specifically because this worker's bootstrap failed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and shared with the pinned upstream implementation rather than introduced by the port. Fresh real-worker reproduction on Deno 2.9.7, Bun 1.4.2 and upstream #3369 23357872ffd4737198edf2657bbb96778af22654 using Node v26.12.0 threads:
- Preregister
badwith a callable whosetoString()returns the stringnull, then start a fixed one-worker pool. Real worker validation rejects replay. - Two successive public removals both reject with the genuine negative ACK (
Unknown error); captured registration remains and readiness stays false. - Corrective public ADD succeeds on the same worker; execution returns 43 and readiness becomes true.
No ACK/readiness is fabricated. The shared queue retries restoration only after await operation() succeeds; the failed remove never deletes captured state. The existing #199 success-after-correction scenario is fixed, but deleting the very absent registration needs an additional upstream recovery policy. Leaving this thread open and recording the limitation in the PR description; no independent algorithm change is silently added to this faithful port.
There was a problem hiding this comment.
Upstream report filed: poolifier/poolifier#3370. The port remains unchanged and faithful to upstream #3369; this recovery-policy change is not folded into PR #202.
|
PR Checklist
Does this PR introduce a breaking change?
Changes
Ports poolifier/poolifier#3369 at
23357872ffd4737198edf2657bbb96778af22654:undefined.errorwithout rejecting the successful mutation.Closes #198, closes #199, closes #200. Targets the current contracts described in the amended port issues.
Fidelity and Web boundaries
AST comparisons confirm the same 12 changed class members as upstream and seven structurally identical copied members. Single-worker replay and cohort ACK handling remain unchanged.
Retain UUIDs, numeric protected hooks, EventTarget errors and two-phase captured-owner exit tracking. The captured-owner iterator needs a membership check when retiring unsent peers. Defer settlement until synchronous dispatch unwinds so a Web exit cannot mask a subsequent send error. Two additional regressions cover these necessary adaptations. No new exports/options or README changes.
Verification
a8768f49edbbb6c3c3c2418b755bd3ad86633181: 15/15 passed.Known shared limitation
Removing the exact registration whose replay failed still rejects when the worker never installed it. Reproduced in the port and pinned upstream; corrective ADD succeeds. Validated review finding remains open. No separate recovery algorithm is introduced here.