Merge pull request #8726 from piwik/scheduled_reports_catch

Catch exceptions in ScheduledReports generation so reports will still generate if individual reports fail

core/API/Proxy.php

@@ -37,10 +37,7 @@ class Proxy extends Singleton
     // when a parameter doesn't have a default value we use this
     private $noDefaultValue;
 
-    /**
-     * protected constructor
-     */
-    protected function __construct()
+    public function __construct()
     {
         $this->noDefaultValue = new NoDefaultValue();
     }

core/ReportRenderer.php

@@ -177,17 +177,17 @@ abstract class ReportRenderer extends BaseFactory
         $filename = ReportRenderer::makeFilenameWithExtension($filename, $extension);
 
         ProxyHttp::overrideCacheControlHeaders();
-        header('Content-Description: File Transfer');
-        header('Content-Type: ' . $contentType);
-        header('Content-Disposition: attachment; filename="' . str_replace('"', '\'', basename($filename)) . '";');
-        header('Content-Length: ' . strlen($content));
+        Common::sendHeader('Content-Description: File Transfer');
+        Common::sendHeader('Content-Type: ' . $contentType);
+        Common::sendHeader('Content-Disposition: attachment; filename="' . str_replace('"', '\'', basename($filename)) . '";');
+        Common::sendHeader('Content-Length: ' . strlen($content));
 
         echo $content;
     }
 
     protected static function inlineToBrowser($contentType, $content)
     {
-        header('Content-Type: ' . $contentType);
+        Common::sendHeader('Content-Type: ' . $contentType);
         echo $content;
     }
 

core/Singleton.php

@@ -63,4 +63,12 @@ class Singleton
         $class = get_called_class();
         self::$instances[$class] = $instance;
     }
+
+    /**
+     * @ignore
+     */
+    public static function clearAll()
+    {
+        self::$instances = array();
+    }
 }

plugins/API/RowEvolution.php

@@ -360,9 +360,16 @@ class RowEvolution
         unset($metadata['logos']);
 
         $subDataTables = $dataTable->getDataTables();
+        if (empty($subDataTables)) {
+            throw new \Exception("Unexpected state: row evolution API call returned empty DataTable\\Map.");
+        }
+
         $firstDataTable = reset($subDataTables);
+        $this->checkDataTableInstance($firstDataTable);
         $firstDataTableRow = $firstDataTable->getFirstRow();
+
         $lastDataTable = end($subDataTables);
+        $this->checkDataTableInstance($lastDataTable);
         $lastDataTableRow = $lastDataTable->getFirstRow();
 
         // Process min/max values
@@ -533,4 +540,11 @@ class RowEvolution
         $label = SafeDecodeLabel::decodeLabelSafe($label);
         return $label;
     }
+
+    private function checkDataTableInstance($lastDataTable)
+    {
+        if (!($lastDataTable instanceof DataTable)) {
+            throw new \Exception("Unexpected state: row evolution returned DataTable\\Map w/ incorrect child table type: " . get_class($lastDataTable));
+        }
+    }
 }

plugins/ScheduledReports/API.php

@@ -26,6 +26,9 @@ use Piwik\Site;
 use Piwik\Tracker;
 use Piwik\Translate;
 use Piwik\Translation\Translator;
+use Piwik\Url;
+use Piwik\UrlHelper;
+use Psr\Log\LoggerInterface;
 
 /**
  * The ScheduledReports API lets you manage Scheduled Email reports, as well as generate, download or email any existing report.
@@ -60,6 +63,16 @@ class API extends \Piwik\Plugin\API
     // static cache storing reports
     public static $cache = array();
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
     /**
      * Creates a new report and schedules it.
      *
@@ -377,7 +390,20 @@ class API extends \Piwik\Plugin\API
                 $params['segment'] = false;
             }
 
-            $processedReport = Request::processRequest('API.getProcessedReport', $params);
+            try {
+                $processedReport = Request::processRequest('API.getProcessedReport', $params);
+            } catch (\Exception $ex) {
+                // NOTE: can't use warning or error because the log message will appear in the UI as a notification
+                $this->logger->info("Error getting '?{report}' when generating scheduled report: {exception}", array(
+                    'report' => http_build_query($params),
+                    'exception' => $ex->getMessage(),
+                ));
+
+                $this->logger->debug($ex);
+
+                continue;
+            }
+
             $processedReport['segment'] = $segment;
 
             // TODO add static method getPrettyDate($period, $date) in Period

plugins/ScheduledReports/tests/Integration/ApiTest.php

@@ -8,12 +8,18 @@
 
 namespace Piwik\Plugins\ScheduledReports\tests;
 
+use Piwik\API\Proxy;
+use Piwik\DataTable;
+use Piwik\Date;
+use Piwik\Plugins\API\API;
 use Piwik\Plugins\MobileMessaging\API as APIMobileMessaging;
 use Piwik\Plugins\MobileMessaging\MobileMessaging;
 use Piwik\Plugins\ScheduledReports\API as APIScheduledReports;
 use Piwik\Plugins\ScheduledReports\Menu;
+use Piwik\Plugins\ScheduledReports\ScheduledReports;
 use Piwik\Plugins\ScheduledReports\Tasks;
 use Piwik\Plugins\SitesManager\API as APISitesManager;
+use Piwik\ReportRenderer;
 use Piwik\Scheduler\Schedule\Monthly;
 use Piwik\Scheduler\Schedule\Schedule;
 use Piwik\Scheduler\Task;
@@ -41,7 +47,8 @@ class ApiTest extends IntegrationTestCase
 
         // setup the access layer
         self::setSuperUser();
-        \Piwik\Plugin\Manager::getInstance()->loadPlugins(array('API', 'UserCountry', 'ScheduledReports', 'MobileMessaging'));
+        \Piwik\Plugin\Manager::getInstance()->loadPlugins(array('API', 'UserCountry', 'ScheduledReports',
+            'MobileMessaging', 'VisitsSummary', 'Referrers'));
         \Piwik\Plugin\Manager::getInstance()->installLoadedPlugins();
 
         APISitesManager::getInstance()->addSite("Test", array("http://piwik.net"));
@@ -426,6 +433,56 @@ class ApiTest extends IntegrationTestCase
         $this->assertEquals($expectedReportTitle, $reportTitle);
     }
 
+    public function test_generateReport_CatchesIndividualReportProcessExceptions_WithoutFailingToGenerateWholeReport()
+    {
+        $realProxy = new Proxy();
+
+        $mockProxy = $this->getMock('Piwik\API\Proxy', array('call'));
+        $mockProxy->expects($this->any())->method('call')->willReturnCallback(function ($className, $methodName, $parametersRequest) use ($realProxy) {
+            switch ($className) {
+                case '\Piwik\Plugins\VisitsSummary\API':
+                    $result = new DataTable();
+                    $result->addRowFromSimpleArray(array('label' => 'visits label', 'nb_visits' => 1));
+                    return $result;
+                case '\Piwik\Plugins\UserCountry\API':
+                    throw new \Exception("error");
+                case '\Piwik\Plugins\Referrers\API':
+                    $result = new DataTable();
+                    $result->addRowFromSimpleArray(array('label' => 'referrers label', 'nb_visits' => 1));
+                    return $result;
+                case '\Piwik\Plugins\API\API':
+                    return $realProxy->call($className, $methodName, $parametersRequest);
+                default:
+                    throw new \Exception("Unexpected method $className::$methodName.");
+            }
+        });
+        Proxy::setSingletonInstance($mockProxy);
+
+        $idReport = APIScheduledReports::getInstance()->addReport(
+            1,
+            '',
+            Schedule::PERIOD_DAY,
+            0,
+            ScheduledReports::EMAIL_TYPE,
+            ReportRenderer::HTML_FORMAT,
+            array(
+                'VisitsSummary_get',
+                'UserCountry_getCountry',
+                'Referrers_getWebsites',
+            ),
+            array(ScheduledReports::DISPLAY_FORMAT_PARAMETER => ScheduledReports::DISPLAY_FORMAT_TABLES_ONLY)
+        );
+
+        ob_start();
+        $result = APIScheduledReports::getInstance()->generateReport($idReport, Date::factory('now')->toString(),
+            $language = false, $outputType = APIScheduledReports::OUTPUT_RETURN);
+        ob_end_clean();
+
+        $this->assertContains('id="VisitsSummary_get"', $result);
+        $this->assertContains('id="Referrers_getWebsites"', $result);
+        $this->assertNotContains('id="UserCountry_getCountry"', $result);
+    }
+
     private function assertReportsEqual($report, $data)
     {
         foreach ($data as $key => $value) {

tests/PHPUnit/Framework/Fixture.php

@@ -40,6 +40,7 @@ use Piwik\Plugins\UsersManager\UsersManager;
 use Piwik\ReportRenderer;
 use Piwik\SettingsPiwik;
 use Piwik\SettingsServer;
+use Piwik\Singleton;
 use Piwik\Site;
 use Piwik\Tests\Framework\Mock\FakeAccess;
 use Piwik\Tests\Framework\TestCase\SystemTestCase;
@@ -350,6 +351,7 @@ class Fixture extends \PHPUnit_Framework_Assert
         PiwikCache::getEagerCache()->flushAll();
         ArchiveTableCreator::clear();
         \Piwik\Plugins\ScheduledReports\API::$cache = array();
+        Singleton::clearAll();
 
         $_GET = $_REQUEST = array();
         Translate::reset();