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

[zend-db] fix MySQLi adapter after changing default reporting mode by PHP 8.1 #156

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

partikus
Copy link
Contributor

@partikus partikus commented Dec 6, 2022

In PHP 8.1, the default error handling behavior of the MySQLi extension has changed from silencing errors to throw an Exception on errors.

MySQLi's default error mode is changed from MYSQLI_REPORT_OFF to MYSQLI_REPORT_ERROR|MYSQLI_REPORT_STRICT

https://php.watch/versions/8.1/mysqli-error-mode
https://www.php.net/manual/en/class.mysqli-sql-exception.php

There were 2 ways how to solve it:

  1. reverting MYSQLI_REPORT_OFF when no special config is given
    if (!isset($this->_config['report_mode']) || PHP_VERSION_ID >= 80100) {
        mysqli_report(
            isset($this->_config['report_mode'])
                ? $this->_config['report_mode']
                : MYSQLI_REPORT_OFF
        );
    }
  2. wrapping all possible places where an exception can be thrown with try/catch

I've chosen 2nd option taking inspiration from doctrine/dbal#4875

After adjusting MySQLi stuff we ended with:

There were 22 incomplete tests: ...
There were 591 skipped tests:  ...

No errors, failures for PHP 8.1.

@partikus partikus force-pushed the zend-db-php81-compat branch 2 times, most recently from 4599ade to 8c2b20d Compare December 6, 2022 14:34
@falkenhawk
Copy link
Member

Thanks @partikus ! Please adjust the PR title to reflect the issue what it is really addressing. Tests are not being adjusted at all in this PR.

@partikus partikus changed the title [zend-db] php 8.1 - tests adjustments [zend-db] adding support for php 8.1 after changing default error mode for MySQLi Dec 6, 2022
@partikus partikus force-pushed the zend-db-php81-compat branch 2 times, most recently from 73e48ed to de263a6 Compare December 6, 2022 17:05
@partikus partikus changed the title [zend-db] adding support for php 8.1 after changing default error mode for MySQLi [zend-db] fix MySQLi adapter after changing default reporting mode by PHP 8.1 Dec 6, 2022
@partikus
Copy link
Contributor Author

partikus commented Dec 6, 2022

@falkenhawk updated, thank you for your comments 👍

Copy link
Member

@falkenhawk falkenhawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks 👏

@partikus partikus merged commit 1661948 into zf1s:php81-compat Dec 6, 2022
@partikus partikus deleted the zend-db-php81-compat branch December 6, 2022 19:17
@@ -37,4 +37,16 @@
*/
class Zend_Db_Adapter_Mysqli_Exception extends Zend_Db_Adapter_Exception
{
public static function fromMysqliException($exception)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typing could be added: public static function fromMysqliException(mysqli_sql_exception $exception)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #158 by @partikus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants