From 64c8427d813445aceb019c08a417633ace17d366 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 14 Jan 2016 09:49:40 +0100 Subject: [PATCH 1/3] Improved error message for failing background job --- lib/private/backgroundjob/job.php | 2 +- tests/lib/backgroundjob/job.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/backgroundjob/job.php b/lib/private/backgroundjob/job.php index 677b37e929..4c4da94d4f 100644 --- a/lib/private/backgroundjob/job.php +++ b/lib/private/backgroundjob/job.php @@ -52,7 +52,7 @@ abstract class Job implements IJob { $this->run($this->argument); } catch (\Exception $e) { if ($logger) { - $logger->error('Error while running background job: ' . $e->getMessage()); + $logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . '): ' . $e->getMessage()); } } } diff --git a/tests/lib/backgroundjob/job.php b/tests/lib/backgroundjob/job.php index 912e0e13b5..f2a5ff894f 100644 --- a/tests/lib/backgroundjob/job.php +++ b/tests/lib/backgroundjob/job.php @@ -28,7 +28,7 @@ class Job extends \Test\TestCase { ->getMock(); $logger->expects($this->once()) ->method('error') - ->with('Error while running background job: '); + ->with('Error while running background job (class: Test\BackgroundJob\TestJob, arguments: ): '); $this->assertCount(1, $jobList->getAll()); $job->execute($jobList, $logger); From 86f08f59d6318a45a20b74e4b2b449ddc6882419 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 14 Jan 2016 10:38:53 +0100 Subject: [PATCH 2/3] use logException() to properly log the exception --- lib/private/backgroundjob/job.php | 3 ++- tests/lib/backgroundjob/job.php | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/private/backgroundjob/job.php b/lib/private/backgroundjob/job.php index 4c4da94d4f..d666cfe085 100644 --- a/lib/private/backgroundjob/job.php +++ b/lib/private/backgroundjob/job.php @@ -52,7 +52,8 @@ abstract class Job implements IJob { $this->run($this->argument); } catch (\Exception $e) { if ($logger) { - $logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . '): ' . $e->getMessage()); + $logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')'); + $logger->logException($e); } } } diff --git a/tests/lib/backgroundjob/job.php b/tests/lib/backgroundjob/job.php index f2a5ff894f..75b4865819 100644 --- a/tests/lib/backgroundjob/job.php +++ b/tests/lib/backgroundjob/job.php @@ -18,8 +18,9 @@ class Job extends \Test\TestCase { public function testRemoveAfterException() { $jobList = new DummyJobList(); - $job = new TestJob($this, function () { - throw new \Exception(); + $e = new \Exception(); + $job = new TestJob($this, function () use ($e) { + throw $e; }); $jobList->add($job); @@ -28,7 +29,10 @@ class Job extends \Test\TestCase { ->getMock(); $logger->expects($this->once()) ->method('error') - ->with('Error while running background job (class: Test\BackgroundJob\TestJob, arguments: ): '); + ->with('Error while running background job (class: Test\BackgroundJob\TestJob, arguments: )'); + $logger->expects($this->once()) + ->method('logException') + ->with($e); $this->assertCount(1, $jobList->getAll()); $job->execute($jobList, $logger); From f6c4b10189c6d76d94b0b01a440a326e6346c301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 15 Jan 2016 13:13:27 +0100 Subject: [PATCH 3/3] Add message key to context of logException --- lib/private/backgroundjob/job.php | 6 ++++-- lib/private/log.php | 4 +++- lib/public/ilogger.php | 8 ++++++++ tests/lib/backgroundjob/job.php | 3 --- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/private/backgroundjob/job.php b/lib/private/backgroundjob/job.php index d666cfe085..e726889484 100644 --- a/lib/private/backgroundjob/job.php +++ b/lib/private/backgroundjob/job.php @@ -52,8 +52,10 @@ abstract class Job implements IJob { $this->run($this->argument); } catch (\Exception $e) { if ($logger) { - $logger->error('Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')'); - $logger->logException($e); + $logger->logException($e, [ + 'app' => 'core', + 'message' => 'Error while running background job (class: ' . get_class($this) . ', arguments: ' . print_r($this->argument, true) . ')' + ]); } } } diff --git a/lib/private/log.php b/lib/private/log.php index 6c1666a9d7..addefe6e53 100644 --- a/lib/private/log.php +++ b/lib/private/log.php @@ -285,6 +285,8 @@ class Log implements ILogger { 'Line' => $exception->getLine(), ); $exception['Trace'] = preg_replace('!(login|checkPassword)\(.*\)!', '$1(*** username and password replaced ***)', $exception['Trace']); - $this->error('Exception: ' . json_encode($exception), $context); + $msg = isset($context['message']) ? $context['message'] : 'Exception'; + $msg .= ': ' . json_encode($exception); + $this->error($msg, $context); } } diff --git a/lib/public/ilogger.php b/lib/public/ilogger.php index 368b25ab69..2a727697a6 100644 --- a/lib/public/ilogger.php +++ b/lib/public/ilogger.php @@ -125,6 +125,14 @@ interface ILogger { /** * Logs an exception very detailed + * An additional message can we written to the log by adding it to the + * context. + * + * + * $logger->logException($ex, [ + * 'message' => 'Exception during cron job execution' + * ]); + * * * @param \Exception $exception * @param array $context diff --git a/tests/lib/backgroundjob/job.php b/tests/lib/backgroundjob/job.php index 75b4865819..12413e2c52 100644 --- a/tests/lib/backgroundjob/job.php +++ b/tests/lib/backgroundjob/job.php @@ -27,9 +27,6 @@ class Job extends \Test\TestCase { $logger = $this->getMockBuilder('OCP\ILogger') ->disableOriginalConstructor() ->getMock(); - $logger->expects($this->once()) - ->method('error') - ->with('Error while running background job (class: Test\BackgroundJob\TestJob, arguments: )'); $logger->expects($this->once()) ->method('logException') ->with($e);