Repository navigation
Fix rivertest.Worker ignoring Config.JobStuckThreshold - #1418
Merged
brandur merged 3 commits intoOct 1, 2026
Merged
Conversation
`rivertest.Worker` builds a `JobExecutor` without setting `StuckThresholdOverride`, so stuck job detection always used the executor's internal 5 second default and ignored `Config.JobStuckThreshold`. The producer has passed the configured value, which defaults to 10 seconds, since riverqueue#1291 added the setting. Pass `w.config.JobStuckThreshold` the same way the producer does. `NewWorker` already runs the config through `WithDefaults`, so an unset threshold becomes the 10 second default without any extra handling here.
Nothing in `rivertest` covered stuck job detection. The new subtest sets a 1ms threshold and a 10ms timeout, the same values as the producer's `JobStuckHandler` test, and has the worker wait until the `Job appears to be stuck` log line is written. The test worker's stuck callback is a no-op, so the log is the only signal available. The test doesn't assert on timing. Without the fix the log line comes after about 5 seconds, which fails against the local 3 second `WaitTimeout()` but not against the 10 second one used on GitHub Actions. An upper bound on elapsed time would catch that on CI too, but would be a timing-sensitive assertion that a slow runner could trip. It's only under `TestWorker_Work` because the threshold is applied in `workJob`, which `Work` and `WorkJob` both go through.
brandur
approved these changes
Oct 1, 2026
brandur
left a comment
brandur
left a comment
Contributor
There was a problem hiding this comment.
Looks great. Thanks Toru!
brandur
added a commit
that referenced
this pull request
Oct 1, 2026
Minor tweak to #1418. This was note in the PR description, but the `WaitOrTimeout` helper allows ten seconds timeout in GitHub's CI. The test's executor only waits five seconds, so it'll never fail in CI. Deconstruct the `WaitOrTimeout` so that we can fail after only three seconds.
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.
When a job worked by
rivertest.Workergets stuck, theJob appears to be stuckwarning is logged 5 seconds after the job's timeout regardless ofConfig.JobStuckThreshold. A real client uses the configured threshold, which defaults to 10 seconds.#1291 added
JobStuckThresholdand passed it to the job executor from the producer only.rivertest.Workerbuilds the same executor and already passesJobTimeoutthrough, but it leavesStuckThresholdOverrideunset, so the executor falls back to its internal 5 second default. This passesw.config.JobStuckThreshold, whichNewWorkerhas already defaulted. If keeping 5 seconds inrivertestwas intentional, let me know and I'll document that instead.The new subtest uses the same 1ms threshold and 10ms timeout as the producer's
JobStuckHandlertest, and only waits for the warning to be logged, about 11ms in. Like that test, it doesn't check timing: without the fix it fails locally at the 3 secondWaitTimeout(), but withGITHUB_ACTIONS=true, whereWaitTimeout()is 10 seconds, it still passes after about 5 seconds.Locally,
-raceonrivertestandgolangci-lint runpass.Claude Code assisted with diagnosing the problem, implementing the fix, and writing the test.