diff --git a/lib/Middleware/ExceptionMiddleware.php b/lib/Middleware/ExceptionMiddleware.php index aa0fdb5f1..0c59a5e17 100644 --- a/lib/Middleware/ExceptionMiddleware.php +++ b/lib/Middleware/ExceptionMiddleware.php @@ -32,6 +32,7 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCSController; use OCP\ILogger; +use OCP\IRequest; use OCP\Util; use OCP\IConfig; @@ -41,6 +42,8 @@ class ExceptionMiddleware extends Middleware { private $logger; /** @var IConfig */ private $config; + /** @var IRequest */ + private $request; /** * SharingMiddleware constructor. @@ -48,9 +51,10 @@ class ExceptionMiddleware extends Middleware { * @param ILogger $logger * @param IConfig $config */ - public function __construct(ILogger $logger, IConfig $config) { + public function __construct(ILogger $logger, IConfig $config, IRequest $request) { $this->logger = $logger; $this->config = $config; + $this->request = $request; } /** @@ -67,43 +71,10 @@ class ExceptionMiddleware extends Middleware { throw $exception; } - if ($exception instanceof ConflictException) { - if ($this->config->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) { - $this->logger->logException($exception); - } - return new JSONResponse([ - 'status' => $exception->getStatus(), - 'message' => $exception->getMessage(), - 'data' => $exception->getData(), - ], $exception->getStatus()); - } - - if ($exception instanceof StatusException) { - if ($this->config->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) { - $this->logger->logException($exception); - } - - if ($controller instanceof OCSController) { - $exception = new OCSException($exception->getMessage(), $exception->getStatus(), $exception); - throw $exception; - } - return new JSONResponse([ - 'status' => $exception->getStatus(), - 'message' => $exception->getMessage() - ], $exception->getStatus()); - } - - if (strpos(get_class($controller), 'OCA\\Deck\\Controller\\') === 0) { - $response = [ - 'status' => 500, - 'message' => $exception->getMessage() - ]; - $this->logger->logException($exception); - if ($this->config->getSystemValue('debug', true) === true) { - $response['exception'] = (array) $exception; - } - return new JSONResponse($response, 500); - } + $debugMode = $this->config->getSystemValue('debug', false); + $exceptionMessage = $debugMode !== true + ? 'Internal server error: Please contact the server administrator if this error reappears multiple times, please include the request ID "' . $this->request->getId() . '" below in your report.' + : $exception->getMessage(); // uncatched DoesNotExistExceptions will be thrown when the main entity is not found // we return a 403 so we don't leak information over existing entries @@ -114,6 +85,43 @@ class ExceptionMiddleware extends Middleware { 'message' => 'Permission denied' ], 403); } + + if ($exception instanceof StatusException) { + if ($this->config->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) { + $this->logger->logException($exception); + } + + if ($exception instanceof ConflictException) { + return new JSONResponse([ + 'status' => $exception->getStatus(), + 'message' => $exception->getMessage(), + 'data' => $exception->getData(), + ], $exception->getStatus()); + } + + if ($controller instanceof OCSController) { + $exception = new OCSException($exception->getMessage(), $exception->getStatus(), $exception); + throw $exception; + } + + return new JSONResponse([ + 'status' => $exception->getStatus(), + 'message' => $exception->getMessage(), + ], $exception->getStatus()); + } + + if (strpos(get_class($controller), 'OCA\\Deck\\Controller\\') === 0) { + $response = [ + 'status' => 500, + 'message' => $exceptionMessage, + 'requestId' => $this->request->getId(), + ]; + $this->logger->logException($exception); + if ($debugMode === true) { + $response['exception'] = (array) $exception; + } + return new JSONResponse($response, 500); + } throw $exception; } diff --git a/lib/StatusException.php b/lib/StatusException.php index 6e88cf009..0c0ce00ef 100644 --- a/lib/StatusException.php +++ b/lib/StatusException.php @@ -23,6 +23,12 @@ namespace OCA\Deck; +/** + * User facing exception that can be thrown with an error being reported to the frontend + * or consumers of the API + * + * This exception is catched in the ExceptionMiddleware + */ class StatusException extends \Exception { public function __construct($message) { parent::__construct($message); diff --git a/tests/unit/Middleware/ExceptionMiddlewareTest.php b/tests/unit/Middleware/ExceptionMiddlewareTest.php index d7df3ac1d..dc00ac40a 100644 --- a/tests/unit/Middleware/ExceptionMiddlewareTest.php +++ b/tests/unit/Middleware/ExceptionMiddlewareTest.php @@ -47,10 +47,12 @@ class ExceptionMiddlewareTest extends \Test\TestCase { public function setUp(): void { $this->logger = $this->createMock(ILogger::class); $this->config = $this->createMock(IConfig::class); + $this->request = $this->createMock(IRequest::class); $this->controller = $this->createMock(Controller::class); $this->exceptionMiddleware = new ExceptionMiddleware( $this->logger, - $this->config + $this->config, + $this->request ); } @@ -81,10 +83,11 @@ class ExceptionMiddlewareTest extends \Test\TestCase { } public function testAfterExceptionFail() { + $this->request->expects($this->any())->method('getId')->willReturn('abc123'); // BoardService $boardService, PermissionService $permissionService, $userId $boardController = new BoardController('deck', $this->createMock(IRequest::class), $this->createMock(BoardService::class), $this->createMock(PermissionService::class), 'admin'); - $result = $this->exceptionMiddleware->afterException($boardController, 'bar', new \Exception('failed hard')); - $this->assertEquals('failed hard', $result->getData()['message']); + $result = $this->exceptionMiddleware->afterException($boardController, 'bar', new \Exception('other exception message')); + $this->assertEquals('Internal server error: Please contact the server administrator if this error reappears multiple times, please include the request ID "abc123" below in your report.', $result->getData()['message']); $this->assertEquals(500, $result->getData()['status']); } }