Fix scheduler loop permanently stopping on reversible states - #787
Merged
Conversation
BackgroundServiceTask.ExecuteAsync used break for states that can change at runtime: task not yet seeded in the DB, task disabled from the admin panel, and task leased to a machine that no longer holds it. Once hit, the loop ended for good and required a process restart to recover - e.g. QueuedMessagesSendScheduleTask silently stops sending order confirmation e-mails after an admin toggles it off and back on. Extract the scheduling logic into a pure Decide(task, machineName, utcNow) function with no side effects and no terminal outcome: every branch resolves to a delay-and-retry action, so there is no 'stop the loop' outcome to accidentally reach - the break statements are gone from ExecuteAsync entirely, by construction rather than by convention. Decide() is unit-tested directly without any timers, DI, or BackgroundService lifecycle involved. Also log unhandled exceptions from the outer catch instead of swallowing them silently, and resolve the logger once from the root provider so it is available there too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
KrzysztofPajak
force-pushed
the
fix/scheduler-loop-recovery
branch
from
August 16, 2026 10:59
bb559f8 to
127ea8c
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #issueNumber
Type: bugfix
Issue
BackgroundServiceTask.ExecuteAsync(the in-process scheduler loop) usedbreakto end the loop for three states that are all reversible at runtime:Once any of these was hit, the loop exited for good — the only way to recover was a full process restart, with no error or alert surfaced. Concretely: an admin toggling a task off and back on (e.g.
QueuedMessagesSendScheduleTask) silently stops order-confirmation e-mails from ever being queued again on that instance.Separately, the outer
catch (Exception)swallowed every exception (e.g. a transient DB failure inGetTaskByName) without logging, making the failure mode above undiagnosable from logs.Solution
breakstatements with delay-then-continue, so the loop keeps polling instead of ending for good.catchviaLogErrorinstead of swallowing them; keep shutdown (OperationCanceledExceptionon cancellation) unlogged and distinct from a real failure.protected virtual TimeSpan Delay(int minutes)seam so tests can shrink the minute-scale waits to milliseconds and observe the loop recovering without a real wait.Cron-style scheduling and a dedicated worker host (also suggested in the review that raised this) are out of scope here — larger, separately breaking changes.
Breaking changes
None.
Testing
dotnet test src/Tests/Grand.Web.Common.Tests/Grand.Web.Common.Tests.csproj --filter "FullyQualifiedName~BackgroundServiceTaskTests"— 7/7 pass, including three new tests added for this fix:ExecuteAsync_TaskDisabled_ThenReEnabled_ResumesExecutionWithoutRestartExecuteAsync_TaskNotYetSeeded_ThenSeeded_ResumesExecutionWithoutRestartExecuteAsync_UnexpectedExceptionInLoop_LogsErrorAndKeepsRunningdotnet build src/Web/Grand.Web.Common/Grand.Web.Common.csproj— succeeds, 0 warnings/errors.