Merge pull request #21457 from nextcloud/enhancement/crash-reporters-registration-boostrap

Allow crash reporters registration during app bootstrap

lib/private/AppFramework/Bootstrap/Coordinator.php

@@ -25,6 +25,7 @@ declare(strict_types=1);
 
 namespace OC\AppFramework\Bootstrap;
 
+use OC\Support\CrashReport\Registry;
 use OC_App;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootstrap;
@@ -42,6 +43,9 @@ class Coordinator {
 	/** @var IServerContainer */
 	private $serverContainer;
 
+	/** @var Registry */
+	private $registry;
+
 	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
@@ -49,9 +53,11 @@ class Coordinator {
 	private $logger;
 
 	public function __construct(IServerContainer $container,
+								Registry $registry,
 								IEventDispatcher $eventListener,
 								ILogger $logger) {
 		$this->serverContainer = $container;
+		$this->registry = $registry;
 		$this->eventDispatcher = $eventListener;
 		$this->logger = $logger;
 	}
@@ -102,6 +108,7 @@ class Coordinator {
 		 * to the actual services
 		 */
 		$context->delegateCapabilityRegistrations($apps);
+		$context->delegateCrashReporterRegistrations($apps, $this->registry);
 		$context->delegateEventListenerRegistrations($this->eventDispatcher);
 		$context->delegateContainerRegistrations($apps);
 		$context->delegateMiddlewareRegistrations($apps);

lib/private/AppFramework/Bootstrap/RegistrationContext.php

@@ -26,6 +26,7 @@ declare(strict_types=1);
 namespace OC\AppFramework\Bootstrap;
 
 use Closure;
+use OC\Support\CrashReport\Registry;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -37,6 +38,9 @@ class RegistrationContext {
 	/** @var array[] */
 	private $capabilities = [];
 
+	/** @var array[] */
+	private $crashReporters = [];
+
 	/** @var array[] */
 	private $services = [];
 
@@ -79,6 +83,13 @@ class RegistrationContext {
 				);
 			}
 
+			public function registerCrashReporter(string $reporterClass): void {
+				$this->context->registerCrashReporter(
+					$this->appId,
+					$reporterClass
+				);
+			}
+
 			public function registerService(string $name, callable $factory, bool $shared = true): void {
 				$this->context->registerService(
 					$this->appId,
@@ -129,6 +140,13 @@ class RegistrationContext {
 		];
 	}
 
+	public function registerCrashReporter(string $appId, string $reporterClass): void {
+		$this->crashReporters[] = [
+			'appId' => $appId,
+			'class' => $reporterClass,
+		];
+	}
+
 	public function registerService(string $appId, string $name, callable $factory, bool $shared = true): void {
 		$this->services[] = [
 			"appId" => $appId,
@@ -189,6 +207,23 @@ class RegistrationContext {
 		}
 	}
 
+	/**
+	 * @param App[] $apps
+	 */
+	public function delegateCrashReporterRegistrations(array $apps, Registry $registry): void {
+		foreach ($this->crashReporters as $registration) {
+			try {
+				$registry->registerLazy($registration['class']);
+			} catch (Throwable $e) {
+				$appId = $registration['appId'];
+				$this->logger->logException($e, [
+					'message' => "Error during crash reporter registration of $appId: " . $e->getMessage(),
+					'level' => ILogger::ERROR,
+				]);
+			}
+		}
+	}
+
 	public function delegateEventListenerRegistrations(IEventDispatcher $eventDispatcher): void {
 		foreach ($this->eventListeners as $registration) {
 			try {

lib/private/Support/CrashReport/Registry.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  *
  *
@@ -25,6 +28,9 @@
 namespace OC\Support\CrashReport;
 
 use Exception;
+use OCP\AppFramework\QueryException;
+use OCP\ILogger;
+use OCP\IServerContainer;
 use OCP\Support\CrashReport\ICollectBreadcrumbs;
 use OCP\Support\CrashReport\IMessageReporter;
 use OCP\Support\CrashReport\IRegistry;
@@ -33,9 +39,22 @@ use Throwable;
 
 class Registry implements IRegistry {
 
+	/** @var string[] */
+	private $lazyReporters = [];
+
 	/** @var IReporter[] */
 	private $reporters = [];
 
+	/** @var IServerContainer */
+	private $serverContainer;
+
+	/** @var ILogger */
+	private $logger;
+
+	public function __construct(IServerContainer $serverContainer) {
+		$this->serverContainer = $serverContainer;
+	}
+
 	/**
 	 * Register a reporter instance
 	 *
@@ -45,6 +64,10 @@ class Registry implements IRegistry {
 		$this->reporters[] = $reporter;
 	}
 
+	public function registerLazy(string $class): void {
+		$this->lazyReporters[] = $class;
+	}
+
 	/**
 	 * Delegate breadcrumb collection to all registered reporters
 	 *
@@ -55,6 +78,8 @@ class Registry implements IRegistry {
 	 * @since 15.0.0
 	 */
 	public function delegateBreadcrumb(string $message, string $category, array $context = []): void {
+		$this->loadLazyProviders();
+
 		foreach ($this->reporters as $reporter) {
 			if ($reporter instanceof ICollectBreadcrumbs) {
 				$reporter->collect($message, $category, $context);
@@ -69,7 +94,8 @@ class Registry implements IRegistry {
 	 * @param array $context
 	 */
 	public function delegateReport($exception, array $context = []): void {
-		/** @var IReporter $reporter */
+		$this->loadLazyProviders();
+
 		foreach ($this->reporters as $reporter) {
 			$reporter->report($exception, $context);
 		}
@@ -84,10 +110,48 @@ class Registry implements IRegistry {
 	 * @return void
 	 */
 	public function delegateMessage(string $message, array $context = []): void {
+		$this->loadLazyProviders();
+
 		foreach ($this->reporters as $reporter) {
 			if ($reporter instanceof IMessageReporter) {
 				$reporter->reportMessage($message, $context);
 			}
 		}
 	}
+
+	private function loadLazyProviders(): void {
+		$classes = $this->lazyReporters;
+		foreach ($classes as $class) {
+			try {
+				/** @var IReporter $reporter */
+				$reporter = $this->serverContainer->query($class);
+			} catch (QueryException $e) {
+				/*
+				 * There is a circular dependency between the logger and the registry, so
+				 * we can not inject it. Thus the static call.
+				 */
+				\OC::$server->getLogger()->logException($e, [
+					'message' => 'Could not load lazy crash reporter: ' . $e->getMessage(),
+					'level' => ILogger::FATAL,
+				]);
+			}
+			/**
+			 * Try to register the loaded reporter. Theoretically it could be of a wrong
+			 * type, so we might get a TypeError here that we should catch.
+			 */
+			try {
+				$this->register($reporter);
+			} catch (Throwable $e) {
+				/*
+				 * There is a circular dependency between the logger and the registry, so
+				 * we can not inject it. Thus the static call.
+				 */
+				\OC::$server->getLogger()->logException($e, [
+					'message' => 'Could not register lazy crash reporter: ' . $e->getMessage(),
+					'level' => ILogger::FATAL,
+				]);
+			}
+		}
+		$this->lazyReporters = [];
+	}
 }

lib/public/AppFramework/Bootstrap/IRegistrationContext.php

@@ -45,6 +45,16 @@ interface IRegistrationContext {
 	 */
 	public function registerCapability(string $capability): void;
 
+	/**
+	 * Register an implementation of \OCP\Support\CrashReport\IReporter that
+	 * will receive unhandled exceptions and throwables
+	 *
+	 * @param string $reporterClass
+	 * @return void
+	 * @since 20.0.0
+	 */
+	public function registerCrashReporter(string $reporterClass): void;
+
 	/**
 	 * Register a service
 	 *

lib/public/Support/CrashReport/IRegistry.php

@@ -27,10 +27,12 @@ declare(strict_types=1);
 namespace OCP\Support\CrashReport;
 
 use Exception;
+use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use Throwable;
 
 /**
  * @since 13.0.0
+ * @deprecated used internally only
  */
 interface IRegistry {
 
@@ -40,6 +42,8 @@ interface IRegistry {
 	 * @param IReporter $reporter
 	 *
 	 * @since 13.0.0
+	 * @deprecated 20.0.0 use IRegistrationContext::registerCrashReporter
+	 * @see IRegistrationContext::registerCrashReporter()
 	 */
 	public function register(IReporter $reporter): void;
 
@@ -50,6 +54,7 @@ interface IRegistry {
 	 * @param string $category
 	 * @param array $context
 	 *
+	 * @deprecated used internally only
 	 * @since 15.0.0
 	 */
 	public function delegateBreadcrumb(string $message, string $category, array $context = []): void;
@@ -60,6 +65,7 @@ interface IRegistry {
 	 * @param Exception|Throwable $exception
 	 * @param array $context
 	 *
+	 * @deprecated used internally only
 	 * @since 13.0.0
 	 */
 	public function delegateReport($exception, array $context = []);
@@ -72,6 +78,7 @@ interface IRegistry {
 	 *
 	 * @return void
 	 *
+	 * @deprecated used internally only
 	 * @since 17.0.0
 	 */
 	public function delegateMessage(string $message, array $context = []): void;

lib/public/Support/CrashReport/IReporter.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  *
  *

tests/lib/AppFramework/Bootstrap/CoordinatorTest.php

@@ -26,6 +26,7 @@ declare(strict_types=1);
 namespace lib\AppFramework\Bootstrap;
 
 use OC\AppFramework\Bootstrap\Coordinator;
+use OC\Support\CrashReport\Registry;
 use OCP\App\IAppManager;
 use OCP\AppFramework\App;
 use OCP\AppFramework\Bootstrap\IBootContext;
@@ -46,6 +47,9 @@ class CoordinatorTest extends TestCase {
 	/** @var IServerContainer|MockObject */
 	private $serverContainer;
 
+	/** @var Registry|MockObject */
+	private $crashReporterRegistry;
+
 	/** @var IEventDispatcher|MockObject */
 	private $eventDispatcher;
 
@@ -60,11 +64,13 @@ class CoordinatorTest extends TestCase {
 
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->serverContainer = $this->createMock(IServerContainer::class);
+		$this->crashReporterRegistry = $this->createMock(Registry::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->logger = $this->createMock(ILogger::class);
 
 		$this->coordinator = new Coordinator(
 			$this->serverContainer,
+			$this->crashReporterRegistry,
 			$this->eventDispatcher,
 			$this->logger
 		);

tests/lib/Support/CrashReport/RegistryTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -26,6 +28,8 @@ namespace Test\Support\CrashReport;
 
 use Exception;
 use OC\Support\CrashReport\Registry;
+use OCP\AppFramework\QueryException;
+use OCP\IServerContainer;
 use OCP\Support\CrashReport\ICollectBreadcrumbs;
 use OCP\Support\CrashReport\IMessageReporter;
 use OCP\Support\CrashReport\IReporter;
@@ -33,26 +37,60 @@ use Test\TestCase;
 
 class RegistryTest extends TestCase {
 
+	/** @var IServerContainer|\PHPUnit\Framework\MockObject\MockObject */
+	private $serverContainer;
+
 	/** @var Registry */
 	private $registry;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->registry = new Registry();
+		$this->serverContainer = $this->createMock(IServerContainer::class);
+
+		$this->registry = new Registry(
+			$this->serverContainer
+		);
 	}
 
 	/**
 	 * Doesn't assert anything, just checks whether anything "explodes"
 	 */
-	public function testDelegateToNone() {
+	public function testDelegateToNone(): void {
 		$exception = new Exception('test');
 
 		$this->registry->delegateReport($exception);
 		$this->addToAssertionCount(1);
 	}
 
-	public function testDelegateBreadcrumbCollection() {
+	public function testRegisterLazyCantLoad(): void {
+		$reporterClass = '\OCA\MyApp\Reporter';
+		$reporter = $this->createMock(IReporter::class);
+		$this->serverContainer->expects($this->once())
+			->method('query')
+			->with($reporterClass)
+			->willReturn($reporter);
+		$reporter->expects($this->once())
+			->method('report');
+		$exception = new Exception('test');
+
+		$this->registry->registerLazy($reporterClass);
+		$this->registry->delegateReport($exception);
+	}
+
+	public function testRegisterLazy(): void {
+		$reporterClass = '\OCA\MyApp\Reporter';
+		$this->serverContainer->expects($this->once())
+			->method('query')
+			->with($reporterClass)
+			->willThrowException(new QueryException());
+		$exception = new Exception('test');
+
+		$this->registry->registerLazy($reporterClass);
+		$this->registry->delegateReport($exception);
+	}
+
+	public function testDelegateBreadcrumbCollection(): void {
 		$reporter1 = $this->createMock(IReporter::class);
 		$reporter2 = $this->createMock(ICollectBreadcrumbs::class);
 		$message = 'hello';
@@ -66,7 +104,7 @@ class RegistryTest extends TestCase {
 		$this->registry->delegateBreadcrumb($message, $category);
 	}
 
-	public function testDelegateToAll() {
+	public function testDelegateToAll(): void {
 		$reporter1 = $this->createMock(IReporter::class);
 		$reporter2 = $this->createMock(IReporter::class);
 		$exception = new Exception('test');
@@ -82,7 +120,7 @@ class RegistryTest extends TestCase {
 		$this->registry->delegateReport($exception);
 	}
 
-	public function testDelegateMessage() {
+	public function testDelegateMessage(): void {
 		$reporter1 = $this->createMock(IReporter::class);
 		$reporter2 = $this->createMock(IMessageReporter::class);
 		$message = 'hello';