Merge pull request #4599 from morozov/issues/4553

[GH-4553] Simplify error handling in the OCI8 driver
doctrine · Apr 14, 2021 · fb9ba59 · fb9ba59
2 parents f2413c4 + 2d232c1
commit fb9ba59
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 93 deletions.
diff --git a/src/Driver/OCI8/Exception/PHPError.php b/src/Driver/OCI8/Exception/PHPError.php
diff --git a/src/Driver/OCI8/Result.php b/src/Driver/OCI8/Result.php
@@ -4,20 +4,18 @@
 
 namespace Doctrine\DBAL\Driver\OCI8;
 
+use Doctrine\DBAL\Driver\Exception;
 use Doctrine\DBAL\Driver\FetchUtils;
-use Doctrine\DBAL\Driver\OCI8\Exception\PHPError;
+use Doctrine\DBAL\Driver\OCI8\Exception\Error;
 use Doctrine\DBAL\Driver\Result as ResultInterface;
 
 use function oci_cancel;
+use function oci_error;
 use function oci_fetch_all;
 use function oci_fetch_array;
 use function oci_num_fields;
 use function oci_num_rows;
-use function preg_match_all;
-use function restore_error_handler;
-use function set_error_handler;
 
-use const E_WARNING;
 use const OCI_ASSOC;
 use const OCI_FETCHSTATEMENT_BY_COLUMN;
 use const OCI_FETCHSTATEMENT_BY_ROW;
@@ -117,55 +115,18 @@ public function free(): void
 
     /**
      * @return mixed|false
+     *
+     * @throws Exception
      */
     private function fetch(int $mode)
     {
-        set_error_handler(
-            static function (int $errno, string $errstr, string $errfile, int $errline): bool {
-                /*
-                 * Screen PHP error messages for any error that the OCI8 extension
-                 * doesn't report via oci_error()
-                 *
-                 * @link http://www.dba-oracle.com/t_error_code_list.htm
-                 */
-                if (preg_match_all('/ORA-(\d+)/', $errstr, $matches) === 0) {
-                    // If no ORA error codes are present in error message,
-                    // skip this error and bubble this error up to
-                    // the normal error handler
-                    return false;
-                }
-
-                foreach ($matches[1] as $oraErrorCode) {
-                    switch ($oraErrorCode) {
-                        case '04061':
-                        case '04065':
-                        case '04068':
-                            throw PHPError::new(
-                                'There was an error before all rows could be fetched.',
-                                $errno,
-                                $errstr,
-                                $errfile,
-                                $errline
-                            );
-                    }
-                }
-
-                // If ORA error codes in message do not match specified blocklist
-                // ORA error codes, skip this error and bubble this error up to
-                // the normal error handler
-                return false;
-            },
-            E_WARNING
-        );
+        $result = oci_fetch_array($this->statement, $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS);
 
-        try {
-            return oci_fetch_array(
-                $this->statement,
-                $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS
-            );
-        } finally {
-            restore_error_handler();
+        if ($result === false && oci_error($this->statement) !== false) {
+            throw Error::new($this->statement);
         }
+
+        return $result;
     }
 
     /**

diff --git a/tests/Functional/Driver/OCI8/ResultTest.php b/tests/Functional/Driver/OCI8/ResultTest.php
@@ -13,6 +13,9 @@
 use function ini_get;
 use function sprintf;
 
+use const E_ALL;
+use const E_WARNING;
+
 /**
  * @requires extension oci8
  */
@@ -65,12 +68,11 @@ public function testTruncatedFetch(
         bool $invalidateDataMidFetch
     ): void {
         if ($invalidateDataMidFetch) {
+            // prevent the PHPUnit error handler from handling the warnings that oci_*() functions may trigger
+            $this->iniSet('error_reporting', (string) (E_ALL & ~E_WARNING));
+
             $this->expectException(DriverException::class);
-            $this->expectErrorMessageMatches(
-                '/^An exception occurred in the driver: '
-                . 'There was an error before all rows could be fetched. '
-                . 'Error number: 2, Error String: oci_fetch_array.*/'
-            );
+            $this->expectExceptionCode(4068);
         }
 
         // Create a pipelined funtion that returns 10 rows more than the
@@ -92,15 +94,15 @@ public function testTruncatedFetch(
 
         // Access the first result to cause the first X rows to be prefetched
         // as defined by oci8.default_prefetch (often 100 rows)
-        $firstNumber = $result->fetchOne();
+        $result->fetchOne();
 
         if ($invalidateDataMidFetch) {
             // Invalidate the original dataset by changing the pipelined function
             // after the initial prefetch that caches locally the first X results
             $this->createOrReplacePipelinedFunction($expectedTotalRowCount + 10);
         }
 
-        while ($number = $result->fetchOne()) {
+        while ($result->fetchOne()) {
             // Attempt to access all remaining rows from the original fetch
             // The rows locally cached from the default prefetch will first be used
             // but when the result attempts to get the remaining 10 rows beyond