`PHPUnit\Framework\Exception` does not handle string error codes (`PDOException` with error code `'HY000'`, for example)

[ludeus]

Q	A
PHPUnit version	10.5.33
PHP version	8.3
Installation Method	Composer

Summary

I encountered an issue where a PDOException with the error code 'HY000' is caught and rethrown in phpunit/src/Framework/Constraint/Cardinality/Count.php:70, but PHPUnit throws an error because PHPUnit\Framework\Exception only accepts integer error codes.

How to reproduce

1. Use a test case that triggers a PDOException with the error code 'HY000'.
2. The exception is caught and rethrown in Count.php:70 as follows:

   try {
       $other = $other->getIterator();
   } catch (\Exception $e) {
       throw new Exception(
           $e->getMessage(),
           $e->getCode(), // This throws the error as it passes 'HY000' (a string) instead of an integer
           $e
       );
   }

3. Since PHPUnit\Framework\Exception expects an integer for the error code, it causes a type mismatch, resulting in an error.

[sebastianbergmann]

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

[ludeus]

Ok that was not easy because a normal Exception does not accept non integer code but with a new class :

<?php
class IteratorWithStringCodeException implements \IteratorAggregate
{
    public function getIterator(): Traversable
    {
        $pdo = new \PDO('sqlite::memory:');
        $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
        $pdo->query('select * from this_table_does_not_exists');
    }
}

and this test:

    public function testCountPdoException(): void
    {
        $iterator = new IteratorWithStringCodeException();
        $this->assertCount(0, $iterator);
    }

That will trigger this error:

TypeError: PHPUnit\Framework\Exception::__construct(): Argument #2 ($code) must be of type int, string given, called in myproject/vendor/phpunit/phpunit/src/Framework/Constraint/Cardinality/Count.php on line 73

[staabm]

@ludeus do you work on a fix?

[sebastianbergmann]

I think the getIterator() method in your example is missing a return statement. Are you sure this example is complete?

[ludeus]

I think the getIterator() method in your example is missing a return statement. Are you sure this example is complete?

The $pdo->query() will throw a PDOException()

[staabm]

sebastians point is, as soon as phpunit get fixed, your test will not get green.
(or produce different errors because of the missing return)

[ludeus]

    public function testCountWithPdoException()
    {
        $pdoException = new \PDOException();
        $reflection = new \ReflectionClass($pdoException);
        $property = $reflection->getProperty('code');
        $property->setValue($pdoException, 'HY000');
        $this->assertEquals('string', gettype($pdoException->getCode()));

        $iterator = new class($pdoException) implements \IteratorAggregate {
            public $exception;
            public function __construct($exception) { $this->exception = $exception; }
            public function getIterator(): \Traversable { throw $this->exception; }
        };
        $this->assertCount(0, $iterator);
    }

[sebastianbergmann]

This is not as simple as

diff --git a/src/Framework/Exception/Exception.php b/src/Framework/Exception/Exception.php
index 9a4d92880..44b860e17 100644
--- a/src/Framework/Exception/Exception.php
+++ b/src/Framework/Exception/Exception.php
@@ -45,7 +45,7 @@ class Exception extends RuntimeException implements \PHPUnit\Exception
      */
     protected array $serializableTrace;

-    public function __construct(string $message = '', int $code = 0, ?Throwable $previous = null)
+    public function __construct(string $message = '', int|string $code = 0, ?Throwable $previous = null)
     {
         parent::__construct($message, $code, $previous);

The above would only move the error somewhere else.

PHPUnit's exception classes extend PHP's exception classes such as RuntimeException. These exception classes do not support non-integer codes: https://3v4l.org/k7Gb3

To me, the fact that PDOException uses/supports string codes is an inconsistency in PHP that I do not know how to deal with.

[ludeus]

I doubt that PDO will ever fix its error codes (unless forced to), there's nothing we can do about it, but if we can avoid breaking tests by simply adding an integer cast to the getCode(), I think it's worth it.

[sebastianbergmann]

Would this make sense?

diff --git a/src/Framework/Exception/Exception.php b/src/Framework/Exception/Exception.php
index 58c661a93..778f26c3d 100644
--- a/src/Framework/Exception/Exception.php
+++ b/src/Framework/Exception/Exception.php
@@ -11,6 +11,8 @@

 use function array_keys;
 use function get_object_vars;
+use function is_int;
+use function sprintf;
 use RuntimeException;
 use Throwable;

@@ -42,8 +44,20 @@ class Exception extends RuntimeException implements \PHPUnit\Exception
 {
     protected array $serializableTrace;

-    public function __construct(string $message = '', int $code = 0, ?Throwable $previous = null)
+    public function __construct(string $message = '', int|string $code = 0, ?Throwable $previous = null)
     {
+        /**
+         * @see https://github.com/sebastianbergmann/phpunit/issues/5965
+         */
+        if (!is_int($code)) {
+            $message .= sprintf(
+                ' (exception code: %s)',
+                $code,
+            );
+
+            $code = (int) $code;
+        }
+
         parent::__construct($message, $code, $previous);

         $this->serializableTrace = $this->getTrace();

[staabm]

instead of $code = (int) $code; I think I would use a more explicit $code = 0;.
this way we have 2 clear camps. int and string code. not a third: numeric-string.

otherwise LGTM

[ludeus]

It makes sense to me, although personally, I wouldn't have touched the exception class but rather at the level of the 'throw exception'.
Like that:

try {
    $other = $other->getIterator();
} catch (\Exception $e) {
    $message = $e->getMessage();
    $code = $e->getCode();
    if (!is_int($code)) {
        $message .= sprintf(
            ' (exception code: %s)',
            $code,
        );
        $code = (int) $code;
    }
    throw new Exception(
        $message,
        $code,
        $e
    );
}