Fatal error at PKPEmailHandler on bulk email sending

[jonasraoni]

Describe the bug
See error log below, the origin is here:

pkp-lib/api/v1/_email/PKPEmailHandler.php

Line 168 in 566c546

Queue::push(function () use ($userIds, $contextId, $subject, $body, $fromEmail, $fromName) {

And fails due to this bug in Laravel (you cannot call isset($closure->property) on a closure):
https://github.com/laravel/framework/blob/ab7586b3b7303958f343c84531b021b1055bd6a3/src/Illuminate/Queue/Queue.php#L328

Slim Application Error:
Type: Error
Message: Closure object cannot have properties
File: /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Queue/Queue.php
Line: 328
Trace: #0 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Queue/Queue.php(304): Illuminate\Queue\Queue->shouldDispatchAfterCommit(Object(Closure))
#1 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Queue/DatabaseQueue.php(97): Illuminate\Queue\Queue->enqueueUsing(Object(Closure), '{"uuid":"9adb69...', 'email_640120019...', NULL, Object(Closure))
#2 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Queue/QueueManager.php(291): Illuminate\Queue\DatabaseQueue->push(Object(Closure), Array, 'email_640120019...')
#3 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(337): Illuminate\Queue\QueueManager->__call('push', Array)
#4 /var/www/html/lib/pkp/api/v1/_email/PKPEmailHandler.php(168): Illuminate\Support\Facades\Facade::__callStatic('push', Array)
#5 [internal function]: PKP\API\v1\_email\PKPEmailHandler->create(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Array)
#6 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func(Array, Object(Slim\Http\Request), Object(PKP\core\APIResponse), Array)
#7 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke(Array, Object(Slim\Http\Request), Object(PKP\core\APIResponse), Array)
#8 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#9 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#10 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/App.php(503): Slim\Route->run(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#11 /var/www/html/lib/pkp/classes/security/authorization/internal/ApiAuthorizationMiddleware.php(87): Slim\App->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#12 [internal function]: PKP\security\authorization\internal\ApiAuthorizationMiddleware->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Slim\App))
#13 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(PKP\security\authorization\internal\ApiAuthorizationMiddleware), Array)
#14 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Slim\App))
#15 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Slim\App))
#16 /var/www/html/lib/pkp/classes/security/authorization/internal/ApiCsrfMiddleware.php(54): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#17 [internal function]: PKP\security\authorization\internal\ApiCsrfMiddleware->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#18 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(PKP\security\authorization\internal\ApiCsrfMiddleware), Array)
#19 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#20 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#21 /var/www/html/lib/pkp/classes/security/authorization/internal/ApiTokenDecodingMiddleware.php(141): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#22 [internal function]: PKP\security\authorization\internal\ApiTokenDecodingMiddleware->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#23 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(PKP\security\authorization\internal\ApiTokenDecodingMiddleware), Array)
#24 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#25 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#26 /var/www/html/lib/pkp/classes/handler/APIHandler.php(84): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#27 [internal function]: PKP\handler\APIHandler->PKP\handler\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#28 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(Closure), Array)
#29 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#30 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#31 /var/www/html/lib/pkp/classes/handler/APIHandler.php(143): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#32 [internal function]: PKP\handler\APIHandler->PKP\handler\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#33 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(Closure), Array)
#34 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#35 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#36 /var/www/html/lib/pkp/classes/handler/APIHandler.php(148): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#37 [internal function]: PKP\handler\APIHandler->PKP\handler\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#38 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(Closure), Array)
#39 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#40 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#41 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->Slim\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#42 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#43 /var/www/html/lib/pkp/classes/handler/APIHandler.php(140): Slim\App->process(Object(Slim\Http\Request), Object(PKP\core\APIResponse))
#44 [internal function]: PKP\handler\APIHandler->PKP\handler\{closure}(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#45 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Object(Closure), Array)
#46 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(PKP\core\APIResponse), Object(Closure))
#47 /var/www/html/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(PKP\core\

Re-production Steps

1. Make sure that bulk email feature is enable for context(journal/server/...) . Enable it from Administration --> Site Settings --> Bulk Emails --> Select specific context --> Save
2. Go to Context(Journal/Server/....) --> User & Roles --> Notify
3. Try to send bulk mail

What application are you using?
OJS 3.4.0 RC1

Additional Information
Details reason at #8734 (comment)

PRs
pkp-lib --> #8827
ui-library --> pkp/ui-library#270
ojs --> pkp/ojs#3838 [TEST ONLY]

[jonasraoni]

I've created an issue at the Laravel repository laravel/framework#46355

[asmecher]

@jonasraoni, what are the reproduction steps to cause this? (This will help determine whether it's a must-fix for 3.4 or can wait on Laravel's schedule.)

[touhidurabir]

This has been resolved in PHP 8.1+ but an issue before that, see the test at https://onlinephp.io/c/dbcd1 . The best solution for this is to extract the code within the Queue::push() to a separate job class and pass an instance of that to Queue::push() .

[jonasraoni]

Yep, the Laravel folks said the same and asked for a fix: laravel/framework#46355
Perhaps I'll create a PR later (e.g. $job instanceof Closure).

[Vitaliy-1]

Might be useful for this issue:

• Example of job batching: https://github.com/pkp/pkp-lib/blob/main/api/v1/announcements/PKPAnnouncementHandler.php#L256
• I've also created a BULK_EMAIL_SIZE_LIMIT const, which is now in Mailer: https://github.com/pkp/pkp-lib/blob/main/classes/mail/Mailer.php#L46

[touhidurabir]

@NateWr can you please review the PRs . The bulk emailing feature now handled by batch queue process with few enhancements .

[NateWr]

@Vitaliy-1 can you code review this? I worked on this back before all of @touhidurabir's work on jobs and your work on emails. I think you're more familiar with best practices here.

[jonasraoni]

A fix has already been added upstream, so the next release of Laravel 9 should include it :)

[Vitaliy-1]

@touhidurabir, looks good! I also think that the separate API endpoint to process the queue isn't needed anymore as the concept has changed. I left a comment, can you check whether we duplicate registering of the database connector with this PR?

[touhidurabir]

@Vitaliy-1 I have replied to your reviews, please check those .

[Vitaliy-1]

Thanks, @touhidurabir. Think that it's ready to be merged after the tests.