better handling for stuck tasks: clear hanging tasks and avoid running tasks that were scheduled for a long time ago #2
Conversation
…g tasks that were scheduled for a long time ago
There was a problem hiding this comment.
Pull Request Overview
This PR improves task handling by implementing better timeout mechanisms to prevent stuck tasks. It introduces two new timeout features: scheduling timeouts for tasks that wait too long to start, and execution timeouts for tasks that run too long.
- Adds
schedulingTimeoutAtandtimeoutAtfields to handle different types of task timeouts - Implements automatic expiration of timed-out tasks with new status types (
timed_out,scheduling_timed_out) - Refactors repeating task logic into a shared helper function
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/taskSchema.js | Core implementation of timeout handling, new schema fields, and refactored task execution logic |
| test/task.test.js | Comprehensive test coverage for timeout scenarios and repeating task behavior |
| package.json | Added ESLint dependencies for code quality |
| eslint.config.js | ESLint configuration for the project |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| previousTaskId: task._id, | ||
| originalTaskId: task.originalTaskId || task._id, | ||
| timeoutMS: task.timeoutMS, | ||
| schedulingTimeoutAt: scheduledAt.valueOf() + 10 * 60 * 1000 |
There was a problem hiding this comment.
Magic number 10 * 60 * 1000 (10 minutes) is repeated multiple times. Consider extracting this into a named constant like DEFAULT_SCHEDULING_TIMEOUT_MS to improve maintainability.
| { status: 'in_progress', startedRunningAt: now, ...additionalParams }, | ||
| { | ||
| status: 'in_progress', | ||
| timeoutAt: new Date(now.valueOf() + 10 * 60 * 1000), // 10 minutes from startedRunningAt |
There was a problem hiding this comment.
Same magic number 10 * 60 * 1000 (10 minutes) should be extracted into a constant. Consider creating DEFAULT_EXECUTION_TIMEOUT_MS for consistency.
| scheduledAt, | ||
| params, | ||
| repeatAfterMS, | ||
| schedulingTimeoutAt: scheduledAt.valueOf() + 10 * 60 * 1000, |
There was a problem hiding this comment.
Another instance of the magic number 10 * 60 * 1000. This should use the same constant as the other timeout values.
| schedulingTimeoutAt: scheduledAt.valueOf() + 10 * 60 * 1000, | |
| schedulingTimeoutAt: scheduledAt.valueOf() + time.TEN_MINUTES_MS, |
| previousTaskId: task._id, | ||
| originalTaskId: task.originalTaskId || task._id, | ||
| timeoutMS: task.timeoutMS, | ||
| schedulingTimeoutAt: scheduledAt.valueOf() + 10 * 60 * 1000 |
There was a problem hiding this comment.
Fourth occurrence of the magic number. All instances of 10 * 60 * 1000 should reference the same constant.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/task into vkarpov15/scheduling-timeouts
No description provided.