Avoid breaking import when fetching fail

graby will throw an Exception in some case (like a bad url, a restricted url or a secured pdf).

Import doesn't handle that case and break the whole import.
With that commit the import isn't stopped but the entry is just skipped.

Also, as a  bonus, I've added extra test on WallabagImportV2 when the json is empty.

3 files changed, 63 insertions, 32 deletions

diff --git a/src/Wallabag/ImportBundle/Import/AbstractImport.php b/src/Wallabag/ImportBundle/Import/AbstractImport.php
new file mode 100644
index 00000000..14377a35
--- /dev/null
+++ b/src/Wallabag/ImportBundle/Import/AbstractImport.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace Wallabag\ImportBundle\Import;
+
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use Doctrine\ORM\EntityManager;
+use Wallabag\CoreBundle\Helper\ContentProxy;
+use Wallabag\CoreBundle\Entity\Entry;
+
+abstract class AbstractImport implements ImportInterface
+{
+    protected $em;
+    protected $logger;
+    protected $contentProxy;
+
+    public function __construct(EntityManager $em, ContentProxy $contentProxy)
+    {
+        $this->em = $em;
+        $this->logger = new NullLogger();
+        $this->contentProxy = $contentProxy;
+    }
+
+    public function setLogger(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
+    /**
+     * Fetch content from the ContentProxy (using graby).
+     * If it fails return false instead of the updated entry.
+     *
+     * @param Entry  $entry   Entry to update
+     * @param string $url     Url to grab content for
+     * @param array  $content An array with AT LEAST keys title, html, url, language & content_type to skip the fetchContent from the url
+     *
+     * @return Entry|false
+     */
+    protected function fetchContent(Entry $entry, $url, array $content = [])
+    {
+        try {
+            return $this->contentProxy->updateEntry($entry, $url, $content);
+        } catch (\Exception $e) {
+            return false;
+        }
+    }
+}
diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php
index 29361a32..678d016a 100644
--- a/src/Wallabag/ImportBundle/Import/PocketImport.php
+++ b/src/Wallabag/ImportBundle/Import/PocketImport.php
@@ -2,7 +2,6 @@
 
 namespace Wallabag\ImportBundle\Import;
 
-use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Doctrine\ORM\EntityManager;
 use GuzzleHttp\Client;
@@ -12,12 +11,9 @@ use Wallabag\CoreBundle\Entity\Entry;
 use Wallabag\CoreBundle\Helper\ContentProxy;
 use Craue\ConfigBundle\Util\Config;
 
-class PocketImport implements ImportInterface
+class PocketImport extends AbstractImport
 {
     private $user;
-    private $em;
-    private $contentProxy;
-    private $logger;
     private $client;
     private $consumerKey;
     private $skippedEntries = 0;
@@ -34,11 +30,6 @@ class PocketImport implements ImportInterface
         $this->logger = new NullLogger();
     }
 
-    public function setLogger(LoggerInterface $logger)
-    {
-        $this->logger = $logger;
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -219,7 +210,13 @@ class PocketImport implements ImportInterface
             }
 
             $entry = new Entry($this->user);
-            $entry = $this->contentProxy->updateEntry($entry, $url);
+            $entry = $this->fetchContent($entry, $url);
+
+            // jump to next entry in case of problem while getting content
+            if (false === $entry) {
+                ++$this->skippedEntries;
+                continue;
+            }
 
             // 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
             if ($pocketEntry['status'] == 1 || $this->markAsRead) {
diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php
index 65803823..a1cc085b 100644
--- a/src/Wallabag/ImportBundle/Import/WallabagImport.php
+++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php
@@ -2,19 +2,12 @@
 
 namespace Wallabag\ImportBundle\Import;
 
-use Psr\Log\LoggerInterface;
-use Psr\Log\NullLogger;
-use Doctrine\ORM\EntityManager;
 use Wallabag\CoreBundle\Entity\Entry;
 use Wallabag\UserBundle\Entity\User;
-use Wallabag\CoreBundle\Helper\ContentProxy;
 
-abstract class WallabagImport implements ImportInterface
+abstract class WallabagImport extends AbstractImport
 {
     protected $user;
-    protected $em;
-    protected $logger;
-    protected $contentProxy;
     protected $skippedEntries = 0;
     protected $importedEntries = 0;
     protected $filepath;
@@ -35,18 +28,6 @@ abstract class WallabagImport implements ImportInterface
         '',
     ];
 
-    public function __construct(EntityManager $em, ContentProxy $contentProxy)
-    {
-        $this->em = $em;
-        $this->logger = new NullLogger();
-        $this->contentProxy = $contentProxy;
-    }
-
-    public function setLogger(LoggerInterface $logger)
-    {
-        $this->logger = $logger;
-    }
-
     /**
      * We define the user in a custom call because on the import command there is no logged in user.
      * So we can't retrieve user from the `security.token_storage` service.
@@ -159,12 +140,18 @@ abstract class WallabagImport implements ImportInterface
 
             $data = $this->prepareEntry($importedEntry, $this->markAsRead);
 
-            $entry = $this->contentProxy->updateEntry(
+            $entry = $this->fetchContent(
                 new Entry($this->user),
                 $importedEntry['url'],
                 $data
             );
 
+            // jump to next entry in case of problem while getting content
+            if (false === $entry) {
+                ++$this->skippedEntries;
+                continue;
+            }
+
             if (array_key_exists('tags', $data)) {
                 $this->contentProxy->assignTagsToEntry(
                     $entry,