Skip to content

fix: track invoked runs in isBusy()/getStatus() and noOverlap#612

Open
sarmah-rup wants to merge 1 commit into
node-cron:mainfrom
sarmah-rup:fix/track-invoked-runs
Open

fix: track invoked runs in isBusy()/getStatus() and noOverlap#612
sarmah-rup wants to merge 1 commit into
node-cron:mainfrom
sarmah-rup:fix/track-invoked-runs

Conversation

@sarmah-rup

Copy link
Copy Markdown

Problem

getStatus() / isBusy() don't account for runs started via execute().
The state machine only flipped to running when execution.reason === 'scheduled',
so a manually invoked run (reason 'invoked') stayed invisible: getStatus()
reported idle and isBusy() returned false mid-run. noOverlap relies on
the same state, so it couldn't protect invoked runs either.

Fix

Drop the reason === 'scheduled' guard in the beforeRun and onFinished
hooks so every run flips running -> idle, matching the already-unconditional
onError hook. changeState remains guarded by runner.isStarted(), so an
execute() on an unstarted task is still a no-op. The idle/running transitions
are already permitted by the state machine.

Tests

Added a regression test in introspection.test.ts that gates an async handler,
invokes execute() without awaiting, waits for execution:started, and asserts
isBusy() === true and getStatus() === 'running'. It fails on the current
code and passes with the fix. Full suite green with 100% coverage.

Closes #611

@merencia

Copy link
Copy Markdown
Member

hello @sarmah-rup thank for the PR, holding on it because the behavior was intentional.

#611 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

execute() bypasses noOverlap / isBusy() tracking

2 participants