Merge pull request #7151 from nextcloud/feature/crash-reporter-registry

Add a mechanism to register crash reporters

lib/composer/composer/autoload_classmap.php

@@ -278,6 +278,8 @@ return array(
     'OCP\\Share_Backend' => $baseDir . '/lib/public/Share_Backend.php',
     'OCP\\Share_Backend_Collection' => $baseDir . '/lib/public/Share_Backend_Collection.php',
     'OCP\\Share_Backend_File_Dependent' => $baseDir . '/lib/public/Share_Backend_File_Dependent.php',
+    'OCP\\Support\\CrashReport\\IRegistry' => $baseDir . '/lib/public/Support/CrashReport/IRegistry.php',
+    'OCP\\Support\\CrashReport\\IReporter' => $baseDir . '/lib/public/Support/CrashReport/IReporter.php',
     'OCP\\SystemTag\\ISystemTag' => $baseDir . '/lib/public/SystemTag/ISystemTag.php',
     'OCP\\SystemTag\\ISystemTagManager' => $baseDir . '/lib/public/SystemTag/ISystemTagManager.php',
     'OCP\\SystemTag\\ISystemTagManagerFactory' => $baseDir . '/lib/public/SystemTag/ISystemTagManagerFactory.php',
@@ -899,6 +901,7 @@ return array(
     'OC\\Share\\Share' => $baseDir . '/lib/private/Share/Share.php',
     'OC\\Streamer' => $baseDir . '/lib/private/Streamer.php',
     'OC\\SubAdmin' => $baseDir . '/lib/private/SubAdmin.php',
+    'OC\\Support\\CrashReport\\Registry' => $baseDir . '/lib/private/Support/CrashReport/Registry.php',
     'OC\\SystemConfig' => $baseDir . '/lib/private/SystemConfig.php',
     'OC\\SystemTag\\ManagerFactory' => $baseDir . '/lib/private/SystemTag/ManagerFactory.php',
     'OC\\SystemTag\\SystemTag' => $baseDir . '/lib/private/SystemTag/SystemTag.php',

lib/composer/composer/autoload_static.php

@@ -308,6 +308,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Share_Backend' => __DIR__ . '/../../..' . '/lib/public/Share_Backend.php',
         'OCP\\Share_Backend_Collection' => __DIR__ . '/../../..' . '/lib/public/Share_Backend_Collection.php',
         'OCP\\Share_Backend_File_Dependent' => __DIR__ . '/../../..' . '/lib/public/Share_Backend_File_Dependent.php',
+        'OCP\\Support\\CrashReport\\IRegistry' => __DIR__ . '/../../..' . '/lib/public/Support/CrashReport/IRegistry.php',
+        'OCP\\Support\\CrashReport\\IReporter' => __DIR__ . '/../../..' . '/lib/public/Support/CrashReport/IReporter.php',
         'OCP\\SystemTag\\ISystemTag' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTag.php',
         'OCP\\SystemTag\\ISystemTagManager' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTagManager.php',
         'OCP\\SystemTag\\ISystemTagManagerFactory' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTagManagerFactory.php',
@@ -929,6 +931,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Share\\Share' => __DIR__ . '/../../..' . '/lib/private/Share/Share.php',
         'OC\\Streamer' => __DIR__ . '/../../..' . '/lib/private/Streamer.php',
         'OC\\SubAdmin' => __DIR__ . '/../../..' . '/lib/private/SubAdmin.php',
+        'OC\\Support\\CrashReport\\Registry' => __DIR__ . '/../../..' . '/lib/private/Support/CrashReport/Registry.php',
         'OC\\SystemConfig' => __DIR__ . '/../../..' . '/lib/private/SystemConfig.php',
         'OC\\SystemTag\\ManagerFactory' => __DIR__ . '/../../..' . '/lib/private/SystemTag/ManagerFactory.php',
         'OC\\SystemTag\\SystemTag' => __DIR__ . '/../../..' . '/lib/private/SystemTag/SystemTag.php',

lib/private/Log.php

@@ -36,7 +36,8 @@ namespace OC;
 
 use InterfaSys\LogNormalizer\Normalizer;
 
-use \OCP\ILogger;
+use OCP\ILogger;
+use OCP\Support\CrashReport\IRegistry;
 use OCP\Util;
 
 /**
@@ -63,6 +64,9 @@ class Log implements ILogger {
 	/** @var Normalizer */
 	private $normalizer;
 
+	/** @var IRegistry */
+	private $crashReporters;
+
 	protected $methodsWithSensitiveParameters = [
 		// Session/User
 		'completeLogin',
@@ -110,9 +114,10 @@ class Log implements ILogger {
 	/**
 	 * @param string $logger The logger that should be used
 	 * @param SystemConfig $config the system config object
-	 * @param null $normalizer
+	 * @param Normalizer|null $normalizer
+	 * @param IRegistry|null $registry
 	 */
-	public function __construct($logger = null, SystemConfig $config = null, $normalizer = null) {
+	public function __construct($logger = null, SystemConfig $config = null, $normalizer = null, IRegistry $registry = null) {
 		// FIXME: Add this for backwards compatibility, should be fixed at some point probably
 		if($config === null) {
 			$config = \OC::$server->getSystemConfig();
@@ -133,7 +138,7 @@ class Log implements ILogger {
 		} else {
 			$this->normalizer = $normalizer;
 		}
-
+		$this->crashReporters = $registry;
 	}
 
 	/**
@@ -346,6 +351,9 @@ class Log implements ILogger {
 		$msg = isset($context['message']) ? $context['message'] : 'Exception';
 		$msg .= ': ' . json_encode($data);
 		$this->log($level, $msg, $context);
+		if (!is_null($this->crashReporters)) {
+			$this->crashReporters->delegateReport($exception);
+		}
 	}
 
 	/**

lib/private/Server.php

@@ -516,12 +516,16 @@ class Server extends ServerContainer implements IServerContainer {
 		});
 		$this->registerAlias('AvatarManager', \OCP\IAvatarManager::class);
 
+		$this->registerAlias(\OCP\Support\CrashReport\IRegistry::class, \OC\Support\CrashReport\Registry::class);
+
 		$this->registerService(\OCP\ILogger::class, function (Server $c) {
 			$logType = $c->query('AllConfig')->getSystemValue('log_type', 'file');
 			$logger = Log::getLogClass($logType);
 			call_user_func(array($logger, 'init'));
+			$config = $this->getSystemConfig();
+			$registry = $c->query(\OCP\Support\CrashReport\IRegistry::class);
 
-			return new Log($logger);
+			return new Log($logger, $config, null, $registry);
 		});
 		$this->registerAlias('Logger', \OCP\ILogger::class);
 

lib/private/Support/CrashReport/Registry.php

@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Support\CrashReport;
+
+use Exception;
+use OCP\Support\CrashReport\IRegistry;
+use OCP\Support\CrashReport\IReporter;
+use Throwable;
+
+class Registry implements IRegistry {
+
+	/** @var array<IReporter> */
+	private $reporters = [];
+
+	/**
+	 * Register a reporter instance
+	 *
+	 * @param IReporter $reporter
+	 */
+	public function register(IReporter $reporter) {
+		$this->reporters[] = $reporter;
+	}
+
+	/**
+	 * Delegate crash reporting to all registered reporters
+	 *
+	 * @param Exception|Throwable $exception
+	 */
+	public function delegateReport($exception) {
+		foreach ($this->reporters as $reporter) {
+			$reporter->report($exception);
+		}
+	}
+
+}

lib/public/Support/CrashReport/IRegistry.php

@@ -0,0 +1,48 @@
+<?php
+
+/**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCP\Support\CrashReport;
+
+use Exception;
+use Throwable;
+
+/**
+ * @since 13.0.0
+ */
+interface IRegistry {
+
+	/**
+	 * Register a reporter instance
+	 *
+	 * @since 13.0.0
+	 * @param IReporter $reporter
+	 */
+	public function register(IReporter $reporter);
+
+	/**
+	 * Delegate crash reporting to all registered reporters
+	 *
+	 * @since 13.0.0
+	 * @param Exception|Throwable $exception
+	 */
+	public function delegateReport($exception);
+}

lib/public/Support/CrashReport/IReporter.php

@@ -0,0 +1,40 @@
+<?php
+
+/**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCP\Support\CrashReport;
+
+use Exception;
+use Throwable;
+
+/**
+ * @since 13.0.0
+ */
+interface IReporter {
+
+	/**
+	 * Report an (unhandled) exception
+	 *
+	 * @since 13.0.0
+	 * @param Exception|Throwable $exception
+	 */
+	public function report($exception);
+}

tests/lib/LoggerTest.php

@@ -11,21 +11,26 @@ namespace Test;
 use OC\Log;
 
 class LoggerTest extends TestCase {
-	/**
+
-	 * @var \OCP\ILogger
+	/** @var \OC\SystemConfig|\PHPUnit_Framework_MockObject_MockObject */
-	 */
+	private $config;
+
+	/** @var \OCP\Support\CrashReport\IRegistry|\PHPUnit_Framework_MockObject_MockObject */
+	private $registry;
+
+	/** @var \OCP\ILogger */
 	private $logger;
+
+	/** @var array */
 	static private $logs = array();
 
 	protected function setUp() {
 		parent::setUp();
 
 		self::$logs = array();
-		$this->config = $this->getMockBuilder(
+		$this->config = $this->createMock(\OC\SystemConfig::class);
-			'\OC\SystemConfig')
+		$this->registry = $this->createMock(\OCP\Support\CrashReport\IRegistry::class);
-			->disableOriginalConstructor()
+		$this->logger = new Log('Test\LoggerTest', $this->config, null, $this->registry);
-			->getMock();
-		$this->logger = new Log('Test\LoggerTest', $this->config);
 	}
 
 	public function testInterpolation() {
@@ -83,6 +88,10 @@ class LoggerTest extends TestCase {
 	 */
 	public function testDetectlogin($user, $password) {
 		$e = new \Exception('test');
+		$this->registry->expects($this->once())
+			->method('delegateReport')
+			->with($e);
+
 		$this->logger->logException($e);
 
 		$logLines = $this->getLogs();
@@ -98,9 +107,13 @@ class LoggerTest extends TestCase {
 	 */
 	public function testDetectcheckPassword($user, $password) {
 		$e = new \Exception('test');
+		$this->registry->expects($this->once())
+			->method('delegateReport')
+			->with($e);
+
 		$this->logger->logException($e);
-		$logLines = $this->getLogs();
 
+		$logLines = $this->getLogs();
 		foreach($logLines as $logLine) {
 			$this->assertNotContains($user, $logLine);
 			$this->assertNotContains($password, $logLine);
@@ -113,9 +126,13 @@ class LoggerTest extends TestCase {
 	 */
 	public function testDetectvalidateUserPass($user, $password) {
 		$e = new \Exception('test');
+		$this->registry->expects($this->once())
+			->method('delegateReport')
+			->with($e);
+
 		$this->logger->logException($e);
-		$logLines = $this->getLogs();
 
+		$logLines = $this->getLogs();
 		foreach($logLines as $logLine) {
 			$this->assertNotContains($user, $logLine);
 			$this->assertNotContains($password, $logLine);
@@ -128,9 +145,13 @@ class LoggerTest extends TestCase {
 	 */
 	public function testDetecttryLogin($user, $password) {
 		$e = new \Exception('test');
+		$this->registry->expects($this->once())
+			->method('delegateReport')
+			->with($e);
+
 		$this->logger->logException($e);
-		$logLines = $this->getLogs();
 
+		$logLines = $this->getLogs();
 		foreach($logLines as $logLine) {
 			$this->assertNotContains($user, $logLine);
 			$this->assertNotContains($password, $logLine);
@@ -145,14 +166,16 @@ class LoggerTest extends TestCase {
 		$a = function($user, $password) {
 			throw new \Exception('test');
 		};
+		$this->registry->expects($this->once())
+			->method('delegateReport');
 
 		try {
 			$a($user, $password);
 		} catch (\Exception $e) {
 			$this->logger->logException($e);
 		}
-		$logLines = $this->getLogs();
 
+		$logLines = $this->getLogs();
 		foreach($logLines as $logLine) {
 			$log = explode('\n', $logLine);
 			unset($log[1]); // Remove `testDetectclosure(` because we are not testing this here, but the closure on stack trace 0

tests/lib/Support/CrashReport/RegistryTest.php

@@ -0,0 +1,68 @@
+<?php
+
+/**
+ * @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Support\CrashReport;
+
+use Exception;
+use OC\Support\CrashReport\Registry;
+use OCP\Support\CrashReport\IReporter;
+use Test\TestCase;
+
+class RegistryTest extends TestCase {
+
+	/** @var Registry */
+	private $registry;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$this->registry = new Registry();
+	}
+
+	/**
+	 * Doesn't assert anything, just checks whether anything "explodes"
+	 */
+	public function testDelegateToNone() {
+		$exception = new Exception('test');
+
+		$this->registry->delegateReport($exception);
+	}
+
+	public function testDelegateToAll() {
+		$reporter1 = $this->createMock(IReporter::class);
+		$reporter2 = $this->createMock(IReporter::class);
+		$exception = new Exception('test');
+		$reporter1->expects($this->once())
+			->method('report')
+			->with($exception);
+		$reporter2->expects($this->once())
+			->method('report')
+			->with($exception);
+
+		$this->registry->register($reporter1);
+		$this->registry->register($reporter2);
+		$this->registry->delegateReport($exception);
+	}
+
+}