Fix unhandled promise rejections in workflow error handling - #1
Conversation
Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
- Add .catch() handler to exec() when not using pool to prevent unhandled rejections - Add .catch() handler to donePromise to prevent unhandled rejection warnings - Add tests to verify error handling works correctly without crashing Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes unhandled promise rejection warnings that occur during workflow enqueue failures when no pool is used. The fix ensures that promise rejections in the exec() function are properly caught and handled by attaching error handlers.
Key changes:
- Added
.catch()handler toexec()call when no pool is used - Added a no-op catch handler to
donePromiseinWorkflowJobconstructor to prevent unhandled rejection warnings - Added comprehensive tests for enqueue failure scenarios without pool
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/workflow.ts | Added .catch() handler to exec() when no pool is used; added no-op catch to donePromise to suppress warnings |
| test/enqueue-failures.spec.ts | Added two new test cases covering enqueue failures without pool |
| dist/workflow.js | Compiled JavaScript output reflecting the source changes |
| dist/workflow.js.map | Updated source map for compiled JavaScript |
| dist/workflow.d.ts.map | Updated TypeScript declaration source map |
| bun.lock | Updated @types/bun dependency version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| await wf.run(api as any); | ||
| // If we reach here, the test should fail because run() should have thrown | ||
| expect(true).toBe(false); // Force test failure if no error was thrown |
There was a problem hiding this comment.
Instead of using expect(true).toBe(false) to force a test failure, use expect.fail() or simply omit this line—the test will naturally fail if no exception is thrown since the catch block won't be entered. Alternatively, restructure with await expect(wf.run(api as any)).rejects.toBeInstanceOf(EnqueueFailedError) for more idiomatic testing of async rejections.
| expect(true).toBe(false);// Force test failure if no error was thrown | |
| expect.fail();// Force test failure if no error was thrown |
When workflow enqueue operations fail, unhandled promise rejections crash the process. This violates the design goal of providing a robust library for server-side usage that gracefully handles all errors.
Changes
workflow.ts
.catch()handler toexec()invocation when not using a pool (line 417) - the async function was previously called without error handling.catch()todonePromiseon creation (line 63) - prevents runtime from detecting unhandled rejection when promise is rejected before user code awaitsdone()test/enqueue-failures.spec.ts
workflow.run()handles enqueue failures without process crashesfailedevents are emittedExample
Before this fix, the following would crash the process:
After the fix, errors are properly caught and the process remains stable.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
xbun test(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
Created from VS Code via the GitHub Pull Request extension.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.