Repository navigation
Preserve PDO SQLSTATE failures when wrapping legacy container exceptions #90
Description
Problem and impact
The public container-exception factories cannot wrap a valid Throwable whose code is a string. Both pass getCode() directly into PHP's native integer-only Exception constructor. PDO's real string SQLSTATE codes therefore trigger a secondary TypeError, losing the original diagnostic and its exception chain.
Classification: reproducible functional bug, P2; independent compatible 2.x patch candidate. This is not a security advisory and does not require waiting for the major.
Evidence and reproduction
Affected code at master da21efd8ccf4e7413f2d7c1d2539beff41a26734:
A real in-memory SQLite failure on PHP 8.5.11 produces the string code HY000:
$database = new PDO('sqlite::memory:');
$database->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
try {
$database->query('SELECT * FROM nonexistent_fixture');
} catch (PDOException $original) {
// Each call throws TypeError instead of returning its documented wrapper.
ContainerException::createFromThrowable($original);
NotFoundException::createFromThrowable($original);
}The two factory calls above must be exercised separately because the first already fails. Observed failure: Exception::__construct(): Argument #2 ($code) must be of type int, string given; its previous exception is not the original PDOException. The proof used no file database, application credentials or external service, and was independently reproduced during review.
The same defect is reachable through ordinary service construction. A parent PSR-11 container advertising CallableThrowableHandler can throw that PDOException while constructing its reporter. Resolving ErrorHandler::class through the library container produces the chain:
ContainerException -> ContainerException -> TypeError
The original PDOException, SQLSTATE and query diagnostic are absent from the entire chain. The outer error still implements the PSR container contract in this particular nested route; the bug is diagnostic replacement, not a claim that every route returns a raw TypeError.
Proposed focused correction
Preserve an integer code exactly when is_int($throwable->getCode()). Use an ordinary integer fallback, proposed 0, for string codes, while retaining the exact original Throwable as getPrevious(). Its original SQLSTATE remains available through that cause. Do not coerce a numeric-looking SQLSTATE into an unrelated integer meaning.
Apply this behavior consistently to both public static factories. Keep their signatures/classes, existing integer-code behavior, message, PSR-11 interfaces, parent delegation, alias resolution and cache behavior unchanged. Prefer a small direct correction over a new public utility class or a container redesign.
Passing the string through unchanged is incompatible with the native constructor. Mutating internal Exception state or dropping the original cause adds unnecessary risk. Removing the old container belongs to the already-reviewed major scope, rather than fixing a currently supported 2.x public API by deleting it.
Delivery and duplicate boundary
The completed SQLSTATE HTTP task fixes a different boundary: HTTP status-code mapping. It did not change either container-exception factory. Keep this patch separately reviewable.
The major Draft PR removes the affected legacy container classes. Do not port dead wrapper code into its new provider integration; reconcile the 2.x fix only while preserving that deliberate removal.
Acceptance criteria
- Add organized PHPUnit unit coverage for both factories, preserving representative integer codes, including zero/negative values, and handling string
HY000and numeric-looking SQLSTATE values without constructor TypeError. - Assert exact original identity through
getPrevious(), unchanged messages, integer wrapper fallback and retained original string code on the cause. - Cover a real library-container service-construction path and verify its full cause chain retains the original Throwable instead of replacing it with TypeError.
- Keep tests synthetic and local; the package must not gain a PDO/database runtime dependency solely to reproduce this edge case.
- Preserve PHP 7.4-compatible source and the declared dependency/runtime contract; supply actual minimum-runtime and current CI evidence. The audit proof on 8.5.11 does not certify a PHP 7.4 dependency graph.
- Update English PHPDoc and relevant container docs/examples, source-linked cookbook/site and
coisa-error-handlerconsumer guidance where the wrapping contract is described; include a patch Changeset. - Run relevant formatting, PHPCS, PHPUnit, maximum analysis and production-container checks, with the exact candidate SHA available for owner review. Do not merge, release or modify the major's removal policy automatically.
Parent: staged 2.x delivery roadmap. Major boundary: remaining modernization.
Activity