[2.x] fix: avoid duplicate ModelNotFoundException logs on orphaned queued jobs - #4634
Merged
Merged
Conversation
When a queued job's serialized model is deleted between dispatch and worker pickup, Laravel's CallQueuedHandler catches the unserialize ModelNotFoundException via handleModelNotFound. Without deleteWhenMissingModels, it then routes the job to fail(), which re-deserializes the payload and throws ModelNotFoundException a second time — this one is not caught and gets logged. Set deleteWhenMissingModels = true on AbstractJob so the handler delete()s the job (already off the queue, lock already released) instead of going through the noisy fail() path. All current AbstractJob subclasses (notification fan-out, mail sends, search index, package manager, gdpr export, etc.) are no-ops when their subject model is missing — there's no useful retry path. Subclasses that genuinely want to be retried/failed on missing models can override with `public bool $deleteWhenMissingModels = false;`. Fixes #4615
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.
Summary
When a queued job's serialized model is deleted between dispatch and worker pickup (a common race for notification fan-out — mentions, subscriptions, FoF byobu/follow-tags, etc.), the worker logs two
ModelNotFoundExceptionentries even though the race is handled correctly.The flow (
CallQueuedHandler):unserialize()s payload →firstOrFail()throwsModelNotFoundException(caught silently).handleModelNotFound()runs. WithdeleteWhenMissingModelsunset (defaultfalse), it falls through to$job->fail($e).fail()callsgetCommand()→unserialize()again →ModelNotFoundException(this time not caught, gets logged).Setting
public bool $deleteWhenMissingModels = true;onAbstractJobmakes the handler$job->delete()the job instead, skipping the noisy second-deserialize path. The job is already off the queue and any unique-job lock has already been released.Fixes #4615
Why apply at the AbstractJob level
All current
AbstractJobsubclasses (12 across core + bundled extensions) follow the same pattern: notify-X-about-Y, send-mail-about-Y, index-Y-for-search, run-composer-command, gdpr-export-for-user, etc. None of them does anything useful if the subject model has been deleted — there's no meaningful retry path.Subclasses that genuinely want the failed-and-retry behavior can opt out with
public bool $deleteWhenMissingModels = false;.Changes
framework/core/src/Queue/AbstractJob.php— setpublic bool $deleteWhenMissingModels = true;with a docblock explaining the override path.Test plan
ModelNotFoundExceptionis logged once at INFO/DEBUG level (Laravel's caught-and-handled path), not twice at ERROR.Notes
getHidden()stub fix) — once that's merged, this PR removes the remaining log noise from the same orphaned-job scenario.AbstractJobsubclass was relying onfailed()being called on missing models. That should be vanishingly rare given Flarum's job patterns, and the override is one line. Worth calling out in upgrade notes.