Commit 84d4e497 modified the Drupal\acquia_connector\Controller\SpiController::get() method to use ControllerBase's ConfigFactory, but this dependency is not injected when the class is used as a service during acquia_connector_cron(). As of version 8.x-1.17, this results in the following error during a cron run --

[error]  Error: Call to a member function getEditable() on null in Drupal\acquia_connector\Controller\SpiController->get() (line 69 of docroot/modules/contrib/acquia_connector/src/Controller/SpiController.php)
#0 docroot/modules/contrib/acquia_connector/src/Controller/SpiController.php(985): Drupal\acquia_connector\Controller\SpiController->get('cron')
#1 docroot/modules/contrib/acquia_connector/acquia_connector.module(107): Drupal\acquia_connector\Controller\SpiController->sendFullSpi('cron')
#2 [internal function]: acquia_connector_cron()
[...]
CommentFileSizeAuthor
#21 acquia_connector-3100935-21-8.x-2.x.patch1.97 KBdanepowell
#21 acquia_connector-3100935-21-8.x-1.x.patch1.82 KBdanepowell
#16 acquia_connector-3100935-16.patch2.18 KBdanepowell
#2 Call_to_a_member_function_getEditable_on_null_during_acquia_connector_cron-3100935-2.patch2.02 KBwells

Comments

wells created an issue. See original summary.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented
StatusFileSize
new Call_to_a_member_function_getEditable_on_null_during_acquia_connector_cron-3100935-2.patch2.02 KB

Not sure if this is the best approach, but the attached patch resolves this issue by injecting the config.factory service with the acquia_connector.spi service explicitly.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
jasonluttrell’s picture

jasonluttrell
Location Washington, DC area
commented

I tried the patch above (#2) and got the following error instead:

The website encountered an unexpected error. Please try again later.
ArgumentCountError: Too few arguments to function Drupal\acquia_connector\Controller\SpiController::__construct(), 1 passed in /PATH/TO/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php on line 269 and exactly 2 expected in Drupal\acquia_connector\Controller\SpiController->__construct() (line 54 of modules/contrib/acquia_connector/src/Controller/SpiController.php).

Drupal\acquia_connector\Controller\SpiController->__construct(Object) (Line: 269)
Drupal\Component\DependencyInjection\Container->createService(Array, 'acquia_connector.spi') (Line: 173)
Drupal\Component\DependencyInjection\Container->get('acquia_connector.spi') (Line: 158)
Drupal::service('acquia_connector.spi') (Line: 161)
Drupal\acquia_connector\Form\SettingsForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 520)
Drupal\Core\Form\FormBuilder->retrieveForm('acquia_connector_settings_form', Object) (Line: 277)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 50)
Drupal\ban\BanMiddleware->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 694)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

#4 — did you try a cache rebuild?

kregan’s picture

kregan commented

#5 - This didn't have any effect for me.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

#6 - Are you having the same result as #4?

Could either of you provide exact steps to get the error?

#4 looks like it is occurring when attempting to access the Acquia Connector settings form, which still works fine for me with this patch.

jasonluttrell’s picture

jasonluttrell
Location Washington, DC area
commented

--scratch what I said--

I will take a second look and get back to you.

kregan’s picture

kregan commented

#7 - I'm able to access the Acquia Connector settings form just fine as well as save changes. I'm receiving this error when attempting to send SPI data manually or via cron.

jasonluttrell’s picture

jasonluttrell
Location Washington, DC area
commented

OK, so I am not seeing it anymore. So it's possible I thought I had cleared the cache but didn't. I will let you know if I see a recurrence. Thanks.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

#9 - Manual send is working for me with this patch. Are you sure have done a cache rebuild? The "too few arguments" error would be expected with this patch before a cache rebuild but should definitely go away after.

kregan’s picture

kregan commented

#11 - After patching and clearing cache it worked. Thanks!

jsutta’s picture

jsutta commented

Patching and clearing cache worked for me too. ty!

danepowell’s picture

danepowell commented
Priority: Normal » Critical

Thanks everyone for the patches and reporting. If I understand correctly, this is breaking all cron runs, correct? Bumping issue priority accordingly.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

Thanks for chiming in, Dane.

Yes, this breaks the SPI communications both during cron runs and using the manual route. I'm not sure what the (or if there is a) "Drupal way" is for properly getting services injected when using a controller extending ControllerBase as a service and I couldn't find other examples of this behavior in the contrib I use. Hence the patch adding only the service the SPI controller needs.

danepowell’s picture

danepowell commented
Version: 8.x-1.x-dev » 8.x-2.x-dev
StatusFileSize
new acquia_connector-3100935-16.patch2.18 KB

This will need to go into 8.x-2.x first and get backported.

danepowell’s picture

danepowell commented

Well this is odd. Patches #2 (for 8.x-1.x) and #16 (for 8.x-2.x) both seem to fix the issue for me locally. But both fail on drupal.org. I don't understand why that would be.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

Do you mean the tests? They are failing on the base branches so I wasn't concerned about them. Or at least that was the case for patch #2.

danepowell’s picture

danepowell commented

The automated tests on drupal.org for both patches are failing with the same error. Here's the failure from #16 for instance: https://dispatcher.drupalci.org/job/drupal8_contrib_patches/16120/console

PHP Fatal error: Uncaught Error: Call to a member function getEditable() on null in /var/www/html/modules/contrib/acquia_connector/src/Controller/SpiController.php:84

It looks like the patch is correctly being applied. Yet the error is still thrown in both patches in the tests.

danepowell’s picture

danepowell commented

Ah, the SPI test class overrides the SPI controller and doesn't call its parent constructor, that's likely why the tests are failing. I don't think it's related to the patch itself, and we can follow up to fix that later.

danepowell’s picture

danepowell commented
StatusFileSize
new acquia_connector-3100935-21-8.x-1.x.patch1.82 KB
new acquia_connector-3100935-21-8.x-2.x.patch1.97 KB

I trimmed the patches slightly and am committing to dev now. New releases 8.x-2.0-beta2 and 8.x-1.18 will be out shortly.

I also found an example of this pattern in core, which makes me more confident that it's correct: https://git.drupalcode.org/project/drupal/blob/8.8.x/core/modules/system...

  • Dane Powell authored e88009b on 8.x-1.x 
    Issue #3100935 by wells, Dane Powell: "Call to a member function...

  • Dane Powell authored 419a2c5 on 8.x-2.x 
    Issue #3100935 by wells, Dane Powell: "Call to a member function...
danepowell’s picture

danepowell commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.