From 1dbb863d948833081b9779d99c501ea40d126f35 Mon Sep 17 00:00:00 2001 From: Anton Komarev Date: Sun, 8 Feb 2026 16:18:12 +0300 Subject: [PATCH 1/3] Add structured exception handling for lock operations --- CLAUDE.md | 1 + src/Exception/AbstractLockException.php | 25 +++ src/Exception/LockAcquireException.php | 40 ++++ src/Exception/LockReleaseException.php | 37 ++++ src/Postgres/PostgresAdvisoryLocker.php | 154 +++++++++------- .../AbstractIntegrationTestCase.php | 6 + .../PostgresAdvisoryLockerExceptionTest.php | 172 ++++++++++++++++++ .../Postgres/PostgresAdvisoryLockerTest.php | 5 - 8 files changed, 372 insertions(+), 68 deletions(-) create mode 100644 src/Exception/AbstractLockException.php create mode 100644 src/Exception/LockAcquireException.php create mode 100644 src/Exception/LockReleaseException.php create mode 100644 test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 7416f41..28d54ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,6 +71,7 @@ Design decisions are documented in `adr/` — consult these before changing hash - PHP 8.1+ features: enums, readonly properties, named arguments, `match` expressions - PSR-4 autoloading: `src/` → `Cog\DbLocker\`, `test/` → `Cog\Test\DbLocker\` - All classes use `declare(strict_types=1)` +- **Abstract classes** must be prefixed with `Abstract` (e.g., `AbstractLockException`, `AbstractIntegrationTestCase`) - SQL comments with `humanReadableValue` appended to lock queries for debugging — these must be sanitized to prevent SQL comment injection (see `sanitizeSqlComment()`) ## Git Workflow diff --git a/src/Exception/AbstractLockException.php b/src/Exception/AbstractLockException.php new file mode 100644 index 0000000..0ad81ed --- /dev/null +++ b/src/Exception/AbstractLockException.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Cog\DbLocker\Exception; + +/** + * Base exception for all database lock-related errors. + * + * This exception is thrown only for exceptional situations (database errors, connection issues, etc.), + * NOT for normal lock contention. When a lock is unavailable due to competition from other processes, + * methods return `false` instead of throwing this exception. + */ +abstract class AbstractLockException extends \RuntimeException +{ +} diff --git a/src/Exception/LockAcquireException.php b/src/Exception/LockAcquireException.php new file mode 100644 index 0000000..4760322 --- /dev/null +++ b/src/Exception/LockAcquireException.php @@ -0,0 +1,40 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Cog\DbLocker\Exception; + +use Cog\DbLocker\Postgres\PostgresLockKey; + +/** + * Exception thrown when a lock cannot be acquired due to a database error. + * + * This exception is NOT thrown when a lock is simply unavailable due to competition + * from other processes. In that case, methods return `false` instead. + * + * This exception IS thrown for genuine errors such as: + * - Connection failures + * - Query execution errors (excluding lock_not_available SQLSTATE 55P03) + * - Transaction state errors + */ +final class LockAcquireException extends AbstractLockException +{ + public static function fromDatabaseError( + PostgresLockKey $key, + \Throwable $previous, + ): self { + return new self( + message: "Failed to acquire lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}", + previous: $previous, + ); + } +} diff --git a/src/Exception/LockReleaseException.php b/src/Exception/LockReleaseException.php new file mode 100644 index 0000000..caf584b --- /dev/null +++ b/src/Exception/LockReleaseException.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Cog\DbLocker\Exception; + +use Cog\DbLocker\Postgres\PostgresLockKey; + +/** + * Exception thrown when a lock cannot be released due to a database error. + * + * This indicates a genuine error condition such as: + * - Connection failures + * - Query execution errors + * - Unexpected database state + */ +final class LockReleaseException extends AbstractLockException +{ + public static function fromDatabaseError( + PostgresLockKey $key, + \Throwable $previous, + ): self { + return new self( + message: "Failed to release lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}", + previous: $previous, + ); + } +} diff --git a/src/Postgres/PostgresAdvisoryLocker.php b/src/Postgres/PostgresAdvisoryLocker.php index ca94117..475f231 100644 --- a/src/Postgres/PostgresAdvisoryLocker.php +++ b/src/Postgres/PostgresAdvisoryLocker.php @@ -14,6 +14,8 @@ namespace Cog\DbLocker\Postgres; use Cog\DbLocker\ConnectionAdapterInterface; +use Cog\DbLocker\Exception\LockAcquireException; +use Cog\DbLocker\Exception\LockReleaseException; use Cog\DbLocker\Postgres\Enum\PostgresLockAccessModeEnum; use Cog\DbLocker\Postgres\Enum\PostgresLockLevelEnum; use Cog\DbLocker\Postgres\LockHandle\SessionLevelLockHandle; @@ -26,6 +28,9 @@ final class PostgresAdvisoryLocker * Acquire a transaction-level advisory lock with configurable timeout and access mode. * * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. + * @return TransactionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition). + * @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention. + * @throws \LogicException If attempting to acquire outside of an active transaction. */ public function acquireTransactionLevelLock( ConnectionAdapterInterface $dbConnection, @@ -62,6 +67,8 @@ public function acquireTransactionLevelLock( * @param callable(SessionLevelLockHandle): TReturn $callback A callback that receives the lock handle. * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. * @return TReturn The return value of the callback. + * @throws LockAcquireException If a database error occurs during lock acquisition. NOT thrown for normal lock contention. + * @throws LockReleaseException If a database error occurs during lock release (only thrown if no other exception occurred during callback execution). * * @see acquireTransactionLevelLock() for preferred locking strategy. * @@ -117,6 +124,8 @@ public function withinSessionLevelLock( * lock handle's release() method. * * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. + * @return SessionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition). + * @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention. * * @see acquireTransactionLevelLock() for preferred locking strategy. * @see withinSessionLevelLock() for automatic session lock management. @@ -144,25 +153,32 @@ public function acquireSessionLevelLock( /** * Release session level advisory lock. + * + * @return bool True if the lock was successfully released, false if it was not held by this session. + * @throws LockReleaseException If a database error occurs during release. */ public function releaseSessionLevelLock( ConnectionAdapterInterface $dbConnection, PostgresLockKey $key, PostgresLockAccessModeEnum $accessMode = PostgresLockAccessModeEnum::Exclusive, ): bool { - $sql = match ($accessMode) { - PostgresLockAccessModeEnum::Exclusive - => 'SELECT PG_ADVISORY_UNLOCK(:class_id, :object_id);', + try { + $sql = match ($accessMode) { + PostgresLockAccessModeEnum::Exclusive + => 'SELECT PG_ADVISORY_UNLOCK(:class_id, :object_id);', - PostgresLockAccessModeEnum::Share - => 'SELECT PG_ADVISORY_UNLOCK_SHARED(:class_id, :object_id);', - }; - $sql .= " -- $key->humanReadableValue"; + PostgresLockAccessModeEnum::Share + => 'SELECT PG_ADVISORY_UNLOCK_SHARED(:class_id, :object_id);', + }; + $sql .= " -- $key->humanReadableValue"; - return $dbConnection->fetchColumn($sql, [ - 'class_id' => $key->classId, - 'object_id' => $key->objectId, - ]); + return $dbConnection->fetchColumn($sql, [ + 'class_id' => $key->classId, + 'object_id' => $key->objectId, + ]); + } catch (\Throwable $exception) { + throw LockReleaseException::fromDatabaseError($key, $exception); + } } /** @@ -209,25 +225,29 @@ private function tryAcquireLock( PostgresLockLevelEnum $level, PostgresLockAccessModeEnum $accessMode, ): bool { - $sql = match ([$level, $accessMode]) { - [PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Exclusive] - => 'SELECT PG_TRY_ADVISORY_LOCK(:class_id, :object_id);', + try { + $sql = match ([$level, $accessMode]) { + [PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Exclusive] + => 'SELECT PG_TRY_ADVISORY_LOCK(:class_id, :object_id);', - [PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Share] - => 'SELECT PG_TRY_ADVISORY_LOCK_SHARED(:class_id, :object_id);', + [PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Share] + => 'SELECT PG_TRY_ADVISORY_LOCK_SHARED(:class_id, :object_id);', - [PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Exclusive] - => 'SELECT PG_TRY_ADVISORY_XACT_LOCK(:class_id, :object_id);', + [PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Exclusive] + => 'SELECT PG_TRY_ADVISORY_XACT_LOCK(:class_id, :object_id);', - [PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Share] - => 'SELECT PG_TRY_ADVISORY_XACT_LOCK_SHARED(:class_id, :object_id);', - }; - $sql .= " -- $key->humanReadableValue"; + [PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Share] + => 'SELECT PG_TRY_ADVISORY_XACT_LOCK_SHARED(:class_id, :object_id);', + }; + $sql .= " -- $key->humanReadableValue"; - return $dbConnection->fetchColumn($sql, [ - 'class_id' => $key->classId, - 'object_id' => $key->objectId, - ]); + return $dbConnection->fetchColumn($sql, [ + 'class_id' => $key->classId, + 'object_id' => $key->objectId, + ]); + } catch (\Throwable $exception) { + throw LockAcquireException::fromDatabaseError($key, $exception); + } } private function acquireLockWithTimeout( @@ -276,32 +296,36 @@ private function acquireTransactionLockWithTimeout( PostgresLockKey $key, TimeoutDuration $timeoutDuration, ): bool { - $timeoutMs = $timeoutDuration->toMilliseconds(); - $dbConnection->execute("SET LOCAL lock_timeout = '$timeoutMs'"); + try { + $timeoutMs = $timeoutDuration->toMilliseconds(); + $dbConnection->execute("SET LOCAL lock_timeout = '$timeoutMs'"); - /** - * Use a savepoint so that a lock_timeout error does not abort the entire transaction. - * PostgreSQL handles same-name savepoints as a stack, so nested calls are safe. - */ - $dbConnection->execute('SAVEPOINT _lock_timeout_savepoint'); + /** + * Use a savepoint so that a lock_timeout error does not abort the entire transaction. + * PostgreSQL handles same-name savepoints as a stack, so nested calls are safe. + */ + $dbConnection->execute('SAVEPOINT _lock_timeout_savepoint'); - try { - $dbConnection->execute($sql, [ - 'class_id' => $key->classId, - 'object_id' => $key->objectId, - ]); + try { + $dbConnection->execute($sql, [ + 'class_id' => $key->classId, + 'object_id' => $key->objectId, + ]); - $dbConnection->execute('RELEASE SAVEPOINT _lock_timeout_savepoint'); + $dbConnection->execute('RELEASE SAVEPOINT _lock_timeout_savepoint'); - return true; - } catch (\Throwable $exception) { - if ($dbConnection->isLockNotAvailable($exception)) { - $dbConnection->execute('ROLLBACK TO SAVEPOINT _lock_timeout_savepoint'); + return true; + } catch (\Throwable $exception) { + if ($dbConnection->isLockNotAvailable($exception)) { + $dbConnection->execute('ROLLBACK TO SAVEPOINT _lock_timeout_savepoint'); - return false; - } + return false; + } - throw $exception; + throw $exception; + } + } catch (\Throwable $exception) { + throw LockAcquireException::fromDatabaseError($key, $exception); } } @@ -311,26 +335,30 @@ private function acquireSessionLockWithTimeout( PostgresLockKey $key, TimeoutDuration $timeoutDuration, ): bool { - $timeoutMs = $timeoutDuration->toMilliseconds(); - $originalLockTimeout = $dbConnection->fetchColumn('SHOW lock_timeout'); - $dbConnection->execute("SET lock_timeout = '$timeoutMs'"); - try { - $dbConnection->execute($sql, [ - 'class_id' => $key->classId, - 'object_id' => $key->objectId, - ]); + $timeoutMs = $timeoutDuration->toMilliseconds(); + $originalLockTimeout = $dbConnection->fetchColumn('SHOW lock_timeout'); + $dbConnection->execute("SET lock_timeout = '$timeoutMs'"); + + try { + $dbConnection->execute($sql, [ + 'class_id' => $key->classId, + 'object_id' => $key->objectId, + ]); + + return true; + } catch (\Throwable $exception) { + if ($dbConnection->isLockNotAvailable($exception)) { + return false; + } - return true; - } catch (\Throwable $exception) { - if ($dbConnection->isLockNotAvailable($exception)) { - return false; + throw $exception; } - - throw $exception; - } - finally { - $dbConnection->execute("SET lock_timeout = '$originalLockTimeout'"); + finally { + $dbConnection->execute("SET lock_timeout = '$originalLockTimeout'"); + } + } catch (\Throwable $exception) { + throw LockAcquireException::fromDatabaseError($key, $exception); } } } diff --git a/test/Integration/AbstractIntegrationTestCase.php b/test/Integration/AbstractIntegrationTestCase.php index 9f64ac3..c871cd9 100644 --- a/test/Integration/AbstractIntegrationTestCase.php +++ b/test/Integration/AbstractIntegrationTestCase.php @@ -16,6 +16,7 @@ use Cog\DbLocker\DbConnection\PdoConnectionAdapter; use Cog\DbLocker\ConnectionAdapterInterface; use Cog\DbLocker\Postgres\Enum\PostgresLockAccessModeEnum; +use Cog\DbLocker\Postgres\PostgresAdvisoryLocker; use Cog\DbLocker\Postgres\PostgresLockKey; use PDO; use PHPUnit\Framework\TestCase; @@ -52,6 +53,11 @@ protected function initConnectionAdapter(): ConnectionAdapterInterface return new PdoConnectionAdapter($this->initPostgresPdoConnection()); } + protected function initLocker(): PostgresAdvisoryLocker + { + return new PostgresAdvisoryLocker(); + } + protected function assertPgAdvisoryLockExistsInConnection( PDO $dbConnection, PostgresLockKey $postgresLockKey, diff --git a/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php b/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php new file mode 100644 index 0000000..07292bd --- /dev/null +++ b/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php @@ -0,0 +1,172 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Cog\Test\DbLocker\Integration\Postgres; + +use Cog\DbLocker\ConnectionAdapterInterface; +use Cog\DbLocker\Exception\LockAcquireException; +use Cog\DbLocker\Exception\LockReleaseException; +use Cog\DbLocker\Postgres\Enum\PostgresLockAccessModeEnum; +use Cog\DbLocker\Postgres\PostgresAdvisoryLocker; +use Cog\DbLocker\Postgres\PostgresLockKey; +use Cog\DbLocker\TimeoutDuration; +use Cog\Test\DbLocker\Integration\AbstractIntegrationTestCase; + +final class PostgresAdvisoryLockerExceptionTest extends AbstractIntegrationTestCase +{ + public function testItThrowsLockAcquireExceptionOnDatabaseErrorForTransactionLock(): void + { + // GIVEN: A mock connection adapter that throws a database error (not lock_not_available) + $locker = $this->initLocker(); + $lockKey = PostgresLockKey::create('test', 'exception-test'); + $connectionAdapter = $this->createMockConnectionWithDatabaseError(); + + // WHEN: Attempting to acquire a transaction-level lock + // THEN: LockAcquireException should be thrown wrapping the original exception + $this->expectException(LockAcquireException::class); + $this->expectExceptionMessage("Failed to acquire lock for key `[test:exception-test]`"); + + $locker->acquireTransactionLevelLock( + $connectionAdapter, + $lockKey, + TimeoutDuration::zero(), + ); + } + + public function testItThrowsLockAcquireExceptionOnDatabaseErrorForSessionLock(): void + { + // GIVEN: A mock connection adapter that throws a database error + $locker = $this->initLocker(); + $lockKey = PostgresLockKey::create('test', 'exception-test'); + $connectionAdapter = $this->createMockConnectionWithDatabaseError(); + + // WHEN: Attempting to acquire a session-level lock + // THEN: LockAcquireException should be thrown wrapping the original exception + $this->expectException(LockAcquireException::class); + $this->expectExceptionMessage("Failed to acquire lock for key `[test:exception-test]`"); + + $locker->acquireSessionLevelLock( + $connectionAdapter, + $lockKey, + TimeoutDuration::zero(), + ); + } + + public function testItReturnsWasAcquiredFalseForLockNotAvailableTimeout(): void + { + // GIVEN: Two database connections and a lock key + $locker = $this->initLocker(); + $connection1 = $this->initConnectionAdapter(); + $connection2 = $this->initConnectionAdapter(); + $lockKey = PostgresLockKey::create('test', 'timeout-test'); + + // GIVEN: First connection holds the lock + $connection1->execute('BEGIN'); + $handle1 = $locker->acquireTransactionLevelLock( + $connection1, + $lockKey, + TimeoutDuration::zero(), + ); + $this->assertTrue($handle1->wasAcquired); + + // WHEN: Second connection attempts to acquire the same lock with a short timeout + $connection2->execute('BEGIN'); + $handle2 = $locker->acquireTransactionLevelLock( + $connection2, + $lockKey, + TimeoutDuration::ofMilliseconds(100), + ); + + // THEN: Lock should not be acquired, but no exception should be thrown + $this->assertFalse($handle2->wasAcquired); + } + + public function testItThrowsLockReleaseExceptionOnDatabaseErrorForRelease(): void + { + // GIVEN: A mock connection adapter that throws a database error on fetchColumn + $locker = $this->initLocker(); + $lockKey = PostgresLockKey::create('test', 'release-exception-test'); + $connectionAdapter = $this->createMockConnectionWithDatabaseError(); + + // WHEN: Attempting to release a session-level lock + // THEN: LockReleaseException should be thrown wrapping the original exception + $this->expectException(LockReleaseException::class); + $this->expectExceptionMessage("Failed to release lock for key `[test:release-exception-test]`"); + + $locker->releaseSessionLevelLock( + $connectionAdapter, + $lockKey, + ); + } + + public function testItDistinguishesLockNotAvailableFromDatabaseErrors(): void + { + // GIVEN: Two connections where first holds the lock + $locker = $this->initLocker(); + $connection1 = $this->initConnectionAdapter(); + $connection2 = $this->initConnectionAdapter(); + $lockKey = PostgresLockKey::create('test', 'distinguish-test'); + + // GIVEN: First connection begins transaction and holds the lock + $connection1->execute('BEGIN'); + $handle1 = $locker->acquireTransactionLevelLock( + $connection1, + $lockKey, + TimeoutDuration::zero(), + ); + $this->assertTrue($handle1->wasAcquired); + + // WHEN: Second connection tries with immediate timeout (lock_not_available is expected) + $connection2->execute('BEGIN'); + $handle2 = $locker->acquireTransactionLevelLock( + $connection2, + $lockKey, + TimeoutDuration::zero(), + ); + + // THEN: Should return false, not throw exception (this is normal lock contention) + $this->assertFalse($handle2->wasAcquired); + } + + private function createMockConnectionWithDatabaseError(): ConnectionAdapterInterface + { + return new class implements ConnectionAdapterInterface { + public function fetchColumn(string $sql, array $params = []): mixed + { + // Simulate a database error (not lock_not_available) + $exception = new \PDOException('Connection lost'); + $exception->code = '08006'; + throw $exception; + } + + public function execute(string $sql, array $params = []): void + { + // Simulate a database error + $exception = new \PDOException('Connection lost'); + $exception->code = '08006'; + throw $exception; + } + + public function isTransactionActive(): bool + { + return true; + } + + public function isLockNotAvailable(\Throwable $exception): bool + { + // This mock returns false to ensure the exception is NOT treated as lock_not_available + return false; + } + }; + } +} diff --git a/test/Integration/Postgres/PostgresAdvisoryLockerTest.php b/test/Integration/Postgres/PostgresAdvisoryLockerTest.php index d12006c..58ab4ae 100644 --- a/test/Integration/Postgres/PostgresAdvisoryLockerTest.php +++ b/test/Integration/Postgres/PostgresAdvisoryLockerTest.php @@ -1096,9 +1096,4 @@ public function testItRestoresOriginalLockTimeoutAfterAcquisition(): void $lockTimeout = $statement->fetchColumn(0); $this->assertSame('10s', $lockTimeout); } - - private function initLocker(): PostgresAdvisoryLocker - { - return new PostgresAdvisoryLocker(); - } } From 8406defe05393a1931923b49380bf04a4e060c3e Mon Sep 17 00:00:00 2001 From: Anton Komarev Date: Sun, 8 Feb 2026 16:22:53 +0300 Subject: [PATCH 2/3] Separate @throws tags with blank line in DocBlocks --- CLAUDE.md | 1 + src/ConnectionAdapterInterface.php | 2 ++ src/Postgres/PostgresAdvisoryLocker.php | 4 ++++ 3 files changed, 7 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 28d54ff..7745590 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,6 +73,7 @@ Design decisions are documented in `adr/` — consult these before changing hash - All classes use `declare(strict_types=1)` - **Abstract classes** must be prefixed with `Abstract` (e.g., `AbstractLockException`, `AbstractIntegrationTestCase`) - SQL comments with `humanReadableValue` appended to lock queries for debugging — these must be sanitized to prevent SQL comment injection (see `sanitizeSqlComment()`) +- **DocBlock formatting**: Always separate `@throws` tags from other tags (like `@param`, `@return`) with a blank line for better readability ## Git Workflow diff --git a/src/ConnectionAdapterInterface.php b/src/ConnectionAdapterInterface.php index cbaaf77..0d025c2 100644 --- a/src/ConnectionAdapterInterface.php +++ b/src/ConnectionAdapterInterface.php @@ -29,6 +29,7 @@ interface ConnectionAdapterInterface * @param string $sql SQL query with named parameters (e.g., :class_id) * @param array $params Named parameters (e.g., ['class_id' => 1, 'object_id' => 2]) * @return mixed The value of the first column (typically bool or string) + * * @throws \Throwable Database-specific exception on error */ public function fetchColumn(string $sql, array $params = []): mixed; @@ -44,6 +45,7 @@ public function fetchColumn(string $sql, array $params = []): mixed; * * @param string $sql SQL statement with optional named parameters * @param array $params Named parameters (e.g., ['class_id' => 1]) + * * @throws \Throwable Database-specific exception on error */ public function execute(string $sql, array $params = []): void; diff --git a/src/Postgres/PostgresAdvisoryLocker.php b/src/Postgres/PostgresAdvisoryLocker.php index 475f231..dd4a1a4 100644 --- a/src/Postgres/PostgresAdvisoryLocker.php +++ b/src/Postgres/PostgresAdvisoryLocker.php @@ -29,6 +29,7 @@ final class PostgresAdvisoryLocker * * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. * @return TransactionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition). + * * @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention. * @throws \LogicException If attempting to acquire outside of an active transaction. */ @@ -67,6 +68,7 @@ public function acquireTransactionLevelLock( * @param callable(SessionLevelLockHandle): TReturn $callback A callback that receives the lock handle. * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. * @return TReturn The return value of the callback. + * * @throws LockAcquireException If a database error occurs during lock acquisition. NOT thrown for normal lock contention. * @throws LockReleaseException If a database error occurs during lock release (only thrown if no other exception occurred during callback execution). * @@ -125,6 +127,7 @@ public function withinSessionLevelLock( * * @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt. * @return SessionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition). + * * @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention. * * @see acquireTransactionLevelLock() for preferred locking strategy. @@ -155,6 +158,7 @@ public function acquireSessionLevelLock( * Release session level advisory lock. * * @return bool True if the lock was successfully released, false if it was not held by this session. + * * @throws LockReleaseException If a database error occurs during release. */ public function releaseSessionLevelLock( From 776ac4151b7f8812adb67a0cc0359fe60ded84c0 Mon Sep 17 00:00:00 2001 From: Anton Komarev Date: Sun, 8 Feb 2026 16:26:43 +0300 Subject: [PATCH 3/3] Replace \Throwable with \Exception in catch blocks and signatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit \Throwable catches both \Exception and \Error. Catching \Error (TypeError, OutOfMemoryError, etc.) and wrapping it in a domain \Exception is incorrect — these are programming errors, not recoverable database exceptions. \Throwable is preserved only in withinSessionLevelLock() where it guards the user callback and the finally block. --- src/ConnectionAdapterInterface.php | 8 ++++---- src/DbConnection/PdoConnectionAdapter.php | 2 +- src/Exception/LockAcquireException.php | 2 +- src/Exception/LockReleaseException.php | 2 +- src/Postgres/PostgresAdvisoryLocker.php | 12 ++++++------ .../Postgres/PostgresAdvisoryLockerExceptionTest.php | 10 +++------- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/ConnectionAdapterInterface.php b/src/ConnectionAdapterInterface.php index 0d025c2..6a5f014 100644 --- a/src/ConnectionAdapterInterface.php +++ b/src/ConnectionAdapterInterface.php @@ -30,7 +30,7 @@ interface ConnectionAdapterInterface * @param array $params Named parameters (e.g., ['class_id' => 1, 'object_id' => 2]) * @return mixed The value of the first column (typically bool or string) * - * @throws \Throwable Database-specific exception on error + * @throws \Exception Database-specific exception on error */ public function fetchColumn(string $sql, array $params = []): mixed; @@ -46,7 +46,7 @@ public function fetchColumn(string $sql, array $params = []): mixed; * @param string $sql SQL statement with optional named parameters * @param array $params Named parameters (e.g., ['class_id' => 1]) * - * @throws \Throwable Database-specific exception on error + * @throws \Exception Database-specific exception on error */ public function execute(string $sql, array $params = []): void; @@ -67,8 +67,8 @@ public function isTransactionActive(): bool; * - Doctrine DBAL: Doctrine\DBAL\Exception with getSQLState() method * - Cycle ORM: wraps \PDOException in its own exception * - * @param \Throwable $exception Exception to inspect + * @param \Exception $exception Exception to inspect * @return bool True if the exception indicates a lock timeout (SQLSTATE 55P03) */ - public function isLockNotAvailable(\Throwable $exception): bool; + public function isLockNotAvailable(\Exception $exception): bool; } diff --git a/src/DbConnection/PdoConnectionAdapter.php b/src/DbConnection/PdoConnectionAdapter.php index a777f63..96f78aa 100644 --- a/src/DbConnection/PdoConnectionAdapter.php +++ b/src/DbConnection/PdoConnectionAdapter.php @@ -56,7 +56,7 @@ public function isTransactionActive(): bool return $this->pdo->inTransaction(); } - public function isLockNotAvailable(\Throwable $exception): bool + public function isLockNotAvailable(\Exception $exception): bool { // PDOException: getCode() returns SQLSTATE string if ($exception instanceof \PDOException) { diff --git a/src/Exception/LockAcquireException.php b/src/Exception/LockAcquireException.php index 4760322..d2cea4e 100644 --- a/src/Exception/LockAcquireException.php +++ b/src/Exception/LockAcquireException.php @@ -30,7 +30,7 @@ final class LockAcquireException extends AbstractLockException { public static function fromDatabaseError( PostgresLockKey $key, - \Throwable $previous, + \Exception $previous, ): self { return new self( message: "Failed to acquire lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}", diff --git a/src/Exception/LockReleaseException.php b/src/Exception/LockReleaseException.php index caf584b..333b7a8 100644 --- a/src/Exception/LockReleaseException.php +++ b/src/Exception/LockReleaseException.php @@ -27,7 +27,7 @@ final class LockReleaseException extends AbstractLockException { public static function fromDatabaseError( PostgresLockKey $key, - \Throwable $previous, + \Exception $previous, ): self { return new self( message: "Failed to release lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}", diff --git a/src/Postgres/PostgresAdvisoryLocker.php b/src/Postgres/PostgresAdvisoryLocker.php index dd4a1a4..54bb017 100644 --- a/src/Postgres/PostgresAdvisoryLocker.php +++ b/src/Postgres/PostgresAdvisoryLocker.php @@ -180,7 +180,7 @@ public function releaseSessionLevelLock( 'class_id' => $key->classId, 'object_id' => $key->objectId, ]); - } catch (\Throwable $exception) { + } catch (\Exception $exception) { throw LockReleaseException::fromDatabaseError($key, $exception); } } @@ -249,7 +249,7 @@ private function tryAcquireLock( 'class_id' => $key->classId, 'object_id' => $key->objectId, ]); - } catch (\Throwable $exception) { + } catch (\Exception $exception) { throw LockAcquireException::fromDatabaseError($key, $exception); } } @@ -319,7 +319,7 @@ private function acquireTransactionLockWithTimeout( $dbConnection->execute('RELEASE SAVEPOINT _lock_timeout_savepoint'); return true; - } catch (\Throwable $exception) { + } catch (\Exception $exception) { if ($dbConnection->isLockNotAvailable($exception)) { $dbConnection->execute('ROLLBACK TO SAVEPOINT _lock_timeout_savepoint'); @@ -328,7 +328,7 @@ private function acquireTransactionLockWithTimeout( throw $exception; } - } catch (\Throwable $exception) { + } catch (\Exception $exception) { throw LockAcquireException::fromDatabaseError($key, $exception); } } @@ -351,7 +351,7 @@ private function acquireSessionLockWithTimeout( ]); return true; - } catch (\Throwable $exception) { + } catch (\Exception $exception) { if ($dbConnection->isLockNotAvailable($exception)) { return false; } @@ -361,7 +361,7 @@ private function acquireSessionLockWithTimeout( finally { $dbConnection->execute("SET lock_timeout = '$originalLockTimeout'"); } - } catch (\Throwable $exception) { + } catch (\Exception $exception) { throw LockAcquireException::fromDatabaseError($key, $exception); } } diff --git a/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php b/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php index 07292bd..763622c 100644 --- a/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php +++ b/test/Integration/Postgres/PostgresAdvisoryLockerExceptionTest.php @@ -144,17 +144,13 @@ private function createMockConnectionWithDatabaseError(): ConnectionAdapterInter public function fetchColumn(string $sql, array $params = []): mixed { // Simulate a database error (not lock_not_available) - $exception = new \PDOException('Connection lost'); - $exception->code = '08006'; - throw $exception; + throw new \PDOException('Connection lost'); } public function execute(string $sql, array $params = []): void { // Simulate a database error - $exception = new \PDOException('Connection lost'); - $exception->code = '08006'; - throw $exception; + throw new \PDOException('Connection lost'); } public function isTransactionActive(): bool @@ -162,7 +158,7 @@ public function isTransactionActive(): bool return true; } - public function isLockNotAvailable(\Throwable $exception): bool + public function isLockNotAvailable(\Exception $exception): bool { // This mock returns false to ensure the exception is NOT treated as lock_not_available return false;