Feeds does not catch file exceptions properly in file mapper and FeedsParser.inc getFile() function

This turned out to be a big problem. In the case where the image url is empty or NULL those nodes do not import. Although the error message is clear, one has to create multiple feeds for the same entity. One for the other fields and one for the images.

This patch worked fine for me on 7.x-2.x head on Drupal 7.8. I also looked over the code in the patch and don't see any problems it would create.

Another patch should be written for the special case where the value of the file url is NULL or empty. The module should not request the file at all.

+++ mappers/file.incundefined
@@ -72,7 +72,13 @@ function file_feeds_set_target($source, $entity, $target, $value) {
+    catch (Exception $e) {
+      watchdog('Feeds', nl2br(check_plain($e->getMessage())), NULL, WATCHDOG_WARNING);
+    }           ¶

This should probably be using watchdog_exception()?

+++ plugins/FeedsParser.incundefined
@@ -323,7 +323,12 @@ class FeedsEnclosure extends FeedsElement {
+        try {
+          $file = file_save_data($this->getContent(), "$destination/$filename");
+        } ¶

Same here.

I apologize if this question has an obvious answer, but could you provide a sample file and/or provide steps to reproduce? That would go a long way toward getting this patch reviewed, tested, and committed. So far I'm having trouble reproducing this issue based on the info provided.

Thanks,
Jed

The problem this patch is attempting to solve is if you run an import with an image field defined, and your source contains a path to an image that does not exist, the expected behavior would be to import the node without an image and log the fact that the image was not found. Currently, however, the whole node fails to import. This patch brings us the expected behavior.

I have tested on D7.12 and the latest dev copy of Feeds - working fine for me. Thank you vaartio.

Here is a rerolled patch against the latest dev with the changes recommended in #4.

+++ b/mappers/file.inc	2012-03-07 18:16:24.000000000 +0000
@@ -75,7 +75,13 @@ function file_feeds_set_target($source, 
+    } ¶
+    catch (Exception $e) {
+      watchdog_exception('Feeds', nl2br(check_plain($e)));
+    }           ¶

+++ b/plugins/FeedsParser.inc	2012-03-07 18:16:37.000000000 +0000
@@ -349,7 +349,12 @@ class FeedsEnclosure extends FeedsElemen
+        } ¶

Some trailing whitespace to remove.

Re-rolled and cleaned.

Code should look like this:

          watchdog_exception('Feeds', $e, nl2br(check_plain($e)));

Exception is second argument

Fixed watchdog_exception() calls.

7.x http://drupalcode.org/project/feeds.git/commit/9e89fdf

Does this need a backport?