Fatal error when triggering Feeds via cron

Same problem with newest dev version of the module and core 8.3.3.

The error apperas, after I deleted a feed.

And now: if i want to run cron:

PHP Fatal error:  Call to a member function hasTranslation() on null in /home/testserver/www/exampe.ch/www.exampe.ch/webroot/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/ChangedItem.php on line 47

Fatal error: Call to a member function hasTranslation() on null in /home/testserver/www/exampe.ch/www.exampe.ch/webroot/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/ChangedItem.php on line 47
Drush command terminated abnormally due to an unrecoverable error.                                                                                                                                                                                   [error]
Error: Call to a member function hasTranslation() on null in /home/testserver/www/exampe.ch/www.exampe.ch/webroot/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/ChangedItem.php, line 47

I debugged this a bit and it seems that the import job is kept in queue after deleting the feed. So the queue runs even for deleted Feeds.

Or actually as there is an exception then it's the Feed::finishImport() that runs for feed that doesn't exist.

Queue items seem to be deleted when deleting FeedType, but not when we delete the Feed itself.

#4 Yes, feeds generate a row on queue mysql table.

My partial solution is removed all rows on queue table that relation feeds. Becareful wrong row if not related with feeds.

Any updates here, this issue still exist on latest alpha1, drupal core 8.5.1

Increasing priority to critical as it's cause the cron to fail each time it's run.

This test should reproduce the reported error.

Unserializing the task for deleted feed does not result into issues: the Feed instance can be unserialized safely, even if it refers to a non-existing feeds_feed entity. So a possible fix is to check if the feeds_feed entity still exists when initiating the queue task. The attached patch does this: In FeedRefresh::processItem() the code checks if the given feed entity still exists in the database. If it doesn't, it aborts.

Removing @group test from testQueueAfterDeletingFeed().

Adjusted FeedRefreshTest to be aware of new parameter $entity_type_manager in the contructor method of FeedQueueWorkerBase.

Fixed FeedQueueWorkerBaseTest + coding standard fixes.

I have tried this patch, it's fixes the issue. But we need to implement a hook_update_N to fix existing data in queue table in the database.

@Anas_maw
I believe we don't need a hook_update_N here, because the queue worker checks if the Feed from the incoming data still exists. Thus queue tasks for no longer existing feeds would automatically get cleaned up after applying the patch.

This way queue item will store much less data & queue items will load real data when worker pick task
Also probably it needs update hook oh extra check that deserialized $feed is not object for BC

+++ b/src/Plugin/QueueWorker/FeedRefresh.php
@@ -76,6 +78,12 @@ class FeedRefresh extends FeedQueueWorkerBase {
+    if (!$this->feedExists($feed)) {

@@ -111,6 +119,25 @@ class FeedRefresh extends FeedQueueWorkerBase {
+    $result = $this->entityTypeManager->getStorage($feed->getEntityTypeId())->getQuery()->condition('fid', $feed->id())->execute();

Looks better to load each feed before execution, this way you may store feed_id and if load failed skip execution

are you sure that entity query is fastest way to make sure entity exists?

@andypost
Good idea to reduce the amount of information stored in a queue item. I cannot oversee yet if that change would break things though. Some information is temporary stored on the Feed entity when an import does not complete in one cron run. It *might* be restored properly when reloading the feed entity, but I'm not completely sure. There is no test coverage yet for an import task taking multiple cron runs to complete (a test like that could be ported from the D7 version of Feeds: FeedsFileHTTPTestCase::testImportSourceWithMultipleCronRuns()). I therefore propose to postpone your change suggestion to a follow-up. Do you agree?

@andypost

are you sure that entity query is fastest way to make sure entity exists?

No, I'm not entirely sure if it is the fastest way, but it would be faster than a full entity load. I took the code from
https://drupal.stackexchange.com/questions/223853/verify-a-node-with-a-g....
A direct database call would probably be faster, but it would also be less flexible. Right now the code could in theory also work for other entity types that implement FeedInterface.

I think no reason to split this issue into follow-up (optimization) better to add BC check for already queued items
Just have no idea which check is faster is_scalar() or instanceof
Probably first is faster
Also entity load could use entity cache so result could be nearly the same as deserialization of job item

IMO the bug caused by storing entity which is orphaned(serialized) - so changing queue item fits into solution

Some information is temporary stored on the Feed entity when an import does not complete in one cron run.

Then you need custom data-object to serialize instead of feed or ID - that's purely job queue item data

+++ b/src/Plugin/QueueWorker/FeedRefresh.php
@@ -72,7 +74,10 @@ class FeedRefresh extends FeedQueueWorkerBase {
+    // Check if the feed still exists.
+    $feed = $this->feedLoad($feed);

I think better to add here another condition kinda is $feed a scalar and then load it

For this extra data better to port test cos it's not clear which data used to be stored in "entity" additionally

Probably I need to dig code deeper to get that

@MegaChriz can you point me to code which manipulate this data?

@andypost
Most of the temporary data on the feed entity is stored on State objects. These State objects are temporary stored on the feed entity on $feed->states and can be get by calling $feed->getState($stage) where $stage is the stage of the import: fetch, parse, process, etc.
These State objects are saved separately on Drupal's keyValue store as well, so these do not form a problem.

It could be that these State objects are everything what is stored temporary and in that case there is no issue with reducing the amount of data on the queue. But since there's a lot of code in Feeds, I cannot be sure of that and because of that we'd better have a test that covers the import process that needs multiple cron runs to complete.

Got it! So yes this needs special test for "steped run"
Maybe just emulate it like a unit test which will check state after each time queue process item?

I'm sure that test should be at queue level instead of cron

I'm sure that test should be at queue level instead of cron

The advantage of testing at the cron level (with a browser test) is that you can mimic the import process happening at two separate PHP processes. This way we can rule out that the import is working because something happens to be still in the static cache. The disadvantage is that it takes more time to write the test.

@andypost
Do you want to write a test that covers an import taking multiple cron runs to complete (aka an import happening in at least 2 separate PHP processes)? See FeedsFileHTTPTestCase::testImportSourceWithMultipleCronRuns() for the D7 version.

@MegaChriz I will try this weekend

@andypost
Cool, thanks in advance.

Some details about how a similar test was developed in D7:

• A CSV file with 9 entries selected for import.
• The queue time of the feeds queue was set to 5 seconds (default is 60 seconds).
• After saving each entity, there was one second of sleep: sleep(1);
• The number of items to be parsed was limited to 5 items.
• Because the queue was not allowed to pick up a new queue task after five seconds had passed and processing the first 5 items would take at least 5 seconds, the test could ensure that no more than 5 items were imported in one cron run. As a result there were still some items left to import for the next cron run.

For the above process to work in D8 there is one extra challenge though: each item to process is a queue task by itself. So to import 10 items, you'll end up with 12 queue tasks. One for fetching, one for parsing and 10 for processing. If the parse limit is brought down to 5 (the default is 50), it will be 13 queue tasks (two for parsing). In the D7 version of Feeds it is 1 queue task for importing 10 items with the default settings and with a parse limit of 5 it is 2 queue tasks.

The patch in #18 give me the following errors when run cron:
array_flip(): Can only flip STRING and INTEGER values! EntityStorageBase.php:227 [warning]
Object of class Drupal\feeds\Entity\Feed could not be converted to string ContentEntityStorageBase.php:1020 [error]

I have imported 100000 user throw feeds, i have 80000 row in queue table for feeds.
-When applying the patch in #15 the queue table row number increased and decreased.
-When applying the patch in #18 the queue table row number not changed.

+1

@andypost
Do you think you have time soon to work on the test? I would like to make a new release of Feeds in about two weeks, preferable with this fix in it. (Sorry if this makes you feel pushed, not sure how to formulate the question differently.)

Committed #15 because I want to include a fix for this in the next release. The optimizations done by @andypost will be postponed to a follow-up which I will create an issue for shortly.

@andypost
I opened a follow-up: #2978490: Optimize Feeds queue

again the issue

the newest drupal 8.7.8 and feeds version 8.3.x-alapha6

below is the error output, than you guys for help!

[error] Error: Call to a member function getPlugins() on null in Drupal\feeds\Entity\Feed->preSave() (line 512 of /var/www/html/docroot/modules/contrib/feeds/src/Entity/Feed.php) #0 /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(499): Drupal\feeds\Entity\Feed->preSave(Object(Drupal\feeds\FeedStorage))
#1 /var/www/html/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php(692): Drupal\Core\Entity\EntityStorageBase->doPreSave(Object(Drupal\feeds\Entity\Feed))
#2 /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(454): Drupal\Core\Entity\ContentEntityStorageBase->doPreSave(Object(Drupal\feeds\Entity\Feed))
#3 /var/www/html/docroot/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php(838): Drupal\Core\Entity\EntityStorageBase->save(Object(Drupal\feeds\Entity\Feed))
#4 /var/www/html/docroot/core/lib/Drupal/Core/Entity/EntityBase.php(394): Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object(Drupal\feeds\Entity\Feed))
#5 /var/www/html/docroot/modules/contrib/feeds/src/FeedImportHandler.php(305): Drupal\Core\Entity\EntityBase->save()
#6 /var/www/html/docroot/modules/contrib/feeds/src/Entity/Feed.php(260): Drupal\feeds\FeedImportHandler->startCronImport(Object(Drupal\feeds\Entity\Feed))
#7 /var/www/html/docroot/modules/contrib/feeds/feeds.module(90): Drupal\feeds\Entity\Feed->startCronImport()
#8 [internal function]: feeds_cron()
#9 /var/www/html/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(392): call_user_func_array('feeds_cron', Array)
#10 /var/www/html/docroot/core/lib/Drupal/Core/Cron.php(236): Drupal\Core\Extension\ModuleHandler->invoke('feeds', 'cron')
#11 /var/www/html/docroot/core/lib/Drupal/Core/Cron.php(134): Drupal\Core\Cron->invokeCronHandlers()
#12 /var/www/html/docroot/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#13 /var/www/html/vendor/drush/drush/src/Drupal/Commands/core/DrupalCommands.php(59): Drupal\Core\ProxyClass\Cron->run()
#14 [internal function]: Drush\Drupal\Commands\core\DrupalCommands->cron(Array)
#15 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#16 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#17 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#18 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#19 /var/www/html/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 /var/www/html/vendor/symfony/console/Application.php(1000): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))#21 /var/www/html/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#22 /var/www/html/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#23 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(118): Symfony\Component\Console\Application->run(Object(Drush\Symfony\DrushArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#24 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(49): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#25 /var/www/html/vendor/drush/drush/drush.php(72): Drush\Runtime\Runtime->run(Array)
#26 /var/www/html/vendor/drush/drush/includes/preflight.inc(18): require('/var/www/html/w...')
#27 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#28 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#29 {main}.

BTW, I deleted a bunch of modules to clean up a dev site and now I'm getting this cron error with version 8.9.3 of Drupal.