14

I'm building an API with Laravel and want to send push notification using the Laravel Notifications system. I've a model for matches (which is basically a post), another user can like this match. When the match is liked, the creator of the post will get a push notification. It's just like Instagram, Facebook, etc.

Often the push notification wasn't send to the user. I installed Laravel Horizon to see if there where errors. Sometimes the notification was send and sometimes it wasn't. With the exact same data:

Laravel Horizon list

The notification fails sometimes with the exact same data (same user, same match).

The error is as followed:

Illuminate\Database\Eloquent\ModelNotFoundException: No query results for model [App\Models\Match] 118 in /home/forge/owowgolf.com/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php:312

I'm sure the match and the user exists in the database, I've verified that before sending the notification. Does anybody know what's going wrong? Everything I could find online is that people didn't save their model before sending the notification into the queue. But the line where the code send's the notification into the queue wouldn't even be reached if the model didn't exists. Because of Implicit Binding in the route/controller.

Controller method:

/**
 * Like a match.
 *
 * @param  \App\Models\Match  $match
 * @return \Illuminate\Http\JsonResponse
 */
public function show(Match $match)
{
    $match->like();

    $players = $match->players()->where('user_id', '!=', currentUser()->id)->get();

    foreach ($players as $user) {
        $user->notify(new NewLikeOnPost($match, currentUser()));
    }

    return ok();
}

Notification:

<?php

namespace App\Notifications;

use App\Models\Match;
use App\Models\User;
use Illuminate\Bus\Queueable;
use NotificationChannels\Apn\ApnChannel;
use NotificationChannels\Apn\ApnMessage;
use Illuminate\Notifications\Notification;
use Illuminate\Contracts\Queue\ShouldQueue;

class NewLikeOnPost extends Notification implements ShouldQueue
{
    use Queueable;

    /**
     * The match instance.
     *
     * @var \App\Models\Match
     */
    private $match;

    /**
     * The user instance.
     *
     * @var \App\Models\User
     */
    private $user;

    /**
     * Create a new notification instance.
     *
     * @param  \App\Models\Match  $match
     * @param  \App\Models\User  $user
     */
    public function __construct(Match $match, User $user)
    {
        $this->user = $user;
        $this->match = $match;

        $this->onQueue('high');
    }

    /**
     * Get the notification's delivery channels.
     *
     * @param  \App\Models\User  $notifiable
     * @return array
     */
    public function via($notifiable)
    {
        if ($notifiable->wantsPushNotification($this)) {
            return ['database', ApnChannel::class];
        }

        return ['database'];
    }

    /**
     * Get the mail representation of the notification.
     *
     * @param  \App\Models\User  $notifiable
     * @return \NotificationChannels\Apn\ApnMessage
     */
    public function toApn($notifiable)
    {
        return ApnMessage::create()
            ->badge($notifiable->unreadNotifications()->count())
            ->sound('success')
            ->body($this->user->username . ' flagged your match.');
    }

    /**
     * Get the array representation of the notification.
     *
     * @param  mixed  $notifiable
     * @return array
     */
    public function toArray($notifiable)
    {
        return [
            'user_id' => $this->user->id,
            'body' => "<flag>Flagged</flag> your match.",
            'link' => route('matches.show', $this->match),
            'match_id' => $this->match->id,
        ];
    }

    /**
     * Get the match attribute.
     *
     * @return \App\Models\Match
     */
    public function getMatch()
    {
        return $this->match;
    }
}
Cy Rossignol
  • 16,216
  • 4
  • 57
  • 83
Dees Oomens
  • 4,554
  • 3
  • 29
  • 61
  • 1
    Show the code of a notification file. – Vikash Oct 05 '17 at 09:36
  • Added the code of `NewLikeOnPost` notification class. – Dees Oomens Oct 05 '17 at 09:56
  • I'm not sure, but anywhere you are trying to access the session data inside your jobs or any function or object you are processing in queue is using session ? – Vikash Oct 05 '17 at 10:22
  • @DeesOomens Can you add the code of the file that triggers the notifications – Charlie Sheather Oct 05 '17 at 10:50
  • @CharlieSheather I updated my question. – Dees Oomens Oct 05 '17 at 11:02
  • have you used `firstorFail()` in like()?? – Mr. Pyramid Oct 29 '17 at 04:18
  • Basically I'd try not to put the models in your queue. Besides the pure size of the queue items, the models may change until the job get processed. Also for debugging, I would try to pass only the IDs to your notification and do a fresh `findOrFail()` while starting to process your queue item. Maybe this already gives you a further hint whats going on here – patriziotomato Jan 24 '18 at 18:39
  • @DeesOomens I am facing the exact same issue. It only happens with one Notification classes for some reason. Have you had any luck in finding out a fix? – qasimalbaqali Jun 21 '18 at 18:45
  • @qasimalbaqali Can you provide a stacktrace? – Jonas Staudenmeir Jun 22 '18 at 15:42
  • @JonasStaudenmeir Here you go https://pastebin.com/sExrkvj1 – qasimalbaqali Jun 23 '18 at 12:59
  • @qasimalbaqali Do you use database transactions in the place where you trigger the notification? – Jonas Staudenmeir Jun 23 '18 at 14:08
  • @JonasStaudenmeir no I don't. I send this notification after a User model gets created `User::create()`, and I notify that user instance. – qasimalbaqali Jun 23 '18 at 14:25
  • @qasimalbaqali Are you using multiple databases? Did you try any debugging in `SerializesAndRestoresModelIdentifiers::getRestoredPropertyValue()`? You could check the content of the `users` table to maybe narrow down the problem. – Jonas Staudenmeir Jun 24 '18 at 00:22
  • No I am not using multiple databases. Its a single database, and the user is created successfully, verified in the database, and before the notification call I create other model instances using the user instance which doesn't return any errors at all. – qasimalbaqali Jun 24 '18 at 18:57
  • I think you have overlap on your jobs, check jobs to avoid overlaps – Payam Khaninejad Jun 26 '18 at 10:15
  • 1
    it's possible that you are facing a postgresql row lock whatever is the cause (mostly the excessive use of currentUser() method). check this post it might help you: https://stackoverflow.com/questions/1063043/how-to-release-possible-postgres-row-locks – N69S Jun 28 '18 at 10:55
  • I'm having the same issue, but only on production. Locally everything works as expected. Any headway? – Grant Aug 10 '18 at 08:31
  • 1
    It had to do with multiple environments on the same servers using the same queue system. The events/jobs for staging were being executed in the production queue. – Dees Oomens Aug 15 '18 at 11:48

3 Answers3

1

This is not a complete solution, but it will lower your chances of running into this error in the future.

Instead of passing in the whole Match model into the job, only pass the id of the model. You can then fetch that model in the constructor.

/**
 * Like a match.
 *
 * @param  \App\Models\Match  $match
 * @return \Illuminate\Http\JsonResponse
 */
public function show(Match $match)
{
    $match->like();

    $players = $match->players()->where('user_id', '!=', currentUser()->id)->get();

    foreach ($players as $user) {
        $user->notify(new NewLikeOnPost($match->id, currentUser()->id));
    }

    return ok();
}

Notification:

class NewLikeOnPost extends Notification implements ShouldQueue
{
    use Queueable;

    private const QUEUE_NAME = 'high';

    /**
     * The match instance.
     *
     * @var \App\Models\Match
     */
    private $match;

    /**
     * The user instance.
     *
     * @var \App\Models\User
     */
    private $user;

    /**
     * Create a new notification instance.
     *
     * @param  int $match
     * @param  int $user
     */
    public function __construct(int $matchId, int $userId)
    {
        $this->user = User::query()->where('id', $userId)->firstOrFail();
        $this->match = Match::query()->where('id', $matchId)->firstOrFail();

        $this->onQueue(self::QUEUE_NAME);
    }

    // Rest of the class is still the same...
}

You can use the SerializesModels trait, but it doesn't work well when you add a delay to a queued job. This is because it will try to reload the model on __wakeup() and sometimes it cannot find the class.

Hopefully this helps :)

Simon D.
  • 73
  • 8
0

Its probably because $user is not an object of User model, its an object of Match model. You need to do a User::findorfail or User::firstOrFail then notify the user.

public function show(Match $match)
{
$match->like();

$players = $match->players()->where('user_id', '!=', currentUser()->id)->get();

foreach ($players as $user) {
    $someUser = User::findOrFail($user->user_id);
    $someUser->notify(new NewLikeOnPost($match, currentUser()));
}

return ok();

}

Unless the notify trait is used in Match model. Or you could use eager loading which will cost way less queries!

Tghosh
  • 180
  • 3
  • 14
-2

Check your .env to be sure that u really use REDIS

BROADCAST_DRIVER=redis
CACHE_DRIVER=redis
SESSION_DRIVER=redis
SESSION_LIFETIME=120
QUEUE_DRIVER=redis

then clear cache ( php artisan cache:clear , php artisan view:clear ), that should clear the issue

EDIT

I had similar problems but now I use Docker only and before I had to check for cached configfiles, wrong file/folderpermissions and so on (REDIS for broadcast only, others were standard). I started using redis only - that`s a lot easier, faster and more debugfriendly for me ! And together with Docker really helpful to not use messed up nginx/apache/php/redis/ ...

BitBay
  • 17
  • 5