Skip to content

Commit

Permalink
Fix database infinite loop (#66)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
tylercd100 authored Jul 3, 2018
1 parent 9b83ff4 commit 105c9d3
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 13 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_**
Expand Down Expand Up @@ -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.
7 changes: 7 additions & 0 deletions config/lern.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'=>[
/**
Expand Down
2 changes: 1 addition & 1 deletion phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnFailure="false">
stopOnFailure="true">
<php>
<env name="APP_KEY" value="8bff3a6e61f548c0ecd9ad52e3cf3738"/>
</php>
Expand Down
27 changes: 22 additions & 5 deletions src/Components/Component.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace Tylercd100\LERN\Components;

use Exception;
use Illuminate\Support\Facades\Cache;
use Carbon\Carbon;

abstract class Component {

Expand All @@ -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.
Expand All @@ -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);
}
}
21 changes: 16 additions & 5 deletions src/Components/Notifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions src/Components/Recorder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -17,6 +19,13 @@ class Recorder extends Component {
*/
protected $config = [];

/**
* @var array
*/
protected $absolutelyDontHandle = [
\Tylercd100\LERN\Exceptions\RecorderFailedException::class,
];

/**
* The constructor
*/
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions tests/LERNTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
26 changes: 26 additions & 0 deletions tests/NotifierTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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);
}
}
23 changes: 23 additions & 0 deletions tests/RecorderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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','mail@test.com','password'=>'foobar','name'=>'Foo Bar'],'status'=>200];
Expand All @@ -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);
}
}
5 changes: 5 additions & 0 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -30,6 +31,7 @@ public function setUp()

public function tearDown()
{
Cache::flush();
parent::tearDown();
}

Expand Down Expand Up @@ -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', [
Expand Down

0 comments on commit 105c9d3

Please sign in to comment.