Feat/compt 77 dynamic scheduling error handling#5
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a lightweight in-memory scheduler service (interval/timeout/cron) with concurrency guarding and configurable error handling, and updates build/lint tooling to support the new package shape.
Changes:
- Introduces
SchedulerService+ISchedulercontract, along withDuplicateJobError. - Adds Jest coverage for scheduler behavior (timers, concurrency guard, error handling, status).
- Updates TypeScript build/lint configs and regenerates the lockfile for the new dependency set.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.eslint.json | Expands ESLint TS project include patterns (now includes *.mjs). |
| tsconfig.build.json | Switches build output to CommonJS/Node resolution and narrows build include list to scheduler-related files. |
| src/services/scheduler.service.ts | Implements the scheduler registry, execution guard, status reporting, and error callback handling. |
| src/services/scheduler.service.spec.ts | Adds unit tests for scheduling, unscheduling, rescheduling, concurrency, and error behavior. |
| src/interfaces/scheduler.interface.ts | Defines the public scheduler interface and discriminated-union timing types. |
| src/index.ts | Updates public exports to expose the scheduler service/types and the error type. |
| src/errors/duplicate-job.error.ts | Adds a dedicated error type for duplicate job registration. |
| src/errors/duplicate-job.error.spec.ts | Adds tests for the error type behavior. |
| package-lock.json | Updates lockfile to reflect new package name/version and dependency set. |
| lint-staged.config.js | Switches lint-staged config export style to CommonJS. |
| eslint.config.mjs | Updates ESLint flat config ignores, TypeScript project wiring, and rule set. |
| eslint.config.js | Removes the previous ESLint config file. |
| { | ||
| "extends": "./tsconfig.json", | ||
| "include": ["src/**/*.ts", "test/**/*.ts", "*.ts", "*.js"], | ||
| "include": ["src/**/*.ts", "test/**/*.ts", "*.ts", "*.js", "*.mjs"], |
There was a problem hiding this comment.
tsconfig.eslint.json includes *.js/*.mjs but does not enable compilerOptions.allowJs. Since ESLint is configured with parserOptions.project, TypeScript will try to build a program from this config and will error on included JS/MJS files. Remove JS/MJS from include (keep it TS-only), or enable allowJs (and optionally checkJs: false) in this tsconfig.
| "include": ["src/**/*.ts", "test/**/*.ts", "*.ts", "*.js", "*.mjs"], | |
| "include": ["src/**/*.ts", "test/**/*.ts", "*.ts"], |
| import { Injectable, Logger } from "@nestjs/common"; | ||
| import { CronJob } from "cron"; | ||
| import { DuplicateJobError } from "../errors/duplicate-job.error"; |
There was a problem hiding this comment.
SchedulerService imports CronJob from the cron package, but cron is not declared in package.json dependencies/peerDependencies. This will break compilation/runtime for consumers unless cron is added as a runtime dependency (or the implementation is changed to avoid it).
| private readonly onJobError: (name: string, error: unknown) => void; | ||
|
|
||
| constructor(options: SchedulerModuleOptions = {}) { | ||
| this.onJobError = | ||
| options.onJobError ?? | ||
| ((name, error) => this.logger.error(`Job '${name}' threw an error`, error)); | ||
| } | ||
|
|
There was a problem hiding this comment.
The SchedulerService constructor takes a plain options object. When instantiated via Nest DI, Nest will try to resolve this parameter as a provider (design type is Object) and fail. Consider removing the constructor param, or injecting options via an explicit injection token (@Inject(...) + @Optional()) provided by a module forRoot/forRootAsync factory.
| private readonly onJobError: (name: string, error: unknown) => void; | |
| constructor(options: SchedulerModuleOptions = {}) { | |
| this.onJobError = | |
| options.onJobError ?? | |
| ((name, error) => this.logger.error(`Job '${name}' threw an error`, error)); | |
| } | |
| private readonly onJobError: (name: string, error: unknown) => void = ( | |
| name, | |
| error, | |
| ) => this.logger.error(`Job '${name}' threw an error`, error); |
| reschedule(name: string, newTiming: ScheduleTiming): void { | ||
| const entry = this.registry.get(name); | ||
| if (!entry) return; | ||
| const newJob: ScheduledJob = { ...entry.job, ...newTiming } as ScheduledJob; | ||
| entry.stop(); | ||
| this.registry.delete(name); | ||
| this.schedule(newJob); | ||
| } |
There was a problem hiding this comment.
reschedule() builds newJob via { ...entry.job, ...newTiming }, which retains the previous timing key(s). Rescheduling from one timing type to another can produce a job object with multiple timing properties (e.g. both interval and cron), violating the discriminated-union contract and potentially leading to ambiguous behavior. Reconstruct the job from { name, handler, ...newTiming } (and avoid the as ScheduledJob cast).
| ((name, error) => this.logger.error(`Job '${name}' threw an error`, error)); | ||
| } | ||
|
|
There was a problem hiding this comment.
The default onJobError implementation passes the caught error object as the second argument to Logger.error, which is treated as the stack/trace string. This often results in unhelpful output like [object Object] for non-Error values and can drop stack traces. Consider normalizing error to a stack string when it is an Error, otherwise String(error).
| ((name, error) => this.logger.error(`Job '${name}' threw an error`, error)); | |
| } | |
| ((name, error) => | |
| this.logger.error( | |
| `Job '${name}' threw an error`, | |
| this.formatJobError(error), | |
| )); | |
| } | |
| private formatJobError(error: unknown): string { | |
| if (error instanceof Error) { | |
| return error.stack ?? error.message; | |
| } | |
| return String(error); | |
| } |
be3ad00 to
d5ec553
Compare
* ops: updated sonar variable * Feat/compt 75 typed job definitions ischeduler interface (#3) * feat(COMPT-75): define typed scheduled job contracts and IScheduler port - Add ScheduledJob type: name, handler, and one of cron | interval | timeout - Add IScheduler interface: schedule, unschedule, reschedule, list - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add DuplicateJobError thrown when registering a duplicate job name - Enforce cron/interval/timeout mutual exclusivity via discriminated union - Add CronExpression named constants for human-readable schedules - Add cron fluent builder: dailyAt, weeklyOn, monthlyOn, every().minutes() - Update tsconfig.build.json: CommonJS output, Task 1 files only - Update package.json: name @ciscode/scheduler-kit, version 0.0.0 * test(COMPT-75): add unit tests for CronExpression and cron builder * feat(COMPT-75): add ScheduledJobStatus type and status/listStatus methods - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add status(name) and listStatus() to IScheduler interface and SchedulerService - Track lastRun timestamp after each execution - Store cronJob reference for nextRun via cronJob.nextDate() - Export ScheduledJobStatus from public API - Fix lint: no-misused-promises, no-floating-promises, prettier formatting * style(COMPT-75): format all files with prettier --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 76 nestjs scheduler module and service (#4) * feat(COMPT-76): NestJS SchedulerModule and SchedulerService integration - SchedulerModule.register() and registerAsync() dynamic module factories - SchedulerService: schedule(), reschedule(), unschedule(), list(), status(), listStatus() - @Cron, @interval, @timeout decorators with optional job name - MetadataScanner auto-discovers decorated provider methods on onModuleInit - DuplicateJobError guard against double registration - IScheduler interface and ScheduledJobStatus type - CronExpression constants and cron fluent builder - Full unit test coverage across module, service and decorators * test(COMPT-76): add unit tests for CronExpression and cron builder --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 77 dynamic scheduling error handling (#5) * chore(COMPT-77): scope index.ts and tsconfig.build.json to COMPT-77 files only * test(COMPT-77): add tests for status, listStatus, cron jobs and default onJobError * chore(COMPT-77): restore index.ts and tsconfig.build.json to develop baseline --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * test(COMPT-78): full coverage with Jest fake timers for all schedulin… (#7) * test(COMPT-78): full coverage with Jest fake timers for all scheduling behaviors - jest.useFakeTimers() for all time-based tests — no real waiting - @interval: fires every N ms, stops after unschedule, does not fire before interval - @timeout: fires exactly once, removes itself from registry, does not fire early - schedule(): job added and fires on tick - unschedule(): timer cleared, handler not called after removal - reschedule(): new schedule applied atomically, old timer cancelled - Job error: caught via onJobError, scheduler continues — other jobs unaffected - Concurrent guard: second execution skipped when first still running (isRunning lock) - status() / listStatus(): reflect isRunning and lastRun correctly - @Cron: registered and unscheduled without throwing - Default onJobError: delegates to Logger.error - Coverage: 98.63% statements, 98.11% branches, 95.34% functions (threshold 85%) * fix(COMPT-78): resolve SonarCloud S1186 and S4524 issues - cron-builder.ts: parseInt -> Number.parseInt (Sonar S4524 x3) - scheduler.decorators.spec.ts: add comments to empty stub methods (Sonar S1186 x6) - scheduler.module.spec.ts: add comments to empty stub methods and class (Sonar S1186 x6) --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * release(COMPT-79): bump version to 0.1.0, rewrite README, add changeset (#8) - package.json: 0.0.0 -> 0.1.0 - README: full documentation (decorators, dynamic API, concurrency guard, error handling, async config, cron helpers, API reference) - .changeset/scheduler-kit-v0-1-0.md: minor bump for initial feature release Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 79 readme changeset publish v0.1.0 (#10) * release(COMPT-79): bump version to 0.1.0, rewrite README, add changeset - package.json: 0.0.0 -> 0.1.0 - README: full documentation (decorators, dynamic API, concurrency guard, error handling, async config, cron helpers, API reference) - .changeset/scheduler-kit-v0-1-0.md: minor bump for initial feature release * fix(COMPT-79): add sonar-project.properties to fix SonarCloud quality gate - sonar.exclusions: exclude spec files from source analysis - sonar.tests + sonar.test.inclusions: declare spec files as test code - fixes 31.9% coverage on new code and 4.8% duplication failures --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> --------- Co-authored-by: Zaiidmo <zaiidmoumnii@gmail.com> Co-authored-by: saad moumou <saad.moumou.coder@gmail.com>
* ops: updated sonar variable * Feat/compt 75 typed job definitions ischeduler interface (#3) * feat(COMPT-75): define typed scheduled job contracts and IScheduler port - Add ScheduledJob type: name, handler, and one of cron | interval | timeout - Add IScheduler interface: schedule, unschedule, reschedule, list - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add DuplicateJobError thrown when registering a duplicate job name - Enforce cron/interval/timeout mutual exclusivity via discriminated union - Add CronExpression named constants for human-readable schedules - Add cron fluent builder: dailyAt, weeklyOn, monthlyOn, every().minutes() - Update tsconfig.build.json: CommonJS output, Task 1 files only - Update package.json: name @ciscode/scheduler-kit, version 0.0.0 * test(COMPT-75): add unit tests for CronExpression and cron builder * feat(COMPT-75): add ScheduledJobStatus type and status/listStatus methods - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add status(name) and listStatus() to IScheduler interface and SchedulerService - Track lastRun timestamp after each execution - Store cronJob reference for nextRun via cronJob.nextDate() - Export ScheduledJobStatus from public API - Fix lint: no-misused-promises, no-floating-promises, prettier formatting * style(COMPT-75): format all files with prettier --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 76 nestjs scheduler module and service (#4) * feat(COMPT-76): NestJS SchedulerModule and SchedulerService integration - SchedulerModule.register() and registerAsync() dynamic module factories - SchedulerService: schedule(), reschedule(), unschedule(), list(), status(), listStatus() - @Cron, @interval, @timeout decorators with optional job name - MetadataScanner auto-discovers decorated provider methods on onModuleInit - DuplicateJobError guard against double registration - IScheduler interface and ScheduledJobStatus type - CronExpression constants and cron fluent builder - Full unit test coverage across module, service and decorators * test(COMPT-76): add unit tests for CronExpression and cron builder --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 77 dynamic scheduling error handling (#5) * chore(COMPT-77): scope index.ts and tsconfig.build.json to COMPT-77 files only * test(COMPT-77): add tests for status, listStatus, cron jobs and default onJobError * chore(COMPT-77): restore index.ts and tsconfig.build.json to develop baseline --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * test(COMPT-78): full coverage with Jest fake timers for all schedulin… (#7) * test(COMPT-78): full coverage with Jest fake timers for all scheduling behaviors - jest.useFakeTimers() for all time-based tests — no real waiting - @interval: fires every N ms, stops after unschedule, does not fire before interval - @timeout: fires exactly once, removes itself from registry, does not fire early - schedule(): job added and fires on tick - unschedule(): timer cleared, handler not called after removal - reschedule(): new schedule applied atomically, old timer cancelled - Job error: caught via onJobError, scheduler continues — other jobs unaffected - Concurrent guard: second execution skipped when first still running (isRunning lock) - status() / listStatus(): reflect isRunning and lastRun correctly - @Cron: registered and unscheduled without throwing - Default onJobError: delegates to Logger.error - Coverage: 98.63% statements, 98.11% branches, 95.34% functions (threshold 85%) * fix(COMPT-78): resolve SonarCloud S1186 and S4524 issues - cron-builder.ts: parseInt -> Number.parseInt (Sonar S4524 x3) - scheduler.decorators.spec.ts: add comments to empty stub methods (Sonar S1186 x6) - scheduler.module.spec.ts: add comments to empty stub methods and class (Sonar S1186 x6) --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * release(COMPT-79): bump version to 0.1.0, rewrite README, add changeset (#8) - package.json: 0.0.0 -> 0.1.0 - README: full documentation (decorators, dynamic API, concurrency guard, error handling, async config, cron helpers, API reference) - .changeset/scheduler-kit-v0-1-0.md: minor bump for initial feature release Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * Feat/compt 79 readme changeset publish v0.1.0 (#10) * release(COMPT-79): bump version to 0.1.0, rewrite README, add changeset - package.json: 0.0.0 -> 0.1.0 - README: full documentation (decorators, dynamic API, concurrency guard, error handling, async config, cron helpers, API reference) - .changeset/scheduler-kit-v0-1-0.md: minor bump for initial feature release * fix(COMPT-79): add sonar-project.properties to fix SonarCloud quality gate - sonar.exclusions: exclude spec files from source analysis - sonar.tests + sonar.test.inclusions: declare spec files as test code - fixes 31.9% coverage on new code and 4.8% duplication failures --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com> * patch lock file deps * chore(tsconfig): silence TS6 deprecation warnings --------- Co-authored-by: saadmoumou <s.moumou@ciscod.com> Co-authored-by: saad moumou <saad.moumou.coder@gmail.com>
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes