Attempt to read property "headers" on null Laravel Middleware

public function handle(Request $request, Closure $next)
{
    $seller = DB::table('seller_accounts')->where('user_id',auth()->id())->first();
    if ($seller->is_active !== 0) {
        return $next($request);
    }
    else{
        $seller_order = DB::table('seller_orders')->where('user_id',auth()->id())->first();
        if(!empty($seller_order))
        {
            if($seller_order->is_paid == 0 && $seller_order->is_cancelled == 0)
            {
                $payu = app(PayUController::class);
                $payu->changeSellerOrderStatus();
                return redirect()->route('seller.after-pay');
            }elseif ($seller_order->is_paid == 1)
            {
                DB::table('seller_accounts')->where('user_id', auth()->id())->update(['is_active' => 1]);
            }elseif ($seller_order->is_cancelled == 1)
            {
                return response()->view("multivendor::dashboard.cancelled");
            }

        }else{
            return response()->view("multivendor::dashboard.buy");
        }

    }
}

I get this error when seller is active I think broblem is on next request but I don't know how to solve this.

CodePudding user response：

To return view in middleware use it like this:

return new response(view('multivendor::dashboard.cancelled'));

Note: Best is to return route.

CodePudding user response：

Your description sounds strange. Since you type-hinted the request, it should never be null.
Especially in a middleware, where Laravel will inject it by default (if it is used according to the standards).

As a general rule, I would strongly suggest trying to work toward avoiding else as much as possible.
In your example, every time you have a return within a condition, this will exit the code anyway, so the else is redundant anyway. With very small changes you can refactor your code to reduce the complexity and make it way easier to read.
You will probably have to touch this code multiple times and each time you need more "brain power" to read this than necessary if there is too much nesting going on.
It is then also easier to process for anyone else, for example in this question here.

Here is a simple example of how this could look (without changing anything in the logic):

    public function handle(Request $request, Closure $next)
    {
        $seller = DB::table('seller_accounts')->where('user_id',auth()->id())->first();
        if ($seller->is_active) {
            return $next($request);
        }

        $seller_order = DB::table('seller_orders')->where('user_id',auth()->id())->first();
        if(empty($seller_order)) {
            return response()->view("multivendor::dashboard.buy");
        }    

        // you could also move this two conditions into a helper method within the order, like "$seller_order->isOpen()"
        if($seller_order->is_paid == 0 && $seller_order->is_cancelled == 0) {
            $payu = app(PayUController::class);
            $payu->changeSellerOrderStatus();
            return redirect()->route('seller.after-pay');
        } 

        if ($seller_order->is_cancelled == 1) {
            return response()->view("multivendor::dashboard.cancelled");
        }
        
        if ($seller_order->is_paid == 1) {
            DB::table('seller_accounts')->where('user_id', auth()->id())->update(['is_active' => 1]);
            // return something?
        } 
    }

Additionally, you should probably cast the is_paid and is_cancelled properties to boolean, which would enable you to remove the (currently unsafe) comparisons in the conditions with this:
if ($seller_order->is_paid) {