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

[Octane] Event use handle instead of asListener #136

Closed
exodusanto opened this issue Oct 5, 2021 · 12 comments · Fixed by #235
Closed

[Octane] Event use handle instead of asListener #136

exodusanto opened this issue Oct 5, 2021 · 12 comments · Fixed by #235
Labels
help wanted Extra attention is needed

Comments

@exodusanto
Copy link

Using Laravel Octane seems that actions are run as object instead of listener.

The problem is similar to #87, the workaround on EventServiceProvider event/listener registration works.

UserUpdated::class => NotifyUserUpdated::class.'@asListener'

@lorisleiva
Copy link
Owner

Hi there 👋

Thanks for raising this! The issue with #87 was that Laravel Actions was not able to recognise the action as being a listener due to a lack of recognised frames.

Since I've not used Octane yet, would you be able to tell me how Laravel dispatches listeners when used with Octane? It could be using a different method or perhaps even a different class than the ones listed above which would cause Laravel Actions to default to returning an object.

@exodusanto
Copy link
Author

exodusanto commented Oct 5, 2021

This is a trace of the exception.

#0 /.../vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(424): App\\Actions\\User\\SetupCompleted->handle(Object(App\\Events\\UserUpdated))
#1 /.../vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(249): Illuminate\\Events\\Dispatcher->Illuminate\\Events\\{closure}('App\\\\Events\\\\User...', Array)
#2 /.../vendor/nuwave/lighthouse/src/Schema/Directives/EventDirective.php(48): Illuminate\\Events\\Dispatcher->dispatch('App\\\\Events\\\\User...')

Note:
I'm using lighthouse graphql plugin, but this problem is only with octane serve

@lorisleiva
Copy link
Owner

According to your stacktrace, Laravel Actions should recognise it as a Listener (see code below).

return $frame->matches(Dispatcher::class, 'dispatch')

Is there anything unusual about how you dispatch these events?

@exodusanto
Copy link
Author

exodusanto commented Oct 10, 2021

I made some tests and I think that this part

protected function extendActions(ActionManager $manager)
{
$this->app->beforeResolving(function ($abstract, $parameters, Application $app) use ($manager) {
if (! class_exists($abstract) || $app->resolved($abstract)) {
return;
}
$manager->extend($abstract);
});
}
}

extends only some classes, maybe octane create a new application instance like there

Edit: I added an var_dump before the $manager->extend($abstract); and only Actions registred as command are logged

@lorisleiva
Copy link
Owner

Hey 👋 Sorry for the late reply. I finally got around to checking it out. Could you try to use dev-main and let me know if that fixes your issue?

@exodusanto
Copy link
Author

Hi, thank you for your fix, but it doesn't work.

I have the same exception:

App\Actions\User\SetupCompleted::handle(): Argument #1 ($user) must be of type App\Models\User, App\Events\UserUpdated given, called in /.../vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php on line 424

I also made some tests but I'm worried that beforeResolving is not supported from Octane, or for integrate it this package need to register some logic into octane.php file

 RequestReceived::class => [
            ...Octane::prepareApplicationForNextOperation(),
            ...Octane::prepareApplicationForNextRequest(),
            //
        ],

@lorisleiva lorisleiva added the help wanted Extra attention is needed label Nov 11, 2021
@lorisleiva
Copy link
Owner

No worries, thanks for trying!

If anyone using Octane would like to open a PR for this, that would be most appreciated. ❤️

@exodusanto
Copy link
Author

Maybe @nunomaduro can give us a hit if nowadays Octane support beforeResolving in some way

@nunomaduro
Copy link

Are you able to make a pull request in Octane with a failing test? Or create an issue, with a repository example, so I can reproduce the issue locally?

@exodusanto

This comment was marked as duplicate.

@Wulfheart
Copy link
Collaborator

@exodusanto I think he meant that you should create an issue in the Octane repo itself.

@exodusanto
Copy link
Author

exodusanto commented Oct 4, 2022

Not yet 😔, I'll find some time next weeks

lorisleiva pushed a commit that referenced this issue Jun 11, 2023
* Fix #136 Octane Event use handle instead of asListener

* Create the ActionManager as a scoped singleton instance
jaulz pushed a commit to jaulz/laravel-actions that referenced this issue Jul 27, 2023
* Fix lorisleiva#136 Octane Event use handle instead of asListener

* Create the ActionManager as a scoped singleton instance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants