Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GH-4553] Simplify error handling in the OCI8 driver #4599

Merged
merged 1 commit into from Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 0 additions & 37 deletions src/Driver/OCI8/Exception/PHPError.php

This file was deleted.

59 changes: 10 additions & 49 deletions src/Driver/OCI8/Result.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down
16 changes: 9 additions & 7 deletions tests/Functional/Driver/OCI8/ResultTest.php
Expand Up @@ -13,6 +13,9 @@
use function ini_get;
use function sprintf;

use const E_ALL;
use const E_WARNING;

/**
* @requires extension oci8
*/
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down