From 2a38605545e26ce68a37e5ebb877fd9c9875a37d Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 17 Jan 2018 15:21:56 +0100 Subject: [PATCH] Properly log the full exception instead of only the message Signed-off-by: Morris Jobke --- apps/encryption/lib/KeyManager.php | 10 +- .../Controller/MountPublicLinkController.php | 44 ++++---- .../Controller/RequestHandlerController.php | 6 +- .../lib/FederatedShareProvider.php | 6 +- .../lib/BackgroundJob/GetSharedSecret.php | 12 +- .../lib/Middleware/AddServerMiddleware.php | 5 +- apps/federation/lib/SyncJob.php | 6 +- apps/federation/lib/TrustedServers.php | 6 +- .../Middleware/AddServerMiddlewareTest.php | 10 +- apps/files_external/lib/Lib/Storage/Swift.php | 45 ++++++-- .../lib/Service/LegacyStoragesService.php | 10 +- .../lib/Service/StoragesService.php | 29 +++-- .../lib/Controller/ShareAPIController.php | 8 +- apps/files_trashbin/lib/Storage.php | 7 +- .../lib/Controller/UsersController.php | 12 +- .../tests/Controller/UsersControllerTest.php | 11 +- apps/sharebymail/lib/ShareByMailProvider.php | 12 +- apps/user_ldap/lib/User/User.php | 8 +- core/ajax/update.php | 4 + lib/base.php | 6 +- .../Security/SecurityMiddleware.php | 5 +- lib/private/CapabilitiesManager.php | 6 +- lib/private/Files/Cache/Scanner.php | 6 +- .../Files/ObjectStore/S3ConnectionTrait.php | 6 +- lib/private/Files/Storage/DAV.php | 3 +- .../Files/Storage/Wrapper/Encryption.php | 7 +- lib/private/Files/View.php | 11 +- lib/private/Installer.php | 5 +- lib/private/Preview/Bitmap.php | 6 +- lib/private/Preview/Office.php | 5 +- lib/private/Preview/SVG.php | 5 +- lib/private/Setup/MySQL.php | 24 ++-- lib/private/Tags.php | 105 +++++++++++++----- settings/Controller/UsersController.php | 6 +- settings/ajax/enableapp.php | 5 +- .../Security/SecurityMiddlewareTest.php | 6 +- tests/lib/CapabilitiesManagerTest.php | 2 +- 37 files changed, 321 insertions(+), 149 deletions(-) diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php index d25a25cdcb..e52da123fe 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -372,11 +372,11 @@ class KeyManager { } catch (DecryptionFailedException $e) { return false; } catch (\Exception $e) { - $this->log->warning( - 'Could not decrypt the private key from user "' . $uid . '"" during login. ' . - 'Assume password change on the user back-end. Error message: ' - . $e->getMessage() - ); + $this->log->logException($e, [ + 'message' => 'Could not decrypt the private key from user "' . $uid . '"" during login. Assume password change on the user back-end.', + 'level' => \OCP\Util::WARN, + 'app' => 'encryption', + ]); return false; } diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index 19f98b264e..a54a3aa69a 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -164,6 +164,10 @@ class MountPublicLinkController extends Controller { try { $this->federatedShareProvider->create($share); } catch (\Exception $e) { + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::WARN, + 'app' => 'federatedfilesharing', + ]); return new JSONResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST); } @@ -286,18 +290,18 @@ class MountPublicLinkController extends Controller { $storage->checkStorageAvailability(); } catch (StorageInvalidException $e) { // note: checkStorageAvailability will already remove the invalid share - Util::writeLog( - 'federatedfilesharing', - 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(), - Util::DEBUG - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Invalid remote storage.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'federatedfilesharing' + ]); return new JSONResponse(['message' => $this->l->t('Could not authenticate to remote share, password might be wrong')], Http::STATUS_BAD_REQUEST); } catch (\Exception $e) { - Util::writeLog( - 'federatedfilesharing', - 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(), - Util::DEBUG - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Invalid remote storage.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'federatedfilesharing' + ]); $externalManager->removeShare($mount->getMountPoint()); return new JSONResponse(['message' => $this->l->t('Storage not valid')], Http::STATUS_BAD_REQUEST); } @@ -312,18 +316,18 @@ class MountPublicLinkController extends Controller { ] ); } catch (StorageInvalidException $e) { - Util::writeLog( - 'federatedfilesharing', - 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(), - Util::DEBUG - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Invalid remote storage.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'federatedfilesharing' + ]); return new JSONResponse(['message' => $this->l->t('Storage not valid')], Http::STATUS_BAD_REQUEST); } catch (\Exception $e) { - Util::writeLog( - 'federatedfilesharing', - 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(), - Util::DEBUG - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Invalid remote storage.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'federatedfilesharing' + ]); return new JSONResponse(['message' => $this->l->t('Couldn\'t add remote share')], Http::STATUS_BAD_REQUEST); } } else { diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index a74342f118..bc81db50b2 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -209,7 +209,11 @@ class RequestHandlerController extends OCSController { return new Http\DataResponse(); } catch (\Exception $e) { - \OCP\Util::writeLog('files_sharing', 'server can not add remote share, ' . $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Server can not add remote share.', + 'level' => \OCP\Util::ERROR, + 'app' => 'files_sharing' + ]); throw new OCSException('internal server error, was not able to add share from ' . $remote, 500); } } diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 27606c5fde..1025c3c4ae 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -264,7 +264,11 @@ class FederatedShareProvider implements IShareProvider { $failure = true; } } catch (\Exception $e) { - $this->logger->error('Failed to notify remote server of federated share, removing share (' . $e->getMessage() . ')'); + $this->logger->logException($e, [ + 'message' => 'Failed to notify remote server of federated share, removing share.', + 'level' => \OCP\Util::ERROR, + 'app' => 'federatedfilesharing', + ]); $failure = true; } diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php index 92bb31e369..829e04ddb2 100644 --- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php @@ -201,10 +201,18 @@ class GetSharedSecret extends Job { } } catch (RequestException $e) { $status = -1; // There is no status code if we could not connect - $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); + $this->logger->logException($e, [ + 'message' => 'Could not connect to ' . $target, + 'level' => \OCP\Util::INFO, + 'app' => 'federation', + ]); } catch (RingException $e) { $status = -1; // There is no status code if we could not connect - $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); + $this->logger->logException($e, [ + 'message' => 'Could not connect to ' . $target, + 'level' => \OCP\Util::INFO, + 'app' => 'federation', + ]); } catch (\Exception $e) { $status = Http::STATUS_INTERNAL_SERVER_ERROR; $this->logger->logException($e, ['app' => 'federation']); diff --git a/apps/federation/lib/Middleware/AddServerMiddleware.php b/apps/federation/lib/Middleware/AddServerMiddleware.php index 247cc95883..c6ac42bd7c 100644 --- a/apps/federation/lib/Middleware/AddServerMiddleware.php +++ b/apps/federation/lib/Middleware/AddServerMiddleware.php @@ -71,7 +71,10 @@ class AddServerMiddleware extends Middleware { if (($controller instanceof SettingsController) === false) { throw $exception; } - $this->logger->error($exception->getMessage(), ['app' => $this->appName]); + $this->logger->logException($exception, [ + 'level' => \OCP\Util::ERROR, + 'app' => $this->appName, + ]); if ($exception instanceof HintException) { $message = $exception->getHint(); } else { diff --git a/apps/federation/lib/SyncJob.php b/apps/federation/lib/SyncJob.php index 0aa1d61aff..23255f2a44 100644 --- a/apps/federation/lib/SyncJob.php +++ b/apps/federation/lib/SyncJob.php @@ -49,7 +49,11 @@ class SyncJob extends TimedJob { protected function run($argument) { $this->syncService->syncThemAll(function($url, $ex) { if ($ex instanceof \Exception) { - $this->logger->error("Error while syncing $url : " . $ex->getMessage(), ['app' => 'fed-sync']); + $this->logger->logException($ex, [ + 'message' => "Error while syncing $url.", + 'level' => \OCP\Util::ERROR, + 'app' => 'fed-sync', + ]); } }); } diff --git a/apps/federation/lib/TrustedServers.php b/apps/federation/lib/TrustedServers.php index 79cf86ab67..2067533c3e 100644 --- a/apps/federation/lib/TrustedServers.php +++ b/apps/federation/lib/TrustedServers.php @@ -241,7 +241,11 @@ class TrustedServers { } } catch (\Exception $e) { - $this->logger->debug('No Nextcloud server: ' . $e->getMessage()); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'No Nextcloud server.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'federation', + ]); return false; } diff --git a/apps/federation/tests/Middleware/AddServerMiddlewareTest.php b/apps/federation/tests/Middleware/AddServerMiddlewareTest.php index 4620234d68..7bae5e566f 100644 --- a/apps/federation/tests/Middleware/AddServerMiddlewareTest.php +++ b/apps/federation/tests/Middleware/AddServerMiddlewareTest.php @@ -68,13 +68,11 @@ class AddServerMiddlewareTest extends TestCase { * @dataProvider dataTestAfterException * * @param \Exception $exception - * @param string $message * @param string $hint */ - public function testAfterException($exception, $message, $hint) { + public function testAfterException($exception, $hint) { - $this->logger->expects($this->once())->method('error') - ->with($message, ['app' => 'AddServerMiddlewareTest']); + $this->logger->expects($this->once())->method('logException'); $this->l10n->expects($this->any())->method('t') ->willReturnCallback( @@ -98,8 +96,8 @@ class AddServerMiddlewareTest extends TestCase { public function dataTestAfterException() { return [ - [new HintException('message', 'hint'), 'message', 'hint'], - [new \Exception('message'), 'message', 'message'], + [new HintException('message', 'hint'), 'hint'], + [new \Exception('message'), 'message'], ]; } diff --git a/apps/files_external/lib/Lib/Storage/Swift.php b/apps/files_external/lib/Lib/Storage/Swift.php index f07087e903..aeedd3dcd6 100644 --- a/apps/files_external/lib/Lib/Storage/Swift.php +++ b/apps/files_external/lib/Lib/Storage/Swift.php @@ -138,7 +138,10 @@ class Swift extends \OC\Files\Storage\Common { } catch (ClientErrorResponseException $e) { // Expected response is "404 Not Found", so only log if it isn't if ($e->getResponse()->getStatusCode() !== 404) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); } return false; } @@ -206,7 +209,10 @@ class Swift extends \OC\Files\Storage\Common { // with all properties $this->objectCache->remove($path); } catch (Exceptions\CreateUpdateError $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } @@ -247,7 +253,10 @@ class Swift extends \OC\Files\Storage\Common { $this->getContainer()->dataObject()->setName($path . '/')->delete(); $this->objectCache->remove($path . '/'); } catch (Exceptions\DeleteError $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } @@ -283,7 +292,10 @@ class Swift extends \OC\Files\Storage\Common { return IteratorDirectory::wrap($files); } catch (\Exception $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } @@ -305,7 +317,10 @@ class Swift extends \OC\Files\Storage\Common { return false; } } catch (ClientErrorResponseException $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } @@ -361,7 +376,10 @@ class Swift extends \OC\Files\Storage\Common { $this->objectCache->remove($path . '/'); } catch (ClientErrorResponseException $e) { if ($e->getResponse()->getStatusCode() !== 404) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); } return false; } @@ -394,7 +412,10 @@ class Swift extends \OC\Files\Storage\Common { } return false; } catch (\Guzzle\Http\Exception\BadResponseException $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } case 'w': @@ -474,7 +495,10 @@ class Swift extends \OC\Files\Storage\Common { $this->objectCache->remove($path2); $this->objectCache->remove($path2 . '/'); } catch (ClientErrorResponseException $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } @@ -490,7 +514,10 @@ class Swift extends \OC\Files\Storage\Common { $this->objectCache->remove($path2); $this->objectCache->remove($path2 . '/'); } catch (ClientErrorResponseException $e) { - \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return false; } diff --git a/apps/files_external/lib/Service/LegacyStoragesService.php b/apps/files_external/lib/Service/LegacyStoragesService.php index 68faf0f285..6588051491 100644 --- a/apps/files_external/lib/Service/LegacyStoragesService.php +++ b/apps/files_external/lib/Service/LegacyStoragesService.php @@ -192,11 +192,11 @@ abstract class LegacyStoragesService { } } catch (\UnexpectedValueException $e) { // don't die if a storage backend doesn't exist - \OCP\Util::writeLog( - 'files_external', - 'Could not load storage: "' . $e->getMessage() . '"', - \OCP\Util::ERROR - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Could not load storage.', + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); } } } diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php index d52bf41046..89458b70c0 100644 --- a/apps/files_external/lib/Service/StoragesService.php +++ b/apps/files_external/lib/Service/StoragesService.php @@ -102,18 +102,18 @@ abstract class StoragesService { return $config; } catch (\UnexpectedValueException $e) { // don't die if a storage backend doesn't exist - \OCP\Util::writeLog( - 'files_external', - 'Could not load storage: "' . $e->getMessage() . '"', - \OCP\Util::ERROR - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Could not load storage.', + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return null; } catch (\InvalidArgumentException $e) { - \OCP\Util::writeLog( - 'files_external', - 'Could not load storage: "' . $e->getMessage() . '"', - \OCP\Util::ERROR - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Could not load storage.', + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); return null; } } @@ -478,11 +478,10 @@ abstract class StoragesService { // can happen either for invalid configs where the storage could not // be instantiated or whenever $user vars where used, in which case // the storage id could not be computed - \OCP\Util::writeLog( - 'files_external', - 'Exception: "' . $e->getMessage() . '"', - \OCP\Util::ERROR - ); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'files_external', + ]); } } diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 10876e1656..990571b778 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -482,7 +482,7 @@ class ShareAPIController extends OCSController { $code = $e->getCode() === 0 ? 403 : $e->getCode(); throw new OCSException($e->getHint(), $code); } catch (\Exception $e) { - throw new OCSForbiddenException($e->getMessage()); + throw new OCSForbiddenException($e->getMessage(), $e); } $output = $this->formatShare($share); @@ -749,7 +749,7 @@ class ShareAPIController extends OCSController { try { $expireDate = $this->parseDate($expireDate); } catch (\Exception $e) { - throw new OCSBadRequestException($e->getMessage()); + throw new OCSBadRequestException($e->getMessage(), $e); } $share->setExpirationDate($expireDate); } @@ -780,7 +780,7 @@ class ShareAPIController extends OCSController { try { $expireDate = $this->parseDate($expireDate); } catch (\Exception $e) { - throw new OCSBadRequestException($e->getMessage()); + throw new OCSBadRequestException($e->getMessage(), $e); } $share->setExpirationDate($expireDate); } @@ -809,7 +809,7 @@ class ShareAPIController extends OCSController { try { $share = $this->shareManager->updateShare($share); } catch (\Exception $e) { - throw new OCSBadRequestException($e->getMessage()); + throw new OCSBadRequestException($e->getMessage(), $e); } return new DataResponse($this->formatShare($share)); diff --git a/apps/files_trashbin/lib/Storage.php b/apps/files_trashbin/lib/Storage.php index 5eaf502f23..e6609034f9 100644 --- a/apps/files_trashbin/lib/Storage.php +++ b/apps/files_trashbin/lib/Storage.php @@ -130,8 +130,11 @@ class Storage extends Wrapper { } } catch (\Exception $e) { // do nothing, in this case we just disable the trashbin and continue - $logger = \OC::$server->getLogger(); - $logger->debug('Trashbin storage could not check if a file was moved out of a shared folder: ' . $e->getMessage()); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Trashbin storage could not check if a file was moved out of a shared folder.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'files_trashbin', + ]); } if($fileMovedOutOfSharedFolder) { diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index ffae68d5a9..9098797f1a 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -197,7 +197,11 @@ class UsersController extends OCSController { } return new DataResponse(); } catch (\Exception $e) { - $this->logger->error('Failed addUser attempt with exception: '.$e->getMessage(), ['app' => 'ocs_api']); + $this->logger->logException($e, [ + 'message' => 'Failed addUser attempt with exception.', + 'level' => \OCP\Util::ERROR, + 'app' => 'ocs_api', + ]); throw new OCSException('Bad request', 101); } } @@ -826,7 +830,11 @@ class UsersController extends OCSController { $emailTemplate = $this->newUserMailHelper->generateTemplate($targetUser, false); $this->newUserMailHelper->sendMail($targetUser, $emailTemplate); } catch(\Exception $e) { - $this->logger->error("Can't send new user mail to $email: " . $e->getMessage(), array('app' => 'settings')); + $this->logger->logException($e, [ + 'message' => "Can't send new user mail to $email", + 'level' => \OCP\Util::ERROR, + 'app' => 'settings', + ]); throw new OCSException('Sending email failed', 102); } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index ef47583e9d..c2ad36c455 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -414,6 +414,7 @@ class UsersControllerTest extends TestCase { * @expectedExceptionMessage Bad request */ public function testAddUserUnsuccessful() { + $exception = new Exception('User backend not found.'); $this->userManager ->expects($this->once()) ->method('userExists') @@ -423,11 +424,15 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('createUser') ->with('NewUser', 'PasswordOfTheNewUser') - ->will($this->throwException(new Exception('User backend not found.'))); + ->will($this->throwException($exception)); $this->logger ->expects($this->once()) - ->method('error') - ->with('Failed addUser attempt with exception: User backend not found.', ['app' => 'ocs_api']); + ->method('logException') + ->with($exception, [ + 'message' => 'Failed addUser attempt with exception.', + 'level' => \OCP\Util::ERROR, + 'app' => 'ocs_api', + ]); $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index f610a1a5fa..287c6abff5 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -351,11 +351,19 @@ class ShareByMailProvider implements IShareProvider { $share->getExpirationDate() ); } catch (HintException $hintException) { - $this->logger->error('Failed to send share by mail: ' . $hintException->getMessage()); + $this->logger->logException($hintException, [ + 'message' => 'Failed to send share by mail.', + 'level' => \OCP\Util::ERROR, + 'app' => 'sharebymail', + ]); $this->removeShareFromTable($shareId); throw $hintException; } catch (\Exception $e) { - $this->logger->error('Failed to send share by email: ' . $e->getMessage()); + $this->logger->logException($e, [ + 'message' => 'Failed to send share by mail.', + 'level' => \OCP\Util::ERROR, + 'app' => 'sharebymail', + ]); $this->removeShareFromTable($shareId); throw new HintException('Failed to send share by mail', $this->l->t('Failed to send share by email')); diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index c93d2a77d8..8bde353a56 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -595,9 +595,11 @@ class User { $avatar = $this->avatarManager->getAvatar($this->uid); $avatar->set($this->image); } catch (\Exception $e) { - \OC::$server->getLogger()->notice( - 'Could not set avatar for ' . $this->dn . ', because: ' . $e->getMessage(), - ['app' => 'user_ldap']); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Could not set avatar for ' . $this->dn, + 'level' => \OCP\Util::INFO, + 'app' => 'user_ldap', + ]); } } diff --git a/core/ajax/update.php b/core/ajax/update.php index 239b073dc9..e2bd03b491 100644 --- a/core/ajax/update.php +++ b/core/ajax/update.php @@ -207,6 +207,10 @@ if (OC::checkUpgrade(false)) { try { $updater->upgrade(); } catch (\Exception $e) { + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'update', + ]); $eventSource->send('failure', get_class($e) . ': ' . $e->getMessage()); $eventSource->close(); exit(); diff --git a/lib/base.php b/lib/base.php index f763ee634f..e55cc4b3f9 100644 --- a/lib/base.php +++ b/lib/base.php @@ -831,7 +831,11 @@ class OC { } catch (\Exception $e) { // a GC exception should not prevent users from using OC, // so log the exception - \OC::$server->getLogger()->warning('Exception when running cache gc: ' . $e->getMessage(), array('app' => 'core')); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Exception when running cache gc.', + 'level' => \OCP\Util::WARN, + 'app' => 'core', + ]); } }); } diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index c147b5b247..1c049fb362 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -252,7 +252,10 @@ class SecurityMiddleware extends Middleware { } } - $this->logger->debug($exception->getMessage()); + $this->logger->logException($exception, [ + 'level' => \OCP\Util::DEBUG, + 'app' => 'core', + ]); return $response; } diff --git a/lib/private/CapabilitiesManager.php b/lib/private/CapabilitiesManager.php index fc8f14a7a8..fb27276459 100644 --- a/lib/private/CapabilitiesManager.php +++ b/lib/private/CapabilitiesManager.php @@ -55,7 +55,11 @@ class CapabilitiesManager { try { $c = $capability(); } catch (QueryException $e) { - $this->logger->error('CapabilitiesManager: {message}', ['app' => 'core', 'message' => $e->getMessage()]); + $this->logger->logException($e, [ + 'message' => 'CapabilitiesManager', + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); continue; } diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 85a89962f7..28a3d11ffa 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -452,7 +452,11 @@ class Scanner extends BasicEmitter implements IScanner { \OC::$server->getDatabaseConnection()->rollback(); \OC::$server->getDatabaseConnection()->beginTransaction(); } - \OCP\Util::writeLog('core', 'Exception while scanning file "' . $child . '": ' . $ex->getMessage(), \OCP\Util::DEBUG); + \OC::$server->getLogger()->logException($ex, [ + 'message' => 'Exception while scanning file "' . $child . '"', + 'level' => \OCP\Util::DEBUG, + 'app' => 'core', + ]); $exceptionOccurred = true; } catch (\OCP\Lock\LockedException $e) { if ($this->useTransactions) { diff --git a/lib/private/Files/ObjectStore/S3ConnectionTrait.php b/lib/private/Files/ObjectStore/S3ConnectionTrait.php index d491083496..7e70cc846e 100644 --- a/lib/private/Files/ObjectStore/S3ConnectionTrait.php +++ b/lib/private/Files/ObjectStore/S3ConnectionTrait.php @@ -109,7 +109,11 @@ trait S3ConnectionTrait { )); $this->testTimeout(); } catch (S3Exception $e) { - \OCP\Util::logException('files_external', $e); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Invalid remote storage.', + 'level' => \OCP\Util::DEBUG, + 'app' => 'files_external', + ]); throw new \Exception('Creation of bucket failed. ' . $e->getMessage()); } } diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 6585dd862e..43347aa0b8 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -830,8 +830,7 @@ class DAV extends Common { * which might be temporary */ protected function convertException(Exception $e, $path = '') { - \OC::$server->getLogger()->logException($e); - Util::writeLog('files_external', $e->getMessage(), Util::ERROR); + \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); if ($e instanceof ClientHttpException) { if ($e->getHttpStatus() === Http::STATUS_LOCKED) { throw new \OCP\Lock\LockedException($path); diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 1ca750f002..d443ccfd49 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -441,8 +441,11 @@ class Encryption extends Wrapper { } } } catch (ModuleDoesNotExistsException $e) { - $this->logger->warning('Encryption module "' . $encryptionModuleId . - '" not found, file will be stored unencrypted (' . $e->getMessage() . ')'); + $this->logger->logException($e, [ + 'message' => 'Encryption module "' . $encryptionModuleId . '" not found, file will be stored unencrypted', + 'level' => \OCP\Util::WARN, + 'app' => 'core', + ]); } // encryption disabled on write of new file and write to existing unencrypted file -> don't encrypt diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 592d4b717c..fb0b116666 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1464,12 +1464,11 @@ class View { continue; } catch (\Exception $e) { // sometimes when the storage is not available it can be any exception - \OCP\Util::writeLog( - 'core', - 'Exception while scanning storage "' . $subStorage->getId() . '": ' . - get_class($e) . ': ' . $e->getMessage(), - \OCP\Util::ERROR - ); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Exception while scanning storage "' . $subStorage->getId() . '"', + 'level' => \OCP\Util::ERROR, + 'app' => 'lib', + ]); continue; } $rootEntry = $subCache->get(''); diff --git a/lib/private/Installer.php b/lib/private/Installer.php index 1c6c3915b4..ed654c2b24 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -195,7 +195,10 @@ class Installer { try { $this->downloadApp($appId); } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['app' => 'core']); + $this->logger->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } return OC_App::updateApp($appId); diff --git a/lib/private/Preview/Bitmap.php b/lib/private/Preview/Bitmap.php index a375717480..ff6093fcf8 100644 --- a/lib/private/Preview/Bitmap.php +++ b/lib/private/Preview/Bitmap.php @@ -48,7 +48,11 @@ abstract class Bitmap extends Provider { try { $bp = $this->getResizedPreview($tmpPath, $maxX, $maxY); } catch (\Exception $e) { - \OCP\Util::writeLog('core', 'ImageMagick says: ' . $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => 'Imagick says:', + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } diff --git a/lib/private/Preview/Office.php b/lib/private/Preview/Office.php index 2ec677c04f..6403cf5dd8 100644 --- a/lib/private/Preview/Office.php +++ b/lib/private/Preview/Office.php @@ -59,7 +59,10 @@ abstract class Office extends Provider { } catch (\Exception $e) { unlink($absPath); unlink($pdfPreview); - \OCP\Util::writeLog('core', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } diff --git a/lib/private/Preview/SVG.php b/lib/private/Preview/SVG.php index 0695c251c8..e0abf04d93 100644 --- a/lib/private/Preview/SVG.php +++ b/lib/private/Preview/SVG.php @@ -53,7 +53,10 @@ class SVG extends Provider { $svg->readImageBlob($content); $svg->setImageFormat('png32'); } catch (\Exception $e) { - \OCP\Util::writeLog('core', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } diff --git a/lib/private/Setup/MySQL.php b/lib/private/Setup/MySQL.php index 30163f0365..9d80bd49ae 100644 --- a/lib/private/Setup/MySQL.php +++ b/lib/private/Setup/MySQL.php @@ -68,9 +68,10 @@ class MySQL extends AbstractDatabase { $query = "CREATE DATABASE IF NOT EXISTS `$name` CHARACTER SET $characterSet COLLATE ${characterSet}_bin;"; $connection->executeUpdate($query); } catch (\Exception $ex) { - $this->logger->error('Database creation failed: {error}', [ + $this->logger->logException($ex, [ + 'message' => 'Database creation failed.', + 'level' => \OCP\Util::ERROR, 'app' => 'mysql.setup', - 'error' => $ex->getMessage() ]); return; } @@ -80,9 +81,10 @@ class MySQL extends AbstractDatabase { $query="GRANT ALL PRIVILEGES ON `$name` . * TO '$user'"; $connection->executeUpdate($query); } catch (\Exception $ex) { - $this->logger->debug('Could not automatically grant privileges, this can be ignored if database user already had privileges: {error}', [ + $this->logger->logException($ex, [ + 'message' => 'Could not automatically grant privileges, this can be ignored if database user already had privileges.', + 'level' => \OCP\Util::DEBUG, 'app' => 'mysql.setup', - 'error' => $ex->getMessage() ]); } } @@ -103,10 +105,11 @@ class MySQL extends AbstractDatabase { $connection->executeUpdate($query); } catch (\Exception $ex){ - $this->logger->error('Database User creation failed: {error}', [ - 'app' => 'mysql.setup', - 'error' => $ex->getMessage() - ]); + $this->logger->logException($ex, [ + 'message' => 'Database user creation failed.', + 'level' => \OCP\Util::ERROR, + 'app' => 'mysql.setup', + ]); } } @@ -157,9 +160,10 @@ class MySQL extends AbstractDatabase { }; } } catch (\Exception $ex) { - $this->logger->info('Can not create a new MySQL user, will continue with the provided user: {error}', [ + $this->logger->logException($ex, [ + 'message' => 'Can not create a new MySQL user, will continue with the provided user.', + 'level' => \OCP\Util::INFO, 'app' => 'mysql.setup', - 'error' => $ex->getMessage() ]); } diff --git a/lib/private/Tags.php b/lib/private/Tags.php index 2ac7048484..1b3505da5b 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -242,8 +242,11 @@ class Tags implements \OCP\ITags { } } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } @@ -292,8 +295,11 @@ class Tags implements \OCP\ITags { return false; } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } @@ -366,8 +372,11 @@ class Tags implements \OCP\ITags { $tag = $this->mapper->insert($tag); $this->tags[] = $tag; } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } \OCP\Util::writeLog('core', __METHOD__.', id: ' . $tag->getId(), \OCP\Util::DEBUG); @@ -410,8 +419,11 @@ class Tags implements \OCP\ITags { $tag->setName($to); $this->tags[$key] = $this->mapper->update($tag); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } return true; @@ -462,8 +474,11 @@ class Tags implements \OCP\ITags { $this->mapper->insert($tag); } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } } @@ -488,8 +503,11 @@ class Tags implements \OCP\ITags { 'type' => $this->type, )); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } } } @@ -518,8 +536,11 @@ class Tags implements \OCP\ITags { \OCP\Util::writeLog('core', __METHOD__. 'DB error: ' . \OCP\DB::getErrorMessage(), \OCP\Util::ERROR); } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } if(!is_null($result)) { @@ -530,13 +551,19 @@ class Tags implements \OCP\ITags { try { $stmt->execute(array($row['id'])); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } } try { @@ -547,8 +574,11 @@ class Tags implements \OCP\ITags { \OCP\Util::writeLog('core', __METHOD__. ', DB error: ' . \OCP\DB::getErrorMessage(), \OCP\Util::ERROR); } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__ . ', exception: ' - . $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); } } @@ -576,8 +606,11 @@ class Tags implements \OCP\ITags { return false; } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: ' . $e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } return true; @@ -592,8 +625,11 @@ class Tags implements \OCP\ITags { try { return $this->getIdsForTag(self::TAG_FAVORITE); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: ' . $e->getMessage(), - \OCP\Util::DEBUG); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return array(); } } @@ -650,8 +686,11 @@ class Tags implements \OCP\ITags { 'type' => $this->type, )); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } return true; @@ -682,8 +721,11 @@ class Tags implements \OCP\ITags { $stmt = \OCP\DB::prepare($sql); $stmt->execute(array($objid, $tagId, $this->type)); } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } return true; @@ -735,8 +777,11 @@ class Tags implements \OCP\ITags { return false; } } catch(\Exception $e) { - \OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), - \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'message' => __METHOD__, + 'level' => \OCP\Util::ERROR, + 'app' => 'core', + ]); return false; } } diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 53ffd62a06..af5aff8371 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -487,7 +487,11 @@ class UsersController extends Controller { $emailTemplate = $this->newUserMailHelper->generateTemplate($user, $generatePasswordResetToken); $this->newUserMailHelper->sendMail($user, $emailTemplate); } catch (\Exception $e) { - $this->log->error("Can't send new user mail to $email: " . $e->getMessage(), ['app' => 'settings']); + $this->log->logException($e, [ + 'message' => "Can't send new user mail to $email", + 'level' => \OCP\Util::ERROR, + 'app' => 'settings', + ]); } } // fetch users groups diff --git a/settings/ajax/enableapp.php b/settings/ajax/enableapp.php index 3d2d7f95a5..4014664481 100644 --- a/settings/ajax/enableapp.php +++ b/settings/ajax/enableapp.php @@ -50,6 +50,9 @@ try { OC_JSON::success(['data' => ['update_required' => $updateRequired]]); } catch (Exception $e) { - \OCP\Util::writeLog('core', $e->getMessage(), \OCP\Util::ERROR); + \OC::$server->getLogger()->logException($e, [ + 'level' => \OCP\Util::DEBUG, + 'app' => 'core', + ]); OC_JSON::error(array("data" => array("message" => $e->getMessage()) )); } diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 151d6935e7..0d7418673d 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -483,8 +483,7 @@ class SecurityMiddlewareTest extends \Test\TestCase { ->will($this->returnValue('http://localhost/nextcloud/index.php/login?redirect_url=nextcloud/index.php/apps/specialapp')); $this->logger ->expects($this->once()) - ->method('debug') - ->with('Current user is not logged in'); + ->method('logException'); $response = $this->middleware->afterException( $this->controller, 'test', @@ -554,8 +553,7 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->middleware = $this->getMiddleware(false, false); $this->logger ->expects($this->once()) - ->method('debug') - ->with($exception->getMessage()); + ->method('logException'); $response = $this->middleware->afterException( $this->controller, 'test', diff --git a/tests/lib/CapabilitiesManagerTest.php b/tests/lib/CapabilitiesManagerTest.php index 8df385e6ef..c967b77130 100644 --- a/tests/lib/CapabilitiesManagerTest.php +++ b/tests/lib/CapabilitiesManagerTest.php @@ -148,7 +148,7 @@ class CapabilitiesManagerTest extends TestCase { }); $this->logger->expects($this->once()) - ->method('error'); + ->method('logException'); $res = $this->manager->getCapabilities();