Merge pull request #17016 from QoboLtd/wrap-exception

Add optional support for wrapping PDOException with DatabaseException

src/Database/Exception/DatabaseException.php

@@ -23,6 +23,10 @@ use Cake\Core\Exception\CakeException;
  */
 class DatabaseException extends CakeException
 {
+    /**
+     * @inheritDoc
+     */
+    protected $_messageTemplate = '%s';
 }
 
 // phpcs:disable

src/Database/Log/LoggingStatement.php

@@ -16,6 +16,8 @@ declare(strict_types=1);
  */
 namespace Cake\Database\Log;
 
+use Cake\Core\Configure;
+use Cake\Database\Exception\DatabaseException;
 use Cake\Database\Statement\StatementDecorator;
 use Exception;
 use Psr\Log\LoggerInterface;
@@ -75,16 +77,31 @@ class LoggingStatement extends StatementDecorator
             $result = parent::execute($params);
             $this->loggedQuery->took = (int)round((microtime(true) - $this->startTime) * 1000, 0);
         } catch (Exception $e) {
+            $this->loggedQuery->error = $e;
+            $this->_log();
+
+            if (Configure::read('Error.convertStatementToDatabaseException', false) === true) {
+                $code = $e->getCode();
+                if (!is_int($code)) {
+                    $code = null;
+                }
+
+                throw new DatabaseException([
+                    'message' => $e->getMessage(),
+                    'queryString' => $this->queryString,
+                ], $code, $e);
+            }
+
             if (version_compare(PHP_VERSION, '8.2.0', '<')) {
                 deprecationWarning(
                     '4.4.12 - Having queryString set on exceptions is deprecated.' .
-                    'If you are not using this attribute there is no action to take.'
+                    'If you are not using this attribute there is no action to take.' .
+                    'Otherwise, enable Error.convertStatementToDatabaseException.'
                 );
                 /** @psalm-suppress UndefinedPropertyAssignment */
                 $e->queryString = $this->queryString;
             }
-            $this->loggedQuery->error = $e;
-            $this->_log();
+
             throw $e;
         }
 

tests/TestCase/Database/Log/LoggingStatementTest.php

@@ -16,7 +16,9 @@ declare(strict_types=1);
  */
 namespace Cake\Test\TestCase\Database\Log;
 
+use Cake\Core\Configure;
 use Cake\Database\DriverInterface;
+use Cake\Database\Exception\DatabaseException;
 use Cake\Database\Log\LoggingStatement;
 use Cake\Database\Log\QueryLogger;
 use Cake\Database\StatementInterface;
@@ -24,6 +26,7 @@ use Cake\Log\Log;
 use Cake\TestSuite\TestCase;
 use DateTime;
 use TestApp\Error\Exception\MyPDOException;
+use TestApp\Error\Exception\MyPDOStringException;
 
 /**
  * Tests LoggingStatement class
@@ -43,6 +46,7 @@ class LoggingStatementTest extends TestCase
     {
         parent::tearDown();
         Log::drop('queries');
+        Configure::delete('Error.convertStatementToDatabaseException');
     }
 
     /**
@@ -174,6 +178,82 @@ class LoggingStatementTest extends TestCase
     }
 
     /**
+     * Tests that we do exception wrapping correctly.
+     * The exception returns a string code like most PDOExceptions
+     */
+    public function testExecuteWithErrorWrapStatementStringCode(): void
+    {
+        Configure::write('Error.convertStatementToDatabaseException', true);
+        $exception = new MyPDOStringException('This is bad', 1234);
+        $inner = $this->getMockBuilder(StatementInterface::class)->getMock();
+        $inner->expects($this->once())
+            ->method('execute')
+            ->will($this->throwException($exception));
+
+        $driver = $this->getMockBuilder(DriverInterface::class)->getMock();
+        $st = $this->getMockBuilder(LoggingStatement::class)
+            ->onlyMethods(['__get'])
+            ->setConstructorArgs([$inner, $driver])
+            ->getMock();
+        $st->expects($this->any())
+            ->method('__get')
+            ->willReturn('SELECT bar FROM foo');
+        $st->setLogger(new QueryLogger(['connection' => 'test']));
+        try {
+            $st->execute();
+            $this->fail('Exception not thrown');
+        } catch (DatabaseException $e) {
+            $attrs = $e->getAttributes();
+
+            $this->assertSame('This is bad', $e->getMessage());
+            $this->assertArrayHasKey('queryString', $attrs);
+            $this->assertSame($st->queryString, $attrs['queryString']);
+        }
+
+        $messages = Log::engine('queries')->read();
+        $this->assertCount(1, $messages);
+        $this->assertMatchesRegularExpression("/^debug: connection=test duration=\d+ rows=0 SELECT bar FROM foo$/", $messages[0]);
+    }
+
+    /**
+     * Tests that we do exception wrapping correctly.
+     * The exception returns an int code.
+     */
+    public function testExecuteWithErrorWrapStatementIntCode(): void
+    {
+        Configure::write('Error.convertStatementToDatabaseException', true);
+        $exception = new MyPDOException('This is bad', 1234);
+        $inner = $this->getMockBuilder(StatementInterface::class)->getMock();
+        $inner->expects($this->once())
+            ->method('execute')
+            ->will($this->throwException($exception));
+
+        $driver = $this->getMockBuilder(DriverInterface::class)->getMock();
+        $st = $this->getMockBuilder(LoggingStatement::class)
+            ->onlyMethods(['__get'])
+            ->setConstructorArgs([$inner, $driver])
+            ->getMock();
+        $st->expects($this->any())
+            ->method('__get')
+            ->willReturn('SELECT bar FROM foo');
+        $st->setLogger(new QueryLogger(['connection' => 'test']));
+        try {
+            $st->execute();
+            $this->fail('Exception not thrown');
+        } catch (DatabaseException $e) {
+            $attrs = $e->getAttributes();
+
+            $this->assertSame('This is bad', $e->getMessage());
+            $this->assertArrayHasKey('queryString', $attrs);
+            $this->assertSame($st->queryString, $attrs['queryString']);
+        }
+
+        $messages = Log::engine('queries')->read();
+        $this->assertCount(1, $messages);
+        $this->assertMatchesRegularExpression("/^debug: connection=test duration=\d+ rows=0 SELECT bar FROM foo$/", $messages[0]);
+    }
+
+    /**
      * Tests setting and getting the logger
      */
     public function testSetAndGetLogger(): void

tests/test_app/TestApp/Error/Exception/MyPDOStringException.php

@@ -0,0 +1,27 @@
+<?php
+declare(strict_types=1);
+
+namespace TestApp\Error\Exception;
+
+use PDOException;
+
+/**
+ * Exception class for testing error page for PDOException.
+ * This also emulates typical PDOException instances that return
+ * strings for getCode().
+ */
+class MyPDOStringException extends PDOException
+{
+    public $queryString;
+
+    public $params;
+
+    /**
+     * @inheritDoc
+     */
+    public function __construct(string $message = '', int $code = 0, ?Throwable $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+        $this->code = 'DB' . strval($code);
+    }
+}