archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.6k stars 424 forks source link

Error in Queue ShouldBroadcast since 3.8.5 #1263

Open jetwes opened 2 days ago

jetwes commented 2 days ago

Bug description

I always get an error "Database connection [tenant] not configured." on an event that implements Shouldbroadcast. This error was introduced since upgrading to 3.8.5 - downgrading to 3.8.4 resolves the issue.

Steps to reproduce

Create a event that implements ShouldBroadcast with broadcastOn and broadcastWith method.

Expected behavior

No error

Laravel version

10.48.22

stancl/tenancy version

3.8.5

stancl commented 2 days ago

Please post the code of the event.

jetwes commented 2 days ago

This is ONE example - happens with other events, too:

jetwes commented 2 days ago

`<?php

namespace App\Events\Tickets;

use App\Models\Tenant; use App\Models\Ticket; use App\Models\TicketComment; use App\Models\User; use Illuminate\Broadcasting\InteractsWithSockets; use Illuminate\Broadcasting\PrivateChannel; use Illuminate\Contracts\Broadcasting\ShouldBroadcast; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; use Illuminate\Foundation\Events\Dispatchable; use Illuminate\Queue\SerializesModels;

class TicketCommentCreated implements ShouldBroadcast { use Dispatchable, InteractsWithSockets, SerializesModels;

public Ticket $ticket;
public array $users = [];

/**
 * Create a new event instance.
 */
public function __construct(public TicketComment $ticketComment, public Tenant $tenant, public bool $notifyAgent = false)
{
    $this->ticket = Ticket::query()
        ->where('id', $this->ticketComment->ticket_id)
        ->with(['user', 'agent'])
        ->first();
    //get the user_ids for notification
    if ($this->notifyAgent) {
        //for system category
        if ($this->ticket->category_id === 1) {
            foreach(User::where('active', 1)
                        ->where(function (Builder $query) {
                            $query->where('email', 'y@x)
                                ->orWhere('email', 'y@x')
                                ->orWhere('email', 'y@x');
                        })->get() as $user) {
                $this->users[] = $user;
            }
        }
        else {
            $this->users[] = $this->ticket->agent;
        }
    }
    if ($this->ticketComment->user_id != $this->ticket->user_id)
        $this->users[] = $this->ticket->user;
}

/**
 * Get the channels the event should broadcast on.
 *
 * @return array<int, \Illuminate\Broadcasting\Channel>
 */
public function broadcastOn(): array
{
    $notify = [];
    foreach ($this->users as $user)
        $notify[] = new PrivateChannel($this->tenant->id.'.tickets.'.$user->id);

    return $notify;
}

public function broadcastWith()
{
    $user = User::where('id', $this->ticketComment->user_id)->first();
    return [
        'title' => 'Neuer Ticket Kommentar',
        'message' => 'Neuer Kommentar im Ticket "'.$this->ticket->subject.'" von '.$user->full_name,
        'ticket_id' => $this->ticket->id
    ];
}

} `

stancl commented 2 days ago

Double check that your queues are configured correctly per the documentation. If they are, create a minimal reproduction example that we can use. See #1260

jetwes commented 2 days ago

We have a correct configured redis queue which worked perfectly well since 1,5 years... I'll have to check wether i can give au minimal reproduction platform. It's a closed app.

stancl commented 2 days ago

A minimal reproduction is written from scratch, so it doesn't matter what business logic you have in your application.

jetwes commented 2 days ago

I'm aware of that - but i want to show it as detailled as possible. And i'll have to look what timeframe i have. I'll look what i can do.

stancl commented 2 days ago

It should have as little detail (and therefore as few variables and moving parts) as possible, hence minimal reproduction. The least amount of code/simplest setup that reproduces this, so that I can track down where the issue is.

iolk commented 12 hours ago

Similar issue here and downgrading to 3.8.4 resolves the issue. It looks like the subscribers are no longer in the tenant context.

We have a subscriber registered in the EventServiceProvider:

class JobEventSubscriber
{
    public function handleEvent(JobProcessed|JobFailed $event): void
    {
        Log::debug(tenant()?->id);
        // ^ this is null, in 3.8.4 this is not null 

        $jobBody = data_get(json_decode($event->job->getRawBody()), 'data.command');
        if ($jobBody == null) {
            return;
        }

        $job = unserialize($jobBody);
        // ^ this throws error "Database connection [tenant] not configured."
        // other stuff
    }

    public function subscribe(Dispatcher $events): array
    {
        return [
            JobProcessed::class => 'handleEvent',
            JobFailed::class => 'handleEvent',
        ];
    }
}
stancl commented 12 hours ago

That's kind of a separate issue since you're hooking into the queue lifecycle directly. The reports were that the old behavior in 3.8.4 made queue:restart signals not get noticed by the worker if it remained in the tenant context after processing a job, so the only thing we could do here would be adding some static properties to hook into our own listeners.