Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PDOException thrown when creating a new tenant from Nova (PHP 8) #720

Closed
WillyReyno opened this issue Sep 16, 2021 · 12 comments
Closed

PDOException thrown when creating a new tenant from Nova (PHP 8) #720

WillyReyno opened this issue Sep 16, 2021 · 12 comments
Assignees

Comments

@WillyReyno
Copy link

WillyReyno commented Sep 16, 2021

Describe the bug

Since upgrading to PHP 8, the Tenant creation process does not work properly when triggered from a Nova resource.

We created a Tenant Resource in Laravel Nova which was working perfectly fine with PHP 7.4, the events were properly triggered, and we could use our new tenant app without any issue.

Now we are getting a There is no active transaction PDO Exception

The exception does not block the JobPipeline, the tenant is created in the central database, and the tenant database is also properly created & migrated.

I can also confirm the exception is not thrown when manually creating a Tenant with new Tenant() or within seeders.

I don't know if the following can help, but here's what I found after a few searches :

Steps to reproduce

Using PHP 8 :

  • Create a Tenant Resource in Laravel Nova
  • Use the Create form on the resource to create a new tenant
  • See the Exception.

Setup

  • PHP 8.0.10
  • Laravel v8.61.0
  • Laravel Nova v3.29.0
  • stancl/tenancy v3.4.4
@WillyReyno WillyReyno added the bug Something isn't working label Sep 16, 2021
@stancl
Copy link
Member

stancl commented Sep 16, 2021

Is this related to tenant creation or just Nova? And if it's just Nova, how does it create tenants?

@WillyReyno
Copy link
Author

I think it's just Nova, because when creating a tenant through a seeder or a simple route containing $tenant = new Tenant(); [...] $tenant->save() it doesn't throw any exception.

What do you mean how?

All I know is that I'm using Nova the regular way, meaning that I have a Resource (CRUD) for my Tenant and when I use the creation form :

  • the Tenant is correctly created in my central db
  • the event in JobPipeline of my TenancyServiceProvider are triggered :
    • Tenant database is created
    • Tenant database is seeder
  • But after that, the exception is thrown.

Thanks for your help

@stancl
Copy link
Member

stancl commented Sep 16, 2021

What if you queue the DB creation?

@stancl stancl removed the bug Something isn't working label Sep 16, 2021
@WillyReyno
Copy link
Author

It works!

The only issue I had was regarding my TenantObserver's created method, which was throwing a TenantDatabaseDoesNotExistException, but I moved the code into a Job in the JobPipeline and everything works fine now.

Thanks for your help!

@stancl
Copy link
Member

stancl commented Sep 17, 2021

Right, the issue then was probably Nova wrapping the create logic in a transaction, but Tenancy would also handle DB creation which now can't be inside a DB transaction

@jwktje
Copy link

jwktje commented Sep 23, 2021

I'm having this issue too. Was the solution to queue something somewhere? Can you explain how you fixed it @WillyReyno?

@WillyReyno
Copy link
Author

WillyReyno commented Sep 23, 2021

@jwktje in TenantServiceProvider.php I changed shouldBeQueued(false) to shouldBeQueued(true) on TenantCreated's JobPipeline.

For example:

Events\TenantCreated::class => [
                JobPipeline::make([
                    Jobs\CreateDatabase::class,
                    Jobs\MigrateDatabase::class,
                ])->send(function (Events\TenantCreated $event) {
                    return $event->tenant;
                })->shouldBeQueued(true), // here
            ],

I already had queues running on my project since we use them for various stuff, so that was the only thing to do for me.

@jwktje
Copy link

jwktje commented Sep 24, 2021

@WillyReyno Yup that fixed it. That's great! Thanks for the quick response. This makes a lot of sense.

@ugurdnlr
Copy link

This method did not solve my problem. I still keep getting the same error when I create tenant from Nova. It does all the operations (such as creating a database, adding records) but throw exception

@scramatte
Copy link

Same problem here with Nova 4

@sajidparwez
Copy link

just wrap the code in event() method in TenancyServiceProvider.php file.

public function events()
{

    try {
        \DB::beginTransaction();
        return [
            // Tenant events
            Events\CreatingTenant::class => [],
            Events\TenantCreated::class => [
                JobPipeline::make([
                    Jobs\CreateDatabase::class,
                    Jobs\MigrateDatabase::class,
                    // Jobs\SeedDatabase::class,

                    // Your own jobs to prepare the tenant.
                    // Provision API keys, create S3 buckets, anything you want!

                ])->send(function (Events\TenantCreated $event) {
                    return $event->tenant;
                })->shouldBeQueued(true), // `false` by default, but you probably want to make this `true` for production.
            ],
            
            Events\SavingTenant::class => [],
            Events\TenantSaved::class => [],
            Events\UpdatingTenant::class => [],
            Events\TenantUpdated::class => [],
            Events\DeletingTenant::class => [],
            Events\TenantDeleted::class => [
                JobPipeline::make([
                    Jobs\DeleteDatabase::class,
                ])->send(function (Events\TenantDeleted $event) {
                    return $event->tenant;
                })->shouldBeQueued(true), // `false` by default, but you probably want to make this `true` for production.
            ],

            // Domain events
            Events\CreatingDomain::class => [],
            Events\DomainCreated::class => [],
            Events\SavingDomain::class => [],
            Events\DomainSaved::class => [],
            Events\UpdatingDomain::class => [],
            Events\DomainUpdated::class => [],
            Events\DeletingDomain::class => [],
            Events\DomainDeleted::class => [],

            // Database events
            Events\DatabaseCreated::class => [],
            Events\DatabaseMigrated::class => [],
            Events\DatabaseSeeded::class => [],
            Events\DatabaseRolledBack::class => [],
            Events\DatabaseDeleted::class => [],

            // Tenancy events
            Events\InitializingTenancy::class => [],
            Events\TenancyInitialized::class => [
                Listeners\BootstrapTenancy::class,
            ],

            Events\EndingTenancy::class => [],
            Events\TenancyEnded::class => [
                Listeners\RevertToCentralContext::class,
            ],

            Events\BootstrappingTenancy::class => [],
            Events\TenancyBootstrapped::class => [],
            Events\RevertingToCentralContext::class => [],
            Events\RevertedToCentralContext::class => [],

            // Resource syncing
            Events\SyncedResourceSaved::class => [
                Listeners\UpdateSyncedResource::class,
            ],

            // Fired only when a synced resource is changed in a different DB than the origin DB (to avoid infinite loops)
            Events\SyncedResourceChangedInForeignDatabase::class => [],
        ];
        
      
        \DB::commit();
      } catch(\Exception $e) {
         \DB::rollback();
         // Handle Error
      }


    
}

@stancl
Copy link
Member

stancl commented Jul 27, 2023

That’s not how PHP works, the try {} will not do anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants