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

Use \Closure instead of callable - fix #401 #403

Merged
merged 1 commit into from
Jul 20, 2021
Merged

Use \Closure instead of callable - fix #401 #403

merged 1 commit into from
Jul 20, 2021

Conversation

rodrigopedra
Copy link
Contributor

PR #398 fixed and issue with Laravel Vapor, but changed \Closure usage to callable.

Problem is Laravel Event Dispatcher treats \Closure...

https://github.com/laravel/framework/blob/c7824d4e163947331d2676dc35213126499736a1/src/Illuminate/Events/Dispatcher.php#L79-L81

... differently than callables ...

https://github.com/laravel/framework/blob/c7824d4e163947331d2676dc35213126499736a1/src/Illuminate/Events/Dispatcher.php#L87-L93

... as such $this refers to different objects when using these two different syntax.

both snippets are from the same Dispatcher@listen method

which causes issues with queued jobs as reported in #401

For example after updating to 2.11.1 I got these errors on production:

image

I reverted back to 2.11.0 to avoid issues.

This PR keeps the Vapor related changes from PR #398 (not using the looping listener) but reverts to call $this->resetFlare(); from within a \Closure (anonymous function) so the $this context is preserved.

I didn't added any automated tests as this was an emergency or hot fix to #401 but I tested manually symlinking the fixed repo to my local app.

(closes #401)

@rodrigopedra
Copy link
Contributor Author

PR #402 is an alternative fix to the issue #401

@rodrigopedra
Copy link
Contributor Author

@AlexVanderbist
Copy link
Contributor

That's an unfortunate oversight on our end. Thanks for providing a fix so quickly. Tagging it as 2.11.2 in a minute.

@rodrigopedra
Copy link
Contributor Author

@AlexVanderbist don't worry, that one was a hard to spot .

Thanks for looking into it so fast, and sorry for spamming y'all with notifications.

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

Successfully merging this pull request may close these issues.

Call to protected method 'resetFlare'
2 participants