From: Christoph Wurst Date: Wed, 17 Jun 2020 13:17:59 +0000 (+0200) Subject: Allow crash reporters registration during app bootstrap X-Git-Tag: v20.0.0beta1~399^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F21457%2Fhead;p=nextcloud-server.git Allow crash reporters registration during app bootstrap Signed-off-by: Christoph Wurst --- diff --git a/lib/private/AppFramework/Bootstrap/Coordinator.php b/lib/private/AppFramework/Bootstrap/Coordinator.php index dea7370e68d..9b0f6a9188c 100644 --- a/lib/private/AppFramework/Bootstrap/Coordinator.php +++ b/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); diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index c7b2290e6b8..de2f80a2cc2 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/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 { diff --git a/lib/private/Support/CrashReport/Registry.php b/lib/private/Support/CrashReport/Registry.php index 44a6d290678..bff62705ceb 100644 --- a/lib/private/Support/CrashReport/Registry.php +++ b/lib/private/Support/CrashReport/Registry.php @@ -1,4 +1,7 @@ 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 = []; + } } diff --git a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php index 589b5def5a8..5d3ffc8d479 100644 --- a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php +++ b/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 * diff --git a/lib/public/Support/CrashReport/IRegistry.php b/lib/public/Support/CrashReport/IRegistry.php index 5ceabcca641..77456663848 100644 --- a/lib/public/Support/CrashReport/IRegistry.php +++ b/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; diff --git a/lib/public/Support/CrashReport/IReporter.php b/lib/public/Support/CrashReport/IReporter.php index b7f84417937..5aaa9d8a6f5 100644 --- a/lib/public/Support/CrashReport/IReporter.php +++ b/lib/public/Support/CrashReport/IReporter.php @@ -1,4 +1,7 @@ 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 ); diff --git a/tests/lib/Support/CrashReport/RegistryTest.php b/tests/lib/Support/CrashReport/RegistryTest.php index d85b006509e..f88902d7065 100644 --- a/tests/lib/Support/CrashReport/RegistryTest.php +++ b/tests/lib/Support/CrashReport/RegistryTest.php @@ -1,5 +1,7 @@ * @@ -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';