Better exception handling, hopefully less exception loops when there …

…is an exception logging the exception 😅 Reverting context->exception->trace back ot getTrace() vs getTraceAsString().
danielme85 · May 6, 2020 · 77ba74a · 77ba74a
1 parent af17616
commit 77ba74a
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 73 deletions.
diff --git a/src/Jobs/SaveNewLogEvent.php b/src/Jobs/SaveNewLogEvent.php
@@ -3,6 +3,7 @@
 namespace danielme85\LaravelLogToDB\Jobs;
 
 use danielme85\LaravelLogToDB\Models\CreateLogFromRecord;
+use danielme85\LaravelLogToDB\Models\DBLogException;
 use Illuminate\Bus\Queueable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Contracts\Queue\ShouldQueue;
@@ -26,7 +27,7 @@ class SaveNewLogEvent implements ShouldQueue
      * Create a new job instance.
      *
      * @param object $logToDb
-     * @param  array $record
+     * @param array $record
      * @return void
      */
     public function __construct($logToDb, $record)
@@ -42,12 +43,15 @@ public function __construct($logToDb, $record)
      */
     public function handle()
     {
-        $model = $this->logToDb->getModel();
-        $log = $model->generate(
-            $this->record,
-            $this->logToDb->getConfig('detailed')
-        );
-
-        $log->save();
+        try {
+            $model = $this->logToDb->getModel();
+            $log = $model->generate(
+                $this->record,
+                $this->logToDb->getConfig('detailed')
+            );
+            $log->save();
+        } catch (\Exception $e) {
+            throw new DBLogException($e->getMessage());
+        }
     }
 }
diff --git a/src/LogToDB.php b/src/LogToDB.php
@@ -146,11 +146,14 @@ public function getModel()
      */
     public function newFromMonolog(array $record)
     {
+        $detailed = $this->getConfig('detailed');
+
         if (!empty($this->connection)) {
-            if (!empty($record['context']) && !empty($record['context']['exception'])) {
-                $record['context']['exception'] = $this->parseIfException($record['context']['exception']);
+            if ($detailed && !empty($record['context']) && !empty($record['context']['exception'])) {
+                $record['context'] = $this->parseIfException($record['context']);
+            } else if (!$detailed){
+                $record['context'] = null;
             }
-
             if ($this->config['queue']) {
                 if (empty($this->config['queue_name']) && empty($this->config['queue_connection'])) {
                     dispatch(new SaveNewLogEvent($this, $record));
@@ -168,8 +171,7 @@ public function newFromMonolog(array $record)
             } else {
                 $model = $this->getModel();
                 $log = $model->generate(
-                    $record,
-                    $this->getConfig('detailed')
+                    $record
                 );
                 if ($log->save()) {
                     return true;
@@ -194,45 +196,52 @@ public function getConfig(string $config)
     /**
      * Parse the exception class
      *
-     * @param mixed $exception
+     * @param mixed $context
      * @return mixed
      */
-    private function parseIfException($exception)
+    private function parseIfException($context)
     {
-        if (is_object($exception)) {
-            if (get_class($exception) === \Exception::class
-                || is_subclass_of($exception, \Exception::class)
-                || strpos(get_class($exception), "Exception") !== false) {
-
-                $newexception = [];
-
-                if (method_exists($exception, 'getMessage')) {
-                    $newexception['message'] = $exception->getMessage();
-                }
-                if (method_exists($exception, 'getCode')) {
-                    $newexception['code'] = $exception->getCode();
-                }
-                if (method_exists($exception, 'getFile')) {
-                    $newexception['file'] = $exception->getFile();
-                }
-                if (method_exists($exception, 'getLine')) {
-                    $newexception['line'] = $exception->getLine();
+        if (!empty($context['exception'])) {
+            $exception = $context['exception'];
+            if (is_object($exception)) {
+                if (get_class($exception) === \Exception::class
+                    || is_subclass_of($exception, \Exception::class)
+                    || strpos(get_class($exception), "Exception") !== false) {
+
+                    $newexception = [];
+
+                    if (method_exists($exception, 'getMessage')) {
+                        $newexception['message'] = $exception->getMessage();
+                    }
+                    if (method_exists($exception, 'getCode')) {
+                        $newexception['code'] = $exception->getCode();
+                    }
+                    if (method_exists($exception, 'getFile')) {
+                        $newexception['file'] = $exception->getFile();
+                    }
+                    if (method_exists($exception, 'getLine')) {
+                        $newexception['line'] = $exception->getLine();
+                    }
+                    if (method_exists($exception, 'getTrace')) {
+                        $newexception['trace'] = $exception->getTrace();
+                    }
+                    if (method_exists($exception, 'getPrevious')) {
+                        $newexception['previous'] = $exception->getPrevious();
+                    }
+                    if (method_exists($exception, 'getSeverity')) {
+                        $newexception['severity'] = $exception->getSeverity();
+                    }
+
+                    $context['exception'] = $newexception;
                 }
-                if (method_exists($exception, 'getTrace')) {
-                    $newexception['trace'] = $exception->getTraceAsString();
-                }
-                if (method_exists($exception, 'getPrevious')) {
-                    $newexception['previous'] = $exception->getPrevious();
-                }
-                if (method_exists($exception, 'getSeverity')) {
-                    $newexception['severity'] = $exception->getSeverity();
-                }
-
-                $exception = $newexception;
             }
         }
 
-        return $exception;
+        if (!empty($context)) {
+            return json_encode($context);
+        }
+
+        return null;
     }
 
 }
diff --git a/src/LogToDbCustomLoggingHandler.php b/src/LogToDbCustomLoggingHandler.php
@@ -55,13 +55,15 @@ function __construct(array $config,
     protected function write(array $record): void
     {
         if (!empty($record)) {
-            if (!empty($record['context']['exception'])
-                && get_class($record['context']['exception']) === DBLogException::class) {
+            if (!empty($record['context']['exception']) &&
+                get_class($record['context']['exception']) === DBLogException::class) {
                 //Do nothing if empty log record or an error Exception from itself.
             } else {
                 try {
                     $log = new LogToDB($this->config);
                     $log->newFromMonolog($record);
+                } catch (DBLogException $e) {
+                    //do nothing if exception of self
                 } catch (\Exception $e) {
                     //convert any runtime Exception while logging to a special class so we can avoid our own
                     //exceptions for 99% less infinite loops!

diff --git a/src/Models/LogToDbCreateObject.php b/src/Models/LogToDbCreateObject.php
@@ -13,21 +13,16 @@ trait LogToDbCreateObject
      * Create a new log object
      *
      * @param array $record
-     * @param bool $detailed
      *
      * @return mixed
      */
-    public function generate(array $record, bool $detailed = false)
+    public function generate(array $record)
     {
         if (isset($record['message'])) {
             $this->message = $record['message'];
         }
-        if ($detailed) {
-            if (isset($record['context'])) {
-                if (!empty($record['context'])) {
-                    $this->context = $record['context'];
-                }
-            }
+        if (!empty($record['context'])) {
+            $this->context = $record['context'];
         }
         if (isset($record['level'])) {
             $this->level = $record['level'];
@@ -41,10 +36,8 @@ public function generate(array $record, bool $detailed = false)
         if (isset($record['datetime'])) {
             $this->datetime = $record['datetime'];
         }
-        if (isset($record['extra'])) {
-            if (!empty($record['extra'])) {
-                $this->extra = $record['extra'];
-            }
+        if (!empty($record['extra'])) {
+            $this->extra = $record['extra'];
         }
         $this->unix_time = time();
 
@@ -73,16 +66,6 @@ public function getExtraAttribute($value)
         return $this->jsonDecodeIfNotEmpty($value);
     }
 
-    /**
-     * Context Mutator
-     *
-     * @param array $value
-     */
-    public function setContextAttribute(array $value)
-    {
-        $this->attributes['context'] = $this->jsonEncodeIfNotEmpty($value);
-    }
-
     /**
      * DateTime Mutator
      *
@@ -114,8 +97,6 @@ private function jsonEncodeIfNotEmpty($value)
         if (!empty($value)) {
             return @json_encode($value) ?? null;
         }
-
-        return $value;
     }
 
     /**

diff --git a/tests/LogToDbTest.php b/tests/LogToDbTest.php
@@ -205,11 +205,12 @@ public function testLoggingToChannels()
     {
         //Test limited config, with limited rows and level
         Log::channel('limited')->debug("This message should not be stored because DEBUG is LOWER then WARNING");
-        $this->assertEmpty(LogToDB::model('limited')->where('channel', 'limited')->where('level_name', 'DEBUG')->get()->toArray());
+        $this->assertEmpty(LogToDB::model('limited')->where('channel', 'limited')->where('level_name', 'DEBUG')->get());
 
         //Test limited config, with limited rows and level
         Log::channel('limited')->warning("This message should be stored because WARNING = WARNING");
-        $this->assertNotEmpty(LogToDB::model('limited')->where('channel', 'limited')->where('level_name', 'WARNING')->get()->toArray());
+        $this->assertNotEmpty(LogToDB::model('limited')->where('channel', 'limited')->where('level_name', 'WARNING')->get());
+
     }
 
     /**
@@ -427,6 +428,8 @@ public function testCustomModel()
      */
     public function testRemoves()
     {
+        $this->assertFalse(LogToDB::model()->removeOldestIfMoreThan(1000));
+
         Log::debug("This is an test DEBUG log event");
         Log::info("This is an test INFO log event");
         Log::notice("This is an test NOTICE log event");