Merge pull request #760 from cakephp/concurrency-errors

Allow saving requests to fail gracefully

Author: markstory

src/Model/Table/RequestsTable.php

@@ -16,8 +16,10 @@
 
 use Cake\Core\Configure;
 use Cake\Database\Driver\Sqlite;
+use Cake\Log\Log;
 use Cake\ORM\Query;
 use Cake\ORM\Table;
+use PDOException;
 
 /**
  * The requests table tracks basic information about each request.
@@ -100,30 +102,36 @@ public function gc()
         if (!$this->shouldGc()) {
             return;
         }
-        $noPurge = $this->find()
-            ->select(['id'])
-            ->enableHydration(false)
-            ->order(['requested_at' => 'desc'])
-            ->limit(Configure::read('DebugKit.requestCount') ?: 20)
-            ->extract('id')
-            ->toArray();
 
-        $query = $this->Panels->query()
-            ->delete()
-            ->where(['request_id NOT IN' => $noPurge]);
-        $statement = $query->execute();
-        $statement->closeCursor();
+        try {
+            $noPurge = $this->find()
+                ->select(['id'])
+                ->enableHydration(false)
+                ->order(['requested_at' => 'desc'])
+                ->limit(Configure::read('DebugKit.requestCount') ?: 20)
+                ->extract('id')
+                ->toArray();
 
-        $query = $this->query()
-            ->delete()
-            ->where(['id NOT IN' => $noPurge]);
+            $query = $this->Panels->query()
+                ->delete()
+                ->where(['request_id NOT IN' => $noPurge]);
+            $statement = $query->execute();
+            $statement->closeCursor();
 
-        $statement = $query->execute();
-        $statement->closeCursor();
+            $query = $this->query()
+                ->delete()
+                ->where(['id NOT IN' => $noPurge]);
 
-        $conn = $this->getConnection();
-        if ($conn->getDriver() instanceof Sqlite) {
-            $conn->execute('VACUUM;');
+            $statement = $query->execute();
+            $statement->closeCursor();
+
+            $conn = $this->getConnection();
+            if ($conn->getDriver() instanceof Sqlite) {
+                $conn->execute('VACUUM;');
+            }
+        } catch (PDOException $e) {
+            Log::warning('Unable to garbage collect requests table. This is probably due to concurrent requests.');
+            Log::warning((string)$e);
         }
     }
 }

src/ToolbarService.php

@@ -22,6 +22,7 @@
 use Cake\ORM\TableRegistry;
 use Cake\Routing\Router;
 use DebugKit\Panel\PanelRegistry;
+use PDOException;
 use Psr\Http\Message\ResponseInterface;
 
 /**
@@ -207,7 +208,7 @@ public function initializePanels()
      *
      * @param \Cake\Http\ServerRequest $request The request
      * @param \Psr\Http\Message\ResponseInterface $response The response
-     * @return null|\DebugKit\Model\Entity\Request Saved request data.
+     * @return false|\DebugKit\Model\Entity\Request Saved request data.
      */
     public function saveData(ServerRequest $request, ResponseInterface $response)
     {
@@ -218,7 +219,7 @@ public function saveData(ServerRequest $request, ResponseInterface $response)
             strpos($path, 'debug-kit') !== false ||
             $request->is('requested')
         ) {
-            return null;
+            return false;
         }
         $data = [
             'url' => $request->getUri()->getPath(),
@@ -253,7 +254,14 @@ public function saveData(ServerRequest $request, ResponseInterface $response)
             ]);
         }
 
-        return $requests->save($row);
+        try {
+            return $requests->save($row);
+        } catch (PDOException $e) {
+            Log::warning('Unable to save request. This is probably due to concurrent requests.');
+            Log::warning((string)$e);
+        }
+
+        return false;
     }
 
     /**

tests/TestCase/ToolbarServiceTest.php

@@ -141,7 +141,7 @@ public function testSaveDataIgnoreDebugKit()
         ]);
 
         $bar = new ToolbarService($this->events, []);
-        $this->assertNull($bar->saveData($request, $response));
+        $this->assertFalse($bar->saveData($request, $response));
     }
 
     /**
@@ -162,7 +162,7 @@ public function testSaveDataIgnoreDebugKitDashedUrl()
         ]);
 
         $bar = new ToolbarService($this->events, []);
-        $this->assertNull($bar->saveData($request, $response));
+        $this->assertFalse($bar->saveData($request, $response));
     }
 
     /**