Worker job queue + cancellation#100
Conversation
|
@Aditya8369 is attempting to deploy a commit to the csxark's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds cancellation and timeout support to cipher computations. ChangesWorker Cancellation and Timeout Handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CipherLayout
participant useCipherWorker
participant Worker
CipherLayout->>useCipherWorker: runCipher(options, signal)
useCipherWorker->>useCipherWorker: terminateWorkerAndRejectAll(previous requests)
useCipherWorker->>Worker: createWorker() / postMessage(payload)
useCipherWorker->>useCipherWorker: install abort listener + 10s timeout
alt signal aborted
useCipherWorker->>CipherLayout: reject(AbortError)
else timeout elapsed
useCipherWorker->>Worker: terminate()
useCipherWorker->>CipherLayout: reject(WORKER_TIMEOUT)
else worker responds
Worker->>useCipherWorker: onmessage(result)
useCipherWorker->>useCipherWorker: clear timeout, remove abort listener
useCipherWorker->>CipherLayout: resolve(result)
end
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/hooks/useCipherWorker.ts`:
- Around line 162-165: The timeout handling in useCipherWorker is using a raw
WORKER_TIMEOUT string and a plain Error, which skips the CipherError contract
and exposes an unfriendly message to CipherLayout. Update the timeout callback
to create and propagate a CipherError from errors.ts with code WORKER_TIMEOUT
and a user-facing message, and set hook error state to that message rather than
the literal code. Make sure terminateWorkerAndRejectAll and any callers continue
to receive the structured error so they can branch on code while displaying a
readable string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7f18d57-7483-43f7-af91-864ceb611d4e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
components/cipher/CipherLayout.tsxlib/hooks/useCipherWorker.tstests/unit/hooks/useCipherWorker.test.ts
| const timeoutId = setTimeout(() => { | ||
| setError('WORKER_TIMEOUT') | ||
| terminateWorkerAndRejectAll(new Error('WORKER_TIMEOUT')) | ||
| }, 10000) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Timeout rejects/sets a raw error code instead of a user-facing message, and bypasses the CipherError contract.
The hook rejects with new Error('WORKER_TIMEOUT') and sets error to the literal 'WORKER_TIMEOUT'. CipherLayout renders {error || workerError} verbatim, so users see the raw code WORKER_TIMEOUT rather than a readable message. Since errors.ts already exposes a CipherError(code, message) type carrying a WORKER_TIMEOUT code, prefer surfacing a CipherError with a friendly message so consumers can both display a sensible string and branch on code.
🛠️ Proposed change
- const timeoutId = setTimeout(() => {
- setError('WORKER_TIMEOUT')
- terminateWorkerAndRejectAll(new Error('WORKER_TIMEOUT'))
- }, 10000)
+ const timeoutId = setTimeout(() => {
+ const timeoutError = new CipherError(
+ 'WORKER_TIMEOUT',
+ 'The computation timed out after 10 seconds.'
+ )
+ setError(timeoutError.message)
+ terminateWorkerAndRejectAll(timeoutError)
+ }, 10000)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timeoutId = setTimeout(() => { | |
| setError('WORKER_TIMEOUT') | |
| terminateWorkerAndRejectAll(new Error('WORKER_TIMEOUT')) | |
| }, 10000) | |
| const timeoutId = setTimeout(() => { | |
| const timeoutError = new CipherError( | |
| 'WORKER_TIMEOUT', | |
| 'The computation timed out after 10 seconds.' | |
| ) | |
| setError(timeoutError.message) | |
| terminateWorkerAndRejectAll(timeoutError) | |
| }, 10000) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/hooks/useCipherWorker.ts` around lines 162 - 165, The timeout handling in
useCipherWorker is using a raw WORKER_TIMEOUT string and a plain Error, which
skips the CipherError contract and exposes an unfriendly message to
CipherLayout. Update the timeout callback to create and propagate a CipherError
from errors.ts with code WORKER_TIMEOUT and a user-facing message, and set hook
error state to that message rather than the literal code. Make sure
terminateWorkerAndRejectAll and any callers continue to receive the structured
error so they can branch on code while displaying a readable string.
csxark
left a comment
csxark
left a comment
There was a problem hiding this comment.
Great work @Aditya8369, and thank you for your contribution! 🎉I've reviewed the changes, and everything looks good. The requested fixes have been addressed correctly, and the implementation aligns well with the requirements. This is ready to be merged. Looking forward to your future contributions!
2 participants
Summary of Changes
lib/hooks/useCipherWorker.ts:
Refactored worker creation into a reusable createWorker helper function.
Automatically aborts any previous request currently running in the worker when a new request begins. It does this by terminating the worker thread (to stop CPU-heavy cipher jobs early) and rejecting the previous request's promise with a standard AbortError.
Supports a custom AbortSignal inside the options parameter.
Enforces a 10-second request timeout budget, raising a WORKER_TIMEOUT error if exceeded.
Clears timeout timers and removes AbortSignal listeners when requests complete (or fail/abort) to prevent memory leaks.
components/cipher/CipherLayout.tsx:
Manages an AbortController using a useRef to handle user input changes.
Aborts previous active calculations before launching new ones.
Ignores AbortError/DOMException in the catch block to prevent temporary error banners and ensure smooth transition between results.
Aborts calculations if the component unmounts or the user switches to a different cipher.
tests/unit/hooks/useCipherWorker.test.ts:
Created unit tests that mock the Web Worker environment.
Verified worker initialization, auto-aborting of overlapping runs, manual AbortSignal aborting, and the 10-second timeout. All new tests pass successfully.
closes #81
Summary by CodeRabbit