Fix database infinite loop (#66)

* Possible fix for #56

* Changed property visibility

* Changed some unit test

* Some more testing

* set proper assertions

* Anotha fix

* Rate limiting by using file cache

* Fixed missing classes

* Fixed missing carbon class

* Added rate limiting test

* debug

* Attempt to fix cache

* Now uses cache default config setting

* Missing use statement!

* Fixed unit test loop

* removed debug

* updated readme and changelogs

* Added config value for ratelimiting

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to `LERN` will be documented in this file.
 
+### 4.5.0
+- Added rate limiting of 1 second per Exception class for recording in the database
+- Added rate limiting of 1 second per Exception class for sending a notification
+- Made internal exception `Tylercd100\LERN\Exceptions\RecorderFailedException` notifiable. (previously it would be ignored and not sent on any notification channels)
+- Made internal exception `Tylercd100\LERN\Exceptions\NotifierFailedException` recordable. (previously it would be ignored and not be recorded in the database)
+
 ### 4.4.0
 - Added ability to use a custom Model
 - Added ability to change database connection for the default Model

README.md

@@ -4,7 +4,6 @@
 [![Build Status](https://travis-ci.org/tylercd100/lern.svg?branch=master)](https://travis-ci.org/tylercd100/lern)
 [![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/tylercd100/lern/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/tylercd100/lern/?branch=master)
 [![Code Coverage](https://scrutinizer-ci.com/g/tylercd100/lern/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/tylercd100/lern/?branch=master)
-[![Dependency Status](https://www.versioneye.com/user/projects/56f3252c35630e0029db0187/badge.svg?style=flat)](https://www.versioneye.com/user/projects/56f3252c35630e0029db0187)
 [![Total Downloads](https://img.shields.io/packagist/dt/tylercd100/lern.svg?style=flat-square)](https://packagist.org/packages/tylercd100/lern)
 
 **_LERN from your mistakes_**
@@ -188,4 +187,3 @@ LERN::notify($exception);
 ## Roadmap
 - Support more Monolog Handlers
 - Exception report page or command to easily identify your application's issues.
-- Notification rate limiting and/or grouping. 

config/lern.php

@@ -2,6 +2,13 @@
 
 return [
 
+    /** 
+     * To avoid infinite loops that generate thousands of records/notifications in an instant 
+     * Please make sure you use a Cache driver that is persistant such as redis, memcache, file, etc
+     * 
+     * Value is in seconds.
+     */
+    'ratelimit' => 1, 
 
     'record'=>[
         /**

phpunit.xml

@@ -8,7 +8,7 @@
          convertNoticesToExceptions="true"
          convertWarningsToExceptions="true"
          processIsolation="false"
-         stopOnFailure="false">
+         stopOnFailure="true">
     <php>
         <env name="APP_KEY" value="8bff3a6e61f548c0ecd9ad52e3cf3738"/>
     </php>

src/Components/Component.php

@@ -3,6 +3,8 @@
 namespace Tylercd100\LERN\Components;
 
 use Exception;
+use Illuminate\Support\Facades\Cache;
+use Carbon\Carbon;
 
 abstract class Component {
 
@@ -12,12 +14,11 @@ abstract class Component {
     protected $dontHandle = [];
 
     /**
+     * This array is overwritten in each component
+     * 
      * @var array
      */
-    private $absolutelyDontHandle = [
-        \Tylercd100\LERN\Exceptions\RecorderFailedException::class,
-        \Tylercd100\LERN\Exceptions\NotifierFailedException::class,
-    ];
+    protected $absolutelyDontHandle = [];
 
     /**
      * Determine if the exception is in the "do not handle" list.
@@ -34,6 +35,22 @@ protected function shouldntHandle(Exception $e) {
             }
         }
 
-        return false;
+        $sent_at = Cache::get($this->getCacheKey($e));
+        if (empty($sent_at) || $sent_at->addSeconds(config('lern.ratelimit', 1))->lte(Carbon::now())) {
+            return false; // The cache is empty or enough time has passed, so lets continue
+        } else {
+            return true;
+        }
+    }
+
+    /**
+     * Returns the cache key for the exception with the current component
+     * 
+     * @param \Exception $e
+     * @return string
+     */
+    protected function getCacheKey(Exception $e)
+    {
+        return "LERN::".static::class."::".get_class($e);
     }
 }

src/Components/Notifier.php

@@ -5,12 +5,14 @@
 use Auth;
 use Exception;
 use Illuminate\Support\Facades\Input;
+use Illuminate\Support\Facades\Cache;
 use Monolog\Handler\HandlerInterface;
 use Monolog\Logger;
 use Request;
 use Tylercd100\LERN\Exceptions\NotifierFailedException;
 use Tylercd100\Notify\Drivers\FromConfig as Notify;
 use View;
+use Carbon\Carbon;
 
 class Notifier extends Component
 {
@@ -20,6 +22,13 @@ class Notifier extends Component
     protected $subjectCb;
     protected $contextCb;
 
+    /**
+     * @var array
+     */
+    protected $absolutelyDontHandle = [
+        \Tylercd100\LERN\Exceptions\NotifierFailedException::class,
+    ];
+
     /**
      * You can provide a Monolog Logger instance to use in the constructor
      * @param Logger|null $log Logger instance to use
@@ -229,16 +238,18 @@ public function send(Exception $e, array $context = [])
         $message = $this->getMessage($e);
         $subject = $this->getSubject($e);
         $context = $this->getContext($e, $context);
-
+        
         try {
             $notify = new Notify($this->config, $this->log, $subject);
-
+            
             $level = (array_key_exists('log_level', $this->config) && !empty($this->config['log_level']))
-                ? $this->config['log_level']
-                : 'critical';
-
+            ? $this->config['log_level']
+            : 'critical';
+            
             $notify->{$level}($message, $context);
 
+            Cache::forever($this->getCacheKey($e), Carbon::now());
+
             return true;
         } catch (Exception $e) {
             $code = (is_int($e->getCode()) ? $e->getCode() : 0);

src/Components/Recorder.php

@@ -6,9 +6,11 @@
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Input;
 use Illuminate\Support\Facades\Request;
+use Illuminate\Support\Facades\Cache;
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 use Tylercd100\LERN\Exceptions\RecorderFailedException;
 use Tylercd100\LERN\Models\ExceptionModel;
+use Carbon\Carbon;
 
 class Recorder extends Component {
 
@@ -17,6 +19,13 @@ class Recorder extends Component {
      */
     protected $config = [];
 
+    /**
+     * @var array
+     */
+    protected $absolutelyDontHandle = [
+        \Tylercd100\LERN\Exceptions\RecorderFailedException::class,
+    ];
+
     /**
      * The constructor
      */
@@ -66,6 +75,9 @@ public function record(Exception $e)
             }
 
             $model->save();
+
+            Cache::forever($this->getCacheKey($e), Carbon::now());
+
             return $model;
         } catch (Exception $e) {
             $code = (is_int($e->getCode()) ? $e->getCode() : 0);

tests/LERNTest.php

@@ -6,6 +6,7 @@
 use Tylercd100\LERN\Facades\LERN as LERNFacade;
 use Tylercd100\LERN\Components\Notifier;
 use Tylercd100\LERN\Components\Recorder;
+use Tylercd100\LERN\Exceptions\RecorderFailedException;
 use Tylercd100\Notify\Factories\MonologHandlerFactory;
 use Exception;
 
@@ -131,4 +132,15 @@ public function testSettingAndGettingLogLevels()
         $result = $lern->getLogLevel();
         $this->assertEquals($result, $level);
     }
+
+    public function testCantConnectToDatabaseError()
+    {
+        $lern = new LERN;
+
+        // Mysql should not work as we have not configured it properly for testing.
+        // this should reproduce an error similar to having the database offline.
+        \Config::set("database.default", "mysql"); 
+        $this->expectException(RecorderFailedException::class);
+        $lern->handle(new Exception);
+    }
 }

tests/NotifierTest.php

@@ -5,7 +5,10 @@
 use Exception;
 use Tylercd100\LERN\Components\Notifier;
 use Tylercd100\LERN\Exceptions\NotifierFailedException;
+use Tylercd100\LERN\Exceptions\RecorderFailedException;
 use Tylercd100\Notify\Factories\MonologHandlerFactory;
+use Illuminate\Support\Facades\Cache;
+
 
 class NotifierTest extends TestCase
 {
@@ -43,6 +46,7 @@ public function testSendsDifferentLogLevels()
         $this->app['config']->set('lern.notify.drivers', ['slack']);
 
         foreach ($logLevels as $logLevel) {
+            Cache::flush();
             $this->app['config']->set('lern.notify.log_level', $logLevel);
 
             $observer = $this->getMockBuilder('Monolog\Logger')
@@ -182,4 +186,26 @@ public function testSendShouldReturnFalseWhenPassedNotifierFailedException()
         $result = $notifier->send(new NotifierFailedException);
         $this->assertEquals(false, $result);
     }
+
+    public function testSendShouldReturnTrueWhenPassedRecorderFailedException()
+    {
+        $notifier = new Notifier;
+        $result = $notifier->send(new RecorderFailedException);
+        $this->assertEquals(true, $result);
+    }
+
+    public function testRateLimiting()
+    {
+        $notifier = new Notifier;
+        $result = $notifier->send(new Exception);
+        $this->assertEquals(true, $result);
+
+        $result = $notifier->send(new Exception);
+        $this->assertEquals(false, $result);
+
+        sleep(config("lern.ratelimit")+2);
+
+        $result = $notifier->send(new Exception);
+        $this->assertEquals(true, $result);
+    }
 }

tests/RecorderTest.php

@@ -4,6 +4,7 @@
 
 use Tylercd100\LERN\Components\Recorder;
 use Tylercd100\LERN\Exceptions\RecorderFailedException;
+use Tylercd100\LERN\Exceptions\NotifierFailedException;
 use Exception;
 use Illuminate\Support\Facades\Input;
 
@@ -80,6 +81,13 @@ public function testRecordShouldReturnFalseWhenPassedRecorderFailedException()
         $this->assertEquals(false, $result);
     }
 
+    public function testRecordShouldReturnTrueWhenPassedNotifierFailedException()
+    {
+        $recorder = new Recorder;
+        $result = $recorder->record(new NotifierFailedException);
+        $this->assertInstanceOf(\Tylercd100\LERN\Models\ExceptionModel::class, $result);
+    }
+
     public function testGetDataFunction()
     {
         $data = ['user'=>['email','[email protected]','password'=>'foobar','name'=>'Foo Bar'],'status'=>200];
@@ -90,4 +98,19 @@ public function testGetDataFunction()
         $this->assertArrayNotHasKey('password', $result['user']);
         $this->assertArrayNotHasKey('email', $result['user']);
     }
+
+    public function testRateLimiting()
+    {
+        $recorder = new Recorder;
+        $result = $recorder->record(new Exception);
+        $this->assertInstanceOf(\Tylercd100\LERN\Models\ExceptionModel::class, $result);
+
+        $result = $recorder->record(new Exception);
+        $this->assertEquals(false, $result);
+
+        sleep(config("lern.ratelimit")+2);
+
+        $result = $recorder->record(new Exception);
+        $this->assertInstanceOf(\Tylercd100\LERN\Models\ExceptionModel::class, $result);
+    }
 }

tests/TestCase.php

@@ -5,6 +5,7 @@
 use Orchestra\Testbench\TestCase as Orchestra;
 use Tylercd100\LERN\Factories\MonologHandlerFactory;
 use Exception;
+use Illuminate\Support\Facades\Cache;
 
 class TestCase extends Orchestra
 {
@@ -30,6 +31,7 @@ public function setUp()
 
     public function tearDown()
     {
+        Cache::flush();
         parent::tearDown();
     }
 
@@ -75,6 +77,9 @@ protected function getPackageAliases($app)
      */
     protected function getEnvironmentSetUp($app)
     {
+        $app['config']->set('cache.default', 'file');
+        $app['config']->set('lern.ratelimit', 5);
+
         // Setup default database to use sqlite :memory:
         $app['config']->set('database.default', 'testbench');
         $app['config']->set('database.connections.testbench', [