framework: [5.1] Jobs fail to run when a dependent model is deleted

If a Model is serialized in let’s say Job A, and it is deleted in Job B before this job can run, it throws an exception during the __wakeup() call, throwing it back in queue infinitely.

The problems with the current Job system in this case:

  • You’re unable to catch any Exceptions during the __wakeup() or deserialization phase
  • The serialization of the model is called with findOrFail()

https://github.com/laravel/framework/blob/5.1/src/Illuminate/Queue/SerializesModels.php#L65

[2015-06-20 18:03:22] production.ERROR: exception 'Illuminate\Database\Eloquent\ModelNotFoundException' with message 'No query results for model [KikFinder\Submission].' in /var/www/kikfinder.com/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php:127
Stack trace:
#0 [internal function]: Illuminate\Database\Eloquent\Builder->findOrFail('127')
#1 /.../bootstrap/cache/compiled.php(11013): call_user_func_array(Array, Array)
#2 /.../vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php(65): Illuminate\Database\Eloquent\Model->__call('findOrFail', Array)
#3 /.../vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php(65): KikFinder\Submission->findOrFail('127')
#4 /.../vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php(40): KikFinder\Jobs\Admin\DeleteSubmission->getRestoredPropertyValue(Object(Illuminate\Contracts\Database\ModelIdentifier))
#5 [internal function]: KikFinder\Jobs\Admin\DeleteSubmission->__wakeup()
#6 /.../vendor/laravel/framework/src/Illuminate/Queue/CallQueuedHandler.php(38): unserialize('O:37:"KikFinder...')

About this issue

  • Original URL
  • State: closed
  • Created 9 years ago
  • Comments: 24 (9 by maintainers)

Most upvoted comments

Thanks @valorin for the 5.1 example. This works for me, while running Laravel 5.2:


namespace App\Traits;

use Illuminate\Contracts\Database\ModelIdentifier;

/**
 * Class SerialisesDeletedModels
 * @see https://github.com/laravel/framework/issues/9347#issuecomment-165647596
 */
trait SerialisesDeletedModels
{
    /**
     * @param $value
     * @return mixed
     */
    protected function getRestoredPropertyValue($value)
    {
        if (!$value instanceof ModelIdentifier) {
            return $value;
        }

        if (is_array($value->id)) {
            return $this->restoreCollection($value);
        }

        $instance = new $value->class;
        $query = $instance->newQuery()->useWritePdo();

        if (property_exists($instance, 'forceDeleting')) {
            return $query->withTrashed()->find($value->id);
        }

        return $query->findOrFail($value->id);
    }
}

I made a new trait and overrode the method:

use SerializesModels, SerialisesDeletedModels {
    SerialisesDeletedModels::getRestoredPropertyValue insteadof SerializesModels;
}
use Illuminate\Contracts\Database\ModelIdentifier;

trait SerialisesDeletedModels
{
    protected function getRestoredPropertyValue($value)
    {
        if ($value instanceof ModelIdentifier) {
            $model = new $value->class;

            if (method_exists($model, 'withTrashed')) {
                return $model->withTrashed()->findOrFail($value->id);
            }

            return $model->findOrFail($value->id);
        } else {
            return $value;
        }
    }
}

@GrahamCampbell @EvanDarwin This is actually an issue with you’re dealing with Soft Deletes. If you try to pass a soft-deleted model into a queued job, it will fail because findOrFail() won’t find it - but the model is a perfectly valid model and is still usable.

For example, we have a common class that handles marking models as deleted on the form submission. It then fires a ModelWasDeleted event. I am trying to set up a listener for that event that will queue a job with the deleted event to clean up extra data in the background - but it’s failing because I am passing the deleted model into it.

A simple work-around is to pass in the ID, rather than the model itself. However, this breaks our usual pattern and the change in behaviour when using SerializesModels isn’t really expected.

Or remove SerializeModels for deleted resources?

Good one. I guess I could have used withTrashed()->findOrFail(), but in this case I only used soft-deleting models so a record would always exist.

When you use soft-delete models and a job gets queued with a non-existing (i.e. no database record) soft-deleted model in it then there is a serious problem: that is a situation that theoretically should not exist 😄