Error we are seeing:

Error: Call to a member function unlink() on null in Drupal\feeds\Result\HttpFetcherResult->cleanUp() (line 60 of /mnt/www/html/secret/docroot/modules/contrib/feeds/src/Result/HttpFetcherResult.php)

Stacktrace

Error: Call to a member function unlink() on null in Drupal\feeds\Result\HttpFetcherResult->cleanUp() (line 60 of /mnt/www/html/secret/docroot/modules/contrib/feeds/src/Result/HttpFetcherResult.php)
#0 /mnt/www/html/secret/docroot/modules/contrib/feeds/src/FeedsExecutable.php(335): Drupal\feeds\Result\HttpFetcherResult->cleanUp()
#1 /mnt/www/html/secret/docroot/modules/contrib/feeds/src/FeedsExecutable.php(119): Drupal\feeds\FeedsExecutable->finish()
#2 /mnt/www/html/secret/docroot/modules/contrib/feeds/src/Plugin/QueueWorker/FeedRefresh.php(42): Drupal\feeds\FeedsExecutable->processItem()
#3 /mnt/www/html/secret/docroot/core/lib/Drupal/Core/Cron.php(183): Drupal\feeds\Plugin\QueueWorker\FeedRefresh->processItem()
#4 /mnt/www/html/secret/docroot/core/lib/Drupal/Core/Cron.php(139): Drupal\Core\Cron->processQueues()
#5 /mnt/www/html/secret/docroot/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#6 /mnt/www/html/secret/vendor/drush/drush/src/Drupal/Commands/core/DrupalCommands.php(69): Drupal\Core\ProxyClass\Cron->run()
#7 [internal function]: Drush\Drupal\Commands\core\DrupalCommands->cron()
#8 /mnt/www/html/secret/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array()
#9 /mnt/www/html/secret/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#10 /mnt/www/html/secret/vendor/consolidation/annotated-command/src/CommandProcessor.php(176): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#11 /mnt/www/html/secret/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(391): Consolidation\AnnotatedCommand\CommandProcessor->process()
#12 /mnt/www/html/secret/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#13 /mnt/www/html/secret/vendor/symfony/console/Application.php(1039): Symfony\Component\Console\Command\Command->run()
#14 /mnt/www/html/secret/vendor/symfony/console/Application.php(275): Symfony\Component\Console\Application->doRunCommand()
#15 /mnt/www/html/secret/vendor/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#16 /mnt/www/html/secret/vendor/drush/drush/src/Runtime/Runtime.php(124): Symfony\Component\Console\Application->run()
#17 /mnt/www/html/secret/vendor/drush/drush/src/Runtime/Runtime.php(51): Drush\Runtime\Runtime->doRun()
#18 /mnt/www/html/secret/vendor/drush/drush/drush.php(79): Drush\Runtime\Runtime->run()
#19 /mnt/www/html/secret/vendor/drush/drush/drush(4): require('...')
#20 /mnt/www/html/secret/vendor/bin/drush(120): include('...')
#21 {main}
CommentFileSizeAuthor
#3 feeds-3390530.patch498 bytessarwan_verma

Issue fork feeds-3390530

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jnicola created an issue. See original summary.

jnicola’s picture

jnicola commented

Relevant contrib code:

/**
   * {@inheritdoc}
   */
  public function cleanUp() {
    if ($this->filePath) {
      $this->fileSystem->unlink($this->filePath);
    }
  }

Offhand the error appears to be the sanity check is looking for if there is a filepath, and if so, call that method on filesystem. Welp, it's possible apparently to get to that point with a filesystem being established.

File system is set in the constructor:

  /**
   * Constructs an HttpFetcherResult object.
   *
   * @param string $file_path
   *   The path to the result file.
   * @param array $headers
   *   An array of HTTP headers.
   * @param \Drupal\Core\File\FileSystemInterface $file_system
   *   (optional) The file system service.
   */
  public function __construct($file_path, array $headers, FileSystemInterface $file_system = NULL) {
    parent::__construct($file_path);
    $this->headers = array_change_key_case($headers);
    if (is_null($file_system)) {
      $file_system = \Drupal::service('file_system');
    }
    $this->fileSystem = $file_system;
  }

In theory... we either get NULL for file system or an object that meets the FileSystemInterface requirement, which in turn has the method UnLink, so that's a dead end. So we have to be passing in NULL, which in turn calls \Drupal::service('file_system');... which may be returning NULL in some instances?

It's late and that's as far as I've puzzled.

sarwan_verma’s picture

sarwan_verma commented
StatusFileSize
new feeds-3390530.patch498 bytes

Hi @jnicola,
I have fixed this issue Call to a member function unlink() on null in Drupal\feeds\Result\HttpFetcherResult->cleanUp() and also attached patch,
please review and verify.

sarwan_verma’s picture

sarwan_verma commented
Status: Active » Needs review
jnicola’s picture

jnicola commented

Nice, thanks for the patch! A very easy fix indeed. We may utilize it. I guess I just wonder if this is a symptom of a larger issue or if this should have some logic in the constructor and throw an exception there about not having a file system instantiated yet, etc etc.

megachriz’s picture

megachriz
Primary language Dutch
commented

Thanks for the patch, but I think it is not the right solution. This solution essentially conditionally skips the cleanup. This could theoretically lead to a non-stop growing filesystem. I say theoretically, because a cleanup of the same file can happen at an other stage in the process as well.

I wonder how this issue can occur. If possible, it would be helpful if you could find the steps to reproduce the issue. Though I can understand that it could be difficult to figure out in this case.

For what's worth: in an earlier version of the code there was a procedural function call. This later had to be replaced with dependency injection because the procedural function got deprecated.

It looks like that somehow that dependency got emptied. Could it be that that happens when serializing and unserializing again?

jnicola’s picture

jnicola commented

Man I wish we could get you steps to reproduce! We've been trying to consistently get this and it just happens one time out of a thousand or so. One theory I have is it is happening when "poormans cron" is triggered on a Drupal request resulting from an AJAX request, and Drupal isn't fully bootstrapped. So NULL is passed in, and then it attempts to load the Drupal service and that in turn returns NULL but FEEDS isn't looking for that.

Another coworker says they can trigger this all from CLI drush cron calls though so... maybe I'm not right about that?

heikkiy’s picture

heikkiy commented

We encountered this while upgrading our site from Drupal 9 Drupal 10.

I initially thought the issue in our case might be that we moved the production database to staging. And we did not move the files at the same time. And it seems like feeds is trying to cleanup a temporary file that is not present anymore.

I added some logging and noticed that it was trying to remove the following file:
/var/www/html/shared/temporary/feeds_http_fetcher15nWQr

When looking at the file system, I can see that the temporary file is there. The problem seems to be that other temporary files are getting more permissions:
-rwxrwxrwx.

But the feed temporary files are just
-rw-------.

This might cause the issue that the temporary file cannot be unlinked. Other option might also be that some of the temporary files are leftovers actually from production and it's unable to unlink them. It's a bit difficult task to understand what feed temporary file might be returning NULL. If someone can give pointers how to debug this, I can take a more closer look.

EDIT: I tried changing some of the temporary file permissions for feeds but seems it's still unable to unlink them. The files are still present in the server but it seems like cron stopped giving errors and few latest cron runs finished without new logs.

heikkiy’s picture

heikkiy commented
Related issues: +#3080098: Delete Empty Files Created If Empty/Error Exception is Returned

We noticed that we are using a patch from another issue that is very similar to this one in another project of ours, #3080098: Delete Empty Files Created If Empty/Error Exception is Returned. It's also related to unlinking files from the system. I also tested that changing file permissions alone did not fix the issue in our project.

heikkiy’s picture

heikkiy commented

A small update. I tested the patch from #3080098: Delete Empty Files Created If Empty/Error Exception is Returned. It did not solve the unlink() error. Testing this patch got rid of the error and now the feeds started to actually update and I get messages like this:

Feed Source: There are no new RSS items.

I think the unlink() error was fatal and it never was able to process the feeds completely.

megachriz’s picture

megachriz
Primary language Dutch
commented
Assigned: Unassigned » megachriz

Hm, I've got an idea about what could be the issue. I see that the property $fileSystem is new in Feeds 8.x-3.0-beta4 (I thought it was there for much longer, but I may got confused with an other class). So if there were any imports not completed yet before updating to Feeds 8.x-3.0-beta4, this error could occur.

So the fix would be to ensure that the service exists when unserializing a HttpFetcherResult object.

Let's see if I can come up with a fix.

MegaChriz opened merge request !147

MegaChriz opened merge request !148

megachriz’s picture

megachriz
Primary language Dutch
commented
Assigned: megachriz » Unassigned

I've provided a possible fix. :)

@HeikkiY
The issue of files not getting cleaned up or fail to get cleaned up sounds like an other issue. There are a few other issue open related to that issue, including the issue you already linked:
#3080098: Delete Empty Files Created If Empty/Error Exception is Returned
#3372368: in_progress filesystem grows indefinitely
#3158678: FetcherResult::checkFile() throws \RuntimeException, which is never caught

maskedjellybean’s picture

maskedjellybean
Location Portland, OR
commented

Thank you @MegaChriz for merge request 147 in comment #11. Applying the patch fixed the error and allowed cron to finish! I work with @jnicola so this addresses the problem the issue was created for.

However after applying the patch, on the first cron run I saw this warning:

[warning] unlink(/mnt/tmp/projectname/feeds_http_fetcherw5KhhB): No such file or directory FileSystem.php:124

I have not seen this warning on subsequent cron runs.

  • MegaChriz committed 12933dcb on 8.x-3.x 
    Issue #3390530 by MegaChriz, sarwan_verma, jnicola, HeikkiY,...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

I've merged the code!

The unlink warning tells us that the file that Feeds tried to delete was already deleted. I'm not sure if that issue is worth to address. Maybe when it would happen a lot. But since 8.x-3.0-beta4, Feeds now stores http downloaded files in a in_progress directory on either the private or the public filesystem. So things have changed that much, that the warning may only occur for data stored by Feeds before 8.x-3.0-beta4.

Status: Fixed » Closed (fixed)

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