From 69ddd71bbb620d8ab4ef57e0c81c095d831aa807 Mon Sep 17 00:00:00 2001
From: Dimitriy Kravchenko <dimitriy.kravchenko@spryker.com>
Date: Fri, 29 Mar 2024 11:11:48 +0200
Subject: [PATCH] Make error logs more informative

---
 .../QueryExecutor/AbstractQueryExecutor.php   | 12 ++++---
 .../AbstractQueryExecutorTest.php             | 31 ++++++++++++++-----
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/Propel/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutor.php b/src/Propel/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutor.php
index 29b88af6f9..e6a2323d87 100644
--- a/src/Propel/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutor.php
+++ b/src/Propel/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutor.php
@@ -111,7 +111,7 @@ protected function executeStatement(PreparedStatementDto $preparedStatementDto)
             }
             $stmt->execute();
         } catch (Throwable $e) {
-            $this->handleStatementException($e, $sql, $stmt ?: null);
+            $this->handleStatementException($e, $params, $sql, $stmt ?: null);
         }
 
         return $stmt;
@@ -121,6 +121,7 @@ protected function executeStatement(PreparedStatementDto $preparedStatementDto)
      * Logs an exception and adds the complete SQL statement to the exception.
      *
      * @param \Throwable $e The initial exception.
+     * @param array<mixed> $params Parameters for the SQL statement which triggered the exception.
      * @param string|null $sql The SQL statement which triggered the exception.
      * @param \Propel\Runtime\Connection\StatementInterface|\PDOStatement|null $stmt The prepared statement.
      *
@@ -128,7 +129,7 @@ protected function executeStatement(PreparedStatementDto $preparedStatementDto)
      *
      * @return void
      */
-    protected function handleStatementException(Throwable $e, ?string $sql, $stmt = null): void
+    protected function handleStatementException(Throwable $e, array $params, ?string $sql, $stmt = null): void
     {
         $internalMessage = $e->getMessage();
         Propel::log($internalMessage, Propel::LOG_ERR);
@@ -137,10 +138,11 @@ protected function handleStatementException(Throwable $e, ?string $sql, $stmt =
         if ($isDebugMode && $stmt instanceof StatementWrapper) {
             $sql = $stmt->getExecutedQueryString();
         }
-        $publicMessage = "Unable to execute statement [$sql]";
-        if ($isDebugMode) {
-            $publicMessage .= PHP_EOL . "Reason: [$internalMessage]";
+        $paramMessage = '';
+        foreach ($params as $parameter) {
+            $paramMessage .= 'column=`' . $parameter['column'] . '` value=`' . $parameter['value'] . '`, ';
         }
+        $publicMessage = "Unable to execute statement [$sql] with params [$paramMessage]. Reason: [$internalMessage]";
 
         throw new QueryExecutionException($publicMessage, 0, $e);
     }
diff --git a/tests/Propel/Tests/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutorTest.php b/tests/Propel/Tests/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutorTest.php
index 14d43ffc58..8f95f11375 100644
--- a/tests/Propel/Tests/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutorTest.php
+++ b/tests/Propel/Tests/Runtime/ActiveQuery/QueryExecutor/AbstractQueryExecutorTest.php
@@ -27,10 +27,26 @@ class AbstractQueryExecutorTest extends BookstoreTestBase
      */
     public function queryExceptionOutputFormatDataProvider()
     {
-        // [$useDebug, $sqlStatement, $internalErrorMessage, $expectedPublicMessage]
         return [
-            [false, '<SQL>', '<ERROR>', 'Unable to execute statement [<SQL>]'],
-            [true, '<SQL>', '<ERROR>', "Unable to execute statement [<SQL>]\nReason: [<ERROR>]"],
+            [
+                '$useDebug' => false,
+                '$sqlStatement' => '<SQL>',
+                '$params' => [
+                    ['column' => 'some_column', 'value' => 'text'],
+                ],
+                '$internalErrorMessage' => '<ERROR>',
+                '$expectedPublicMessage' => 'Unable to execute statement [<SQL>] with params [column=`some_column` value=`text`, ]. Reason: [<ERROR>]',
+            ],
+            [
+                '$useDebug' => true,
+                '$sqlStatement' => '<SQL>',
+                '$params' => [
+                    ['column' => 'some_column', 'value' => 'text'],
+                    ['column' => 'id', 'value' => 1],
+                ],
+                '$internalErrorMessage' => '<ERROR>',
+                '$expectedPublicMessage' => 'Unable to execute statement [<SQL>] with params [column=`some_column` value=`text`, column=`id` value=`1`, ]. Reason: [<ERROR>]',
+            ],
         ];
     }
 
@@ -39,26 +55,27 @@ public function queryExceptionOutputFormatDataProvider()
      *
      * @param bool $useDebug
      * @param string $sqlStatement
+     * @param array $params
      * @param string $internalErrorMessage
      * @param string $expectedPublicMessage
      *
      * @return void
      */
-    public function testQueryExceptionOutputFormat($useDebug, $sqlStatement, $internalErrorMessage, $expectedPublicMessage)
+    public function testQueryExceptionOutputFormat($useDebug, $sqlStatement, $params, $internalErrorMessage, $expectedPublicMessage)
     {
         $query = BookQuery::create();
         $con = new ConnectionWrapper($this->con->getWrappedConnection());
         $con->useDebug($useDebug);
 
         $c = new class ($query, $con) extends AbstractQueryExecutor {
-            public function simulateException($msg, $sql, $con)
+            public function simulateException($msg, $sql, $params, $con)
             {
-                return $this->handleStatementException(new PropelException($msg), $sql);
+                return $this->handleStatementException(new PropelException($msg), $params, $sql);
             }
         };
 
         try {
-            $c->simulateException($internalErrorMessage, $sqlStatement, $con);
+            $c->simulateException($internalErrorMessage, $sqlStatement, $params, $con);
             $this->fail('Cannot test without exception');
         } catch (QueryExecutionException $e) {
             $this->assertEquals($expectedPublicMessage, $e->getMessage());