PHP 5.6: Fatal error during import: FormattableMarkup::__toString() must not throw an exception

Did you add any mappings? On the Feed type edit form there is a tab called 'Mapping'. (At some point I would like to add a button called "Save and add mapping" on the feed type add form, so you go directly to mapping after creating a feed type.)

Anyway, if you did *not* add any mappings, I think Feeds should warn you when creating a feed.

I run into the same problem when importing a csv-file with mapping for title as unique field.

It sounds like that there was a validation error and reporting that validation error resulted into another error.

In order to help, I will need details about your configuration plus a sample of the feed you are importing (@jpoesen already provided a sample source file). Can you provide the following:

• The source file you were trying to import (may also be a sample file, as long as that sample file can demonstrate the problem).
• An export of your configuration, packed inside a feature module, which includes:
  • The Feed type;
  • The content type selected on the processor;
  • Fields used on your content type;
  • Field storage items of your fields;
  • Other configuration that is related.

Here is feature module, and test file that reproduces the bug is feeds/tests/resources/csv/content.csv

And steps to reproduce

1. Add a required field to any entity type
2. Create importer that imports entities of that type
3. Do not add any mappings to this required field

and this happens when I want to import additional data to existing entities - but I do not want to create new entities if existing entity is not found.

Thanks for all your work on feeds. I had similar errors - wrote up a huge issue thread stepping through what I was doing and then d.o logged me out and I lost it last night.

I think that some things are treated as required when they are not required in the content type / field level . It should just skip the cell/field instance instead of trying to populate from an empty cell.

If I try to use http addresses to populate the filefields/imagefields it is quite finicky. Typos in urls will produce 500 php errors instead of continuing to the next cell.

I would suggest making it easier to put feeds into a debug mode that dumps more information like the $url value and major values from the CSV row like title, into the logger, so it is easier to find bad data. Also somehow getting exception information into the logger would help people.

In my case I confirmed when trying to import image files and/or vcf text files . the src/Feeds/Target/File.php: getFileName($url) function gets called when there is no file name, eg when a cell is empty. So the test is actually 'false' value is compared to $this->fileExtensions . Then it throws exception. This may be more prone to happen on the last node / csv row, it seemed like when i cut off my csv file to only a couple rows it would still throw errrors.

On mine this is line 190 but normally is line 188 - i tried dumping some vars in the process but it didn't work. (they are commented out right now)

PHP Fatal error:  Method Drupal\\Component\\Render\\FormattableMarkup::__toString() must not throw an exception, caught Drupal\\feeds\\Exception\\TargetValidationException:  in /web/modules/contrib/feeds/src/Feeds/Target/File.php on line 190, referer: /batch?id=48&op=start

This is the exception: throw new TargetValidationException('The file, %url, failed to save because the extension, %ext, is invalid.', ['%url' => $url, '%ext' => $extension]);

It was called from getFile - line 156: $filepath = $destination . '/' . $this->getFileName($value);

I will post more as I can determine it. Thank you!

@mikran
I tried to reproduce the bug reported in this issue with your feature module. The import did not succeed indeed, but I did not get a fatal error. Only a message that field_debug should not be empty. This happened after adding a feed and clicking on "Save and import". Did you initiate an import in a other way to trigger the fatal error?

That an import does not complete because of missing values for a field you did not map to is indeed annoying, but it is Drupal's way of saying "I do not accept any entity that is missing a value for a required field". Personally, I think Feeds should indeed skip validation for fields that you don't map to, but on the other hand Drupal has a point that it should not allow invalid data. An entity in the database where a required field is empty is invalid. It is an issue in the D7 version of Feeds as well: #2918840: Field validation errors in item: only validate mapped fields. In that issue I posted a patch to limit the validation to only mapped fields.

@HongPong
Your issue with empty values for file targets sounds a lot like #2772595: When mapping to File or Image fields, empty source for target_id causes Exception and breaks import process.. Is it the same or is your issue different?

We should definitely catch errors in case of a typo in an URL.

Logging failed imports is on my wishlist too. I was thinking such logging feature could be modeled after the commerce_log module (part of the Commerce project). At least one thing I think we should aim for when developing that feature is that the logs can be written to something else as the database. That was a feature request in the D7 version of Feeds: #1367044: Use syslog.module for feeds_log and/or an option to disable log. There is already an issue open about adding a logging feature for D8: #2907721: Log items that failed to import.

@MegaChriz Yeah I think my error is similar but I got similar errors from both vcf text files and images. So I think we need to look carefully at File.php. If I have some luck with Xdebug I'll try to nail this down more & possibly file a separate issue if it is truly different than these.

@MegaChriz
Ok, I figured it out what we're doing differently. PHP 5.6.33 throws this error, PHP 7.0.29 does not.

jpoesen reported PHP version as 7.0.22 that produced this error, so is it something between PHP 7.0.22 and 7.0.29 that changes this?

I'm running php version 5.6.30 and am also seeing this issue. From what I can tell, using the FormattableMarkup object as the message causes PHP to think that it is the FormattableMarkup object throwing an error, which isn't allowed. Forcing it to a string fixes the error for me. Not sure if having a string stored as the message messes with anything else.

My STR are similar.
Fresh install of D8. Enabled feeds. Created an article feed type, added mappings to title and image. Ran the feed against a test CSV and had an error that triggered the exception on line 188 of File.php. That was triggering the fatal error.

Edit: I can feature the feed if need be

wolffereast It appears that this prevents the fatal error! I had much better luck getting my import with images through successfully. There is still curl errors thrown on non existent files. (as if curl was invoked with no target url I believe). What excellent timing thank you.

fwiw my errors have been happening with PHP 7.1.14 as well. I haven't tried 5.6 on this.

@HongPong check out https://www.drupal.org/project/feeds/issues/2772595 for the empty images issue. Im currently tracking that down, hoping to have a patch soon.

The patch in #11 makes error messages to the user less readable. See image.

@mikran, @HongPong, @wolffereast, @others
Can you provide the full call stack? It just sounds weird that a FormattableMarkup object would throw a TargetValidationException. Like the code is in some sort of loop (though not an infinite one).

I tried to reproduce the error in PHP 7.1.14 today, no luck yet.

Perhaps also useful to know: does the error happen with the Bartik theme? Or do you have a different theme installed? Maybe the theme uses a FormattableMarkup object to render something and it somehow does that before the exception is thrown.

@megachriz yes I do think I saw a loop of some sort going on (like it would seem to complete the batch and start over). It will take a couple days but I should be able to get you something better reproducible.

I have zurb_foundation theme and I do tend to see <em class="placeholder"> tags here and there. So perhaps there is something getting altered in the theme that makes a FormattableMarkup pop up.

Here is the text of the full error. I'm not seeing much in the way of looping in there, it hits File.php and dies. Im currently running through a cache grind to see if I can track the issue any further.

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /batch?id=130&op=do_nojs&op=do
StatusText: OK
ResponseText: 
( ! ) Fatal error: Method Drupal\Component\Render\FormattableMarkup::__toString() must not throw an exception in C:\xampp\htdocs\dev.d8\modules\feeds\src\Feeds\Target\File.php on line 188
Call Stack
#TimeMemoryFunctionLocation
10.0020139136{main}(  )...\index.php:0
20.04701518072Drupal\Core\DrupalKernel->handle(  )...\index.php:19
30.15703406472Stack\StackedHttpKernel->handle(  )...\DrupalKernel.php:664
40.15703406528Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(  )...\StackedHttpKernel.php:23
50.15703406672Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(  )...\NegotiationMiddleware.php:50
60.15703406904Drupal\page_cache\StackMiddleware\PageCache->handle(  )...\ReverseProxyMiddleware.php:47
70.15703407240Drupal\page_cache\StackMiddleware\PageCache->pass(  )...\PageCache.php:78
80.15703407280Drupal\Core\StackMiddleware\KernelPreHandle->handle(  )...\PageCache.php:99
90.29107704320Drupal\Core\StackMiddleware\Session->handle(  )...\KernelPreHandle.php:47
100.34098072496Symfony\Component\HttpKernel\HttpKernel->handle(  )...\Session.php:57
110.34098073056Symfony\Component\HttpKernel\HttpKernel->handleRaw(  )...\HttpKernel.php:68
120.843012747208call_user_func_array:{C:\xampp\htdocs\dev.d8\vendor\symfony\http-kernel\HttpKernel.php:151}
(  )...\HttpKernel.php:151
130.843012747320Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}(  )...\HttpKernel.php:151
140.843012747448Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(  )...\EarlyRenderingControllerWrapperSubscriber.php:97
150.844012759424Drupal\Core\Render\Renderer->executeInRenderContext(  )...\EarlyRenderingControllerWrapperSubscriber.php:124
160.844012759664Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}(  )...\Renderer.php:582
170.844012759696call_user_func_array:{C:\xampp\htdocs\dev.d8\core\lib\Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber.php:123}
(  )...\EarlyRenderingControllerWrapperSubscriber.php:123
180.844012759920Drupal\system\Controller\BatchController->batchPage(  )...\EarlyRenderingControllerWrapperSubscriber.php:123
190.846012821584_batch_page(  )...\BatchController.php:55
201.045016731736_batch_do(  )...\batch.inc:93
211.045016731896_batch_process(  )...\batch.inc:137
221.582020813648call_user_func_array:{C:\xampp\htdocs\dev.d8\core\includes\batch.inc:294}
(  )...\batch.inc:294
231.582020814056Drupal\feeds\FeedImportHandler->batchProcess(  )...\batch.inc:294
241.582020814104Drupal\feeds\FeedImportHandler->doProcess(  )...\FeedImportHandler.php:200
251.585020819256Drupal\feeds\FeedHandlerBase->dispatchEvent(  )...\FeedImportHandler.php:373
261.585020819256Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(  )...\EventDispatcherTrait.php:37
271.585020819256call_user_func:{C:\xampp\htdocs\dev.d8\core\lib\Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher.php:111}
(  )...\ContainerAwareEventDispatcher.php:111
281.585020819344Drupal\feeds\EventSubscriber\LazySubscriber->Drupal\feeds\EventSubscriber\{closure}(  )...\ContainerAwareEventDispatcher.php:111
291.604020831520Drupal\feeds\Feeds\Processor\EntityProcessorBase->process(  )...\LazySubscriber.php:94
301.876022407704Drupal\feeds\Feeds\Processor\EntityProcessorBase->map(  )...\EntityProcessorBase.php:151
311.985023223544Drupal\feeds\Plugin\Type\Target\FieldTargetBase->setTarget(  )...\EntityProcessorBase.php:813
321.985023223584Drupal\feeds\Plugin\Type\Target\FieldTargetBase->prepareValues(  )...\FieldTargetBase.php:78
331.985023224056Drupal\feeds\Feeds\Target\Image->prepareValue(  )...\FieldTargetBase.php:98
341.985023224280Drupal\feeds\Feeds\Target\File->getFile(  )...\Image.php:36
352.109024018112Drupal\feeds\Feeds\Target\File->getFileName(  )...\File.php:156

Missed the theme question. I am using the default themes. To be precise I have Bartik 8.5.3-dev and Seven 8.5.3-dev enabled. I just pulled updates, so they should be up to date. The feed runs in the admin theme, so it would be Seven if it is coming from the theme.

From what I am seeing in core the most common way to set the message of the Exception is to use parent::__construct();. In this case that would look like

  public function __construct($message = '', array $args = []) {
    parent::__construct(new FormattableMarkup($message, $args));

    $this->messageString = $message;
    $this->args = $args;
    $this->code = 0;
  }

Unfortunately the drupal_set_message call in the catch around line 105 of FieldTargetBase is encoding the html in the message string, which is giving the less than optimal output @megachriz posted above. Because the __construct function is overwritten and methods to retrieve the messageString and args implemented we can trade the drupal_set_message($e->getMessage()); for

  catch (TargetValidationException $e) {
    // Validation failed.
    $messenger = \Drupal::messenger();
    $messenger->addError(new TranslatableMarkup($e->getMessageString(), $e->getMessageArgs()));
  }

With those two changes in place I get the correctly formatted error message shown in the attached thumbnail.

I'm uploading a patch with the two fixes in place just in case this is a legitimate way to handle the output.

@wolffereast
Thanks for looking into this. Meanwhile, I've worked on test coverage for rendering an error message caused by a ValidationException in #2966515: Add test coverage for display a field validation error.

Mostly all over core exceptions use sprintf() to format exceptions, except ones in which in progress #2055851: Remove translation of exception messages

1. +++ b/src/Exception/ValidationException.php
   @@ -32,8 +32,9 @@ class ValidationException extends FeedsRuntimeException {
   +    parent::__construct(new FormattableMarkup($message, $args));
   
   +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -103,7 +104,8 @@ abstract class FieldTargetBase extends TargetBase {
   +        $messenger->addError(new TranslatableMarkup($e->getMessageString(), $e->getMessageArgs()));
   

   Core does not use translation for exceptions #2514044: Do not use SafeMarkup::format in exceptions

2. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -103,7 +104,8 @@ abstract class FieldTargetBase extends TargetBase {
   -        drupal_set_message($e->getMessage(), 'error');
   +        $messenger = \Drupal::messenger();
   

   Looks better to create separate protected method to return messeger (or inject it with constructor)

@andypost

Core does not use translation for exceptions

The issue here is that these messages can be shown to end users. The exceptions here aren't caused by a bug or a bad API call, but by an user interaction. The exceptions are thrown when input validation fails. These messages aren't "developer-facing strings". Therefore I think that in this case the messages should be translated.

Maybe the code catching the exception should be responsible for marking the message as safe as in install.core.inc:

catch (InstallerException $e) {
  // In the non-interactive installer, exceptions are always thrown directly.
  if (!$install_state['interactive']) {
    throw $e;
  }
  $output = [
    '#title' => $e->getTitle(),
    '#markup' => $e->getMessage(),
  ];
}

$e->getMessage() is assigned to '#markup'.

Catching a (validation) exception during import happens - among more - in EntityProcessorBase::process(). It also happens in FieldTargetBase. Maybe it should wrap the message in #markup there? Or we can add a method to ValidationException to return a 'formatted' message.

The only downside is that external code that is catching a ValidationException as well, will receive an exception message containing HTML code. And rendering that results into double sanitation.

Got it, then better to catch this exceptions and display user friendly messages to UI

The problem here is that pots and other extraction tools needs real strings in t() to expose them to translation system... otherwise they will be translatable only at runtime and no way to export them to l.d.o

Good news! I've been able to reproduce the error during executing tests from #2940280: Add test coverage for the file target. locally on PHP 7.1.4.

The attached patch makes these test not fail on the error reported in this issue. It also makes error messages thrown in exceptions by the file target translatable.

Although the tests fail on PHP 7.1, PHP 7.2 and D8.6, they did not cause new test failures. The patch even passes on D8.4. So committed #28.

Fixing the test failures for PHP 7.1, etc. is food for another issue.