-
-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Unhandled error on Executor Manager #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes unhandled error handling in the Executor Manager by ensuring that unhandled errors during job execution are properly caught and result in a retry transition instead of crashing the system.
- Fixed a typo in a test description ("retyurne" → "return")
- Added proper error handling for unhandled errors in ExecutorManager with retry mechanism
- Added comprehensive test coverage for both abort scenarios and unhandled error scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/engine/src/job/job.test.ts | Fixed typo in test description and added test for unhandled promise rejection |
packages/engine/src/execution/executor-manager.ts | Replaced error re-throwing with retry transition for unhandled errors |
packages/engine/src/execution/executor-manager.test.ts | Added tests for abort handling and unhandled error retry behavior |
Comments suppressed due to low confidence (2)
packages/engine/src/execution/executor-manager.test.ts:124
- The test is checking for
RunTransition
but the actual implementation only appliesRetryTransition
for unhandled errors. This assertion may be incorrect or testing unintended behavior.
// eslint-disable-next-line @typescript-eslint/unbound-method
packages/engine/src/job/job.test.ts:222
- [nitpick] The test description is grammatically incomplete. It should be "should return RetryResult if run throws unhandled promise rejection" or similar to be more descriptive.
sidequestTest("should return RetryResult if run unhandled promise", async () => {
# [1.0.0-next.15](v1.0.0-next.14...v1.0.0-next.15) (2025-07-30) ### Bug Fixes * author ([ec1734a](ec1734a)) * author ([e81372f](e81372f)) * Proper dashboard and Sidequest shutdown ([#16](#16)) ([ed1d853](ed1d853)) * release ([4b2f4e7](4b2f4e7)) * release ([#26](#26)) ([c6093ae](c6093ae)) * release.yml ([c37c57c](c37c57c)) * revert unlimited jobs ([#23](#23)) ([96ccc5d](96ccc5d)), closes [#21](#21) * Unhandled error on Executor Manager ([#24](#24)) ([81bd5e0](81bd5e0)) * using personal token ([#25](#25)) ([91c5834](91c5834)) * using sidequest release app ([1ecf1a7](1ecf1a7)) ### Features * `Sidequest.build` from job ([#20](#20)) ([d7f5286](d7f5286)) * unlimited jobs ([#21](#21)) ([0055f4a](0055f4a))
🎉 This PR is included in version 1.0.0-next.15 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
# 1.0.0 (2025-08-01) ### Bug Fixes * add close method to Engine class for backend cleanup ([90ece50](90ece50)) * add engines field to specify required Node.js version ([#42](#42)) ([44c9523](44c9523)) * author ([ec1734a](ec1734a)) * author ([e81372f](e81372f)) * changed to patch and fixed job path call on table ([6e2399e](6e2399e)) * claim ([b535e98](b535e98)) * deduplication ([#58](#58)) ([e0d6ae7](e0d6ae7)) * default job state ([626882e](626882e)) * enqueue ([a1cc0aa](a1cc0aa)) * ensure consistent path formatting in buildPath function ([55de875](55de875)) * escape backslashes in file paths for consistency ([36cc48c](36cc48c)) * fix build of dashboard on windows ([e92f15d](e92f15d)) * Fix Dashboard stats and graph, and removes Redis ([#154](https://github.com/sidequestjs/sidequest/issues/154)) ([b3ab912](b3ab912)) * fix second shutdown attempt ([295f6b4](295f6b4)) * fixed a few inconsistencies ([54d338d](54d338d)) * fixed time range ([fb19cac](fb19cac)) * fixed yarn lock ([#140](https://github.com/sidequestjs/sidequest/issues/140)) ([68176ec](68176ec)) * format ([c51ecf6](c51ecf6)) * Future canceled jobs not running ([#34](#34)) ([85a5212](85a5212)) * improve job failure handling in RetryTransition ([#143](https://github.com/sidequestjs/sidequest/issues/143)) ([f448c35](f448c35)) * including migrations on backends ([a670e89](a670e89)) * make config parameter optional in Engine.start method ([2359040](2359040)) * min concurrency ([#28](#28)) ([5e9877b](5e9877b)) * new version ([c104052](c104052)) * Proper dashboard and Sidequest shutdown ([#16](#16)) ([ed1d853](ed1d853)) * publish ([17d036b](17d036b)) * publish to npm ([477e9a2](477e9a2)) * queue list ([b09d103](b09d103)) * release ([4b2f4e7](4b2f4e7)) * release ([d731dd2](d731dd2)) * release ([b736f50](b736f50)) * release ([#26](#26)) ([c6093ae](c6093ae)) * release next ([66b9329](66b9329)) * release next ([dd9f2af](dd9f2af)) * release next ([cc4b2bd](cc4b2bd)) * release next ([586ec83](586ec83)) * release.yml ([c37c57c](c37c57c)) * remove topological ([f25b881](f25b881)) * remove unused delete button from job view ([#36](#36)) ([eedd15f](eedd15f)) * removing returning usage from sql backend ([5d61c29](5d61c29)) * removing version from monorepo ([e4c5a76](e4c5a76)) * removing version script ([e7736a4](e7736a4)) * revert unlimited jobs ([#23](#23)) ([96ccc5d](96ccc5d)), closes [#21](#21) * sending config to sidequest main ([44a314c](44a314c)) * serialize errors ([#44](#44)) ([683b7e1](683b7e1)) * show more and show less not saving state on page refresh ([#41](#41)) ([fcc99e7](fcc99e7)) * states ([81be666](81be666)) * test ([1da410e](1da410e)) * test ([8444aef](8444aef)) * Unhandled error on Executor Manager ([#24](#24)) ([81bd5e0](81bd5e0)) * update default concurrency value in documentation ([dfc328d](dfc328d)) * update jobDefaults handling and add engine tests ([#4](#4)) ([bbd8dde](bbd8dde)) * update logger import to use @sidequest/core ([d1f7825](d1f7825)) * update staleJobs test to handle negative timeout values ([514b330](514b330)) * url ([74651c2](74651c2)) * using exec ([80e47be](80e47be)) * using personal token ([#25](#25)) ([91c5834](91c5834)) * using sidequest release app ([1ecf1a7](1ecf1a7)) * version ([be0e7d4](be0e7d4)) * versions ([903c601](903c601)) * versions ([abd5a80](abd5a80)) * yarn lock ([d9cdc68](d9cdc68)) ### Features * `Sidequest.build` from job ([#20](#20)) ([d7f5286](d7f5286)) * add better logging and tests for createBackendFromDriver function ([#142](https://github.com/sidequestjs/sidequest/issues/142)) ([00942cb](00942cb)) * add comprehensive test suite for job and queue management ([a775de8](a775de8)) * add job builder ([#39](#39)) ([5af07bb](5af07bb)) * add JobData to job object. ([#171](https://github.com/sidequestjs/sidequest/issues/171)) ([c61df01](c61df01)) * add nullable timeout column to sidequest_jobs table ([b2bacbd](b2bacbd)) * add timeout on executor start ([5c0164d](5c0164d)) * add updateJob method and related tests for job management ([6f7a024](6f7a024)) * added cli ([442b031](442b031)) * added dasy ui ([59411e9](59411e9)) * added job view - work in progress ([e274af6](e274af6)) * added mongodb backend ([#155](https://github.com/sidequestjs/sidequest/issues/155)) ([0149d82](0149d82)) * allow nested jobs ([#55](#55)) ([a8ff7f0](a8ff7f0)) * allowing to set maxAttempts and availabeAt on enqueuing job. ([#77](#77)) ([c469da2](c469da2)) * basic auth on dashboard ([#66](#66)) ([add020b](add020b)) * cancel job ([8434c70](8434c70)) * cleanup job and coverage ([#76](#76)) ([f58f2d0](f58f2d0)) * dashboard config ([e6741d2](e6741d2)) * enhance queue management with defaults and force update options ([#5](#5)) ([40acbec](40acbec)) * Facades and better transitions ([#161](https://github.com/sidequestjs/sidequest/issues/161)) ([8066f5d](8066f5d)) * implement backend initialization and configuration for dashboard ([34737b9](34737b9)) * implement staleJobs method for job management across backends ([a10959a](a10959a)) * improve DX ([#71](#71)) ([075b589](075b589)) * job flow control ([#42](#42)) ([f7dd600](f7dd600)) * job list ([5208e7c](5208e7c)) * job view ([31f6112](31f6112)) * mock shared-runner for improved test isolation in main.test.ts ([a1bfac2](a1bfac2)) * move magic numbers to Sidequest config ([#147](https://github.com/sidequestjs/sidequest/issues/147)) ([f701f1a](f701f1a)) * moving args to run function ([225bee3](225bee3)) * realising staled claimed and running jobs ([#59](#59)) ([148f64c](148f64c)) * recurring jobs ([#12](#12)) ([cd66663](cd66663)) * redis backend ([#129](https://github.com/sidequestjs/sidequest/issues/129)) ([f08e9b7](f08e9b7)) * refactor backend import paths and introduce setTestBackend function ([f5439da](f5439da)) * refactor uniquiness ([#67](#67)) ([fbea055](fbea055)) * removed dev server ([ffe153d](ffe153d)) * rerun jobs and fixing small issues ([6e2925c](6e2925c)) * routines ([6abf0d4](6abf0d4)) * run from dashboard ([b65b058](b65b058)) * unlimited jobs ([#21](#21)) ([0055f4a](0055f4a)) * Update branch triggers to include 'develop' for workflows ([#1](#1)) ([eef8a35](eef8a35)) * Update job arguments type to array in SidequestDashboard ([a920c27](a920c27)) ### Reverts * Revert "Chore/fix env ([#166](https://github.com/sidequestjs/sidequest/issues/166))" ([#167](https://github.com/sidequestjs/sidequest/issues/167)) ([6254959](6254959))
🎉 This PR is included in version 1.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Checklist for Pull Requests
yarn test:all
)Summary of Changes
Closes #19
Also reintroduces timeout