ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Keep exceptions http response generic and return the request ID for further tracing

Browse Source 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl
2021-10-25 13:45:59 +02:00
parent 100bc30d9b
commit 83fc4323f4
3 changed files with 58 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
lib/Middleware/ExceptionMiddleware.php
View File

@@ -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;
}

6
lib/StatusException.php
View File

@@ -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);

9
tests/unit/Middleware/ExceptionMiddlewareTest.php
View File

@@ -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']);
}
}