Why doesn't the code always work?

Question

PHP

There is a site on Laravel, a payment system is connected to it.
There is a code that works in case of successful replenishment.

public static function updateDataUser($waitingPayment, $balance)
{

    try {
        DB::beginTransaction();
        $waitingPayment = HistoryPayment::find($waitingPayment->id);
        if ($waitingPayment->status_transaction === 'success') {
            return false;
        }

        $waitingPayment->status_transaction = 'success';
        $waitingPayment->save();

        $user = User::find($waitingPayment->user->id);

        $user->balance += $balance;
        $user->save();

        DB::commit();

    } catch (\Throwable $e) {
        DB::rollBack();
        return false;
    }
    ChangeBalance::dispatch($user->id, $user->balance);
    return true;
}

As you can see, first we assign the 'success' status to the operation, and then we increase the user's balance.
The problem is that sometimes it happens when the 'success' status is assigned, but the balance simply does not add up. I don't understand why this is happening.

A person writes to technical support, we look at transactions, we see successful ones, but there is no balance and purchase history too. Those. really not counted and we manually replenish it. But of course this needs to be fixed.

What could be the problem?

Answer 1

Add logging so that you know that it was within this $waitingPayment->id that the status and balance changed.
ps Updates must be done atomically,
User::where('id', $waitingPayment->user->id)->increment('balance', $balance);
The ChangeBalance::dispatch event is also better to be observed at the model level, otherwise you will forget to throw it away and break the logic.
$waitingPayment->status_transaction - encapsulate all the logic inside the model, otherwise you are doing a comparison with the string twice and this is potentially fragile code, you won't even notice in case of a typo.

Answer 2

Here it is a little strange done:
DB::beginTransaction();
$waitingPayment = HistoryPayment::find($waitingPayment->id);
if ($waitingPayment->status_transaction === 'success') {
return false;
}
Open a transaction, check that status_transaction === 'success' and exit without closing it or rolling back. Can make a check before opening a transaction.
It’s just that the transaction opened here can, somewhere further, not be explicitly used and it is possible to influence the fact that something is then rolled back during rollback, which I should not roll back.
There is also a call to
ChangeBalance::dispatch($user->id, $user->balance);
It is outside the main transaction and not in try, can it affect the balance, for example, if an error occurs inside it?

Answer 3

Like this...

if (preg_match('#^\/item\/[0-9]+\/.+#i', $_SERVER['REQUEST_URI'])) {
   header("HTTP/1.0 404 Not Found");
   include $_SERVER['DOCUMENT_ROOT'].'/404.php';
   exit();
}

Answer 4

if (!empty($_GET)) {
  header("HTTP/1.0 404 Not Found");
   include $_SERVER['DOCUMENT_ROOT'].'/404.php';
   exit();
}

Answer 5

if(preg_match('#^/item/[0-9]/.+$#is',$_SERVER['REQUEST_URI'])){