Skip to content

Commit ea10c91

Browse files
antonkomarevantonkomarev
authored
Add structured exception handling for lock operations (#21)
1 parent 2a7530d commit ea10c91

10 files changed

Lines changed: 380 additions & 73 deletions

‎CLAUDE.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ Design decisions are documented in `adr/` — consult these before changing hash
7171
- PHP 8.1+ features: enums, readonly properties, named arguments, `match` expressions
7272
- PSR-4 autoloading: `src/` → `Cog\DbLocker\`, `test/` → `Cog\Test\DbLocker\`
7373
- All classes use `declare(strict_types=1)`
74+
- **Abstract classes** must be prefixed with `Abstract` (e.g., `AbstractLockException`, `AbstractIntegrationTestCase`)
7475
- SQL comments with `humanReadableValue` appended to lock queries for debugging — these must be sanitized to prevent SQL comment injection (see `sanitizeSqlComment()`)
76+
- **DocBlock formatting**: Always separate `@throws` tags from other tags (like `@param`, `@return`) with a blank line for better readability
7577

7678
## Git Workflow
7779

‎src/ConnectionAdapterInterface.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ interface ConnectionAdapterInterface
2929
* @param string $sql SQL query with named parameters (e.g., :class_id)
3030
* @param array<string, mixed> $params Named parameters (e.g., ['class_id' => 1, 'object_id' => 2])
3131
* @return mixed The value of the first column (typically bool or string)
32-
* @throws \Throwable Database-specific exception on error
32+
*
33+
* @throws \Exception Database-specific exception on error
3334
*/
3435
public function fetchColumn(string $sql, array $params = []): mixed;
3536

@@ -44,7 +45,8 @@ public function fetchColumn(string $sql, array $params = []): mixed;
4445
*
4546
* @param string $sql SQL statement with optional named parameters
4647
* @param array<string, mixed> $params Named parameters (e.g., ['class_id' => 1])
47-
* @throws \Throwable Database-specific exception on error
48+
*
49+
* @throws \Exception Database-specific exception on error
4850
*/
4951
public function execute(string $sql, array $params = []): void;
5052

@@ -65,8 +67,8 @@ public function isTransactionActive(): bool;
6567
* - Doctrine DBAL: Doctrine\DBAL\Exception with getSQLState() method
6668
* - Cycle ORM: wraps \PDOException in its own exception
6769
*
68-
* @param \Throwable $exception Exception to inspect
70+
* @param \Exception $exception Exception to inspect
6971
* @return bool True if the exception indicates a lock timeout (SQLSTATE 55P03)
7072
*/
71-
public function isLockNotAvailable(\Throwable $exception): bool;
73+
public function isLockNotAvailable(\Exception $exception): bool;
7274
}

‎src/DbConnection/PdoConnectionAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function isTransactionActive(): bool
5656
return $this->pdo->inTransaction();
5757
}
5858

59-
public function isLockNotAvailable(\Throwable $exception): bool
59+
public function isLockNotAvailable(\Exception $exception): bool
6060
{
6161
// PDOException: getCode() returns SQLSTATE string
6262
if ($exception instanceof \PDOException) {

‎src/Exception/AbstractLockException.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/*
4+
* This file is part of PHP DB Locker.
5+
*
6+
* (c) Anton Komarev <anton@komarev.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace Cog\DbLocker\Exception;
15+
16+
/**
17+
* Base exception for all database lock-related errors.
18+
*
19+
* This exception is thrown only for exceptional situations (database errors, connection issues, etc.),
20+
* NOT for normal lock contention. When a lock is unavailable due to competition from other processes,
21+
* methods return `false` instead of throwing this exception.
22+
*/
23+
abstract class AbstractLockException extends \RuntimeException
24+
{
25+
}

‎src/Exception/LockAcquireException.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/*
4+
* This file is part of PHP DB Locker.
5+
*
6+
* (c) Anton Komarev <anton@komarev.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace Cog\DbLocker\Exception;
15+
16+
use Cog\DbLocker\Postgres\PostgresLockKey;
17+
18+
/**
19+
* Exception thrown when a lock cannot be acquired due to a database error.
20+
*
21+
* This exception is NOT thrown when a lock is simply unavailable due to competition
22+
* from other processes. In that case, methods return `false` instead.
23+
*
24+
* This exception IS thrown for genuine errors such as:
25+
* - Connection failures
26+
* - Query execution errors (excluding lock_not_available SQLSTATE 55P03)
27+
* - Transaction state errors
28+
*/
29+
final class LockAcquireException extends AbstractLockException
30+
{
31+
public static function fromDatabaseError(
32+
PostgresLockKey $key,
33+
\Exception $previous,
34+
): self {
35+
return new self(
36+
message: "Failed to acquire lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}",
37+
previous: $previous,
38+
);
39+
}
40+
}

‎src/Exception/LockReleaseException.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
/*
4+
* This file is part of PHP DB Locker.
5+
*
6+
* (c) Anton Komarev <anton@komarev.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace Cog\DbLocker\Exception;
15+
16+
use Cog\DbLocker\Postgres\PostgresLockKey;
17+
18+
/**
19+
* Exception thrown when a lock cannot be released due to a database error.
20+
*
21+
* This indicates a genuine error condition such as:
22+
* - Connection failures
23+
* - Query execution errors
24+
* - Unexpected database state
25+
*/
26+
final class LockReleaseException extends AbstractLockException
27+
{
28+
public static function fromDatabaseError(
29+
PostgresLockKey $key,
30+
\Exception $previous,
31+
): self {
32+
return new self(
33+
message: "Failed to release lock for key `{$key->humanReadableValue}`: {$previous->getMessage()}",
34+
previous: $previous,
35+
);
36+
}
37+
}

‎src/Postgres/PostgresAdvisoryLocker.php‎

Lines changed: 95 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
namespace Cog\DbLocker\Postgres;
1515

1616
use Cog\DbLocker\ConnectionAdapterInterface;
17+
use Cog\DbLocker\Exception\LockAcquireException;
18+
use Cog\DbLocker\Exception\LockReleaseException;
1719
use Cog\DbLocker\Postgres\Enum\PostgresLockAccessModeEnum;
1820
use Cog\DbLocker\Postgres\Enum\PostgresLockLevelEnum;
1921
use Cog\DbLocker\Postgres\LockHandle\SessionLevelLockHandle;
@@ -26,6 +28,10 @@ final class PostgresAdvisoryLocker
2628
* Acquire a transaction-level advisory lock with configurable timeout and access mode.
2729
*
2830
* @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt.
31+
* @return TransactionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition).
32+
*
33+
* @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention.
34+
* @throws \LogicException If attempting to acquire outside of an active transaction.
2935
*/
3036
public function acquireTransactionLevelLock(
3137
ConnectionAdapterInterface $dbConnection,
@@ -63,6 +69,9 @@ public function acquireTransactionLevelLock(
6369
* @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt.
6470
* @return TReturn The return value of the callback.
6571
*
72+
* @throws LockAcquireException If a database error occurs during lock acquisition. NOT thrown for normal lock contention.
73+
* @throws LockReleaseException If a database error occurs during lock release (only thrown if no other exception occurred during callback execution).
74+
*
6675
* @see acquireTransactionLevelLock() for preferred locking strategy.
6776
*
6877
* @template TReturn
@@ -117,6 +126,9 @@ public function withinSessionLevelLock(
117126
* lock handle's release() method.
118127
*
119128
* @param TimeoutDuration $timeoutDuration Maximum wait time. Use TimeoutDuration::zero() for an immediate (non-blocking) attempt.
129+
* @return SessionLevelLockHandle Handle with wasAcquired=false if lock is held by another process (normal competition).
130+
*
131+
* @throws LockAcquireException If a database error occurs (connection failures, query errors, etc.). NOT thrown for normal lock contention.
120132
*
121133
* @see acquireTransactionLevelLock() for preferred locking strategy.
122134
* @see withinSessionLevelLock() for automatic session lock management.
@@ -144,25 +156,33 @@ public function acquireSessionLevelLock(
144156

145157
/**
146158
* Release session level advisory lock.
159+
*
160+
* @return bool True if the lock was successfully released, false if it was not held by this session.
161+
*
162+
* @throws LockReleaseException If a database error occurs during release.
147163
*/
148164
public function releaseSessionLevelLock(
149165
ConnectionAdapterInterface $dbConnection,
150166
PostgresLockKey $key,
151167
PostgresLockAccessModeEnum $accessMode = PostgresLockAccessModeEnum::Exclusive,
152168
): bool {
153-
$sql = match ($accessMode) {
154-
PostgresLockAccessModeEnum::Exclusive
155-
=> 'SELECT PG_ADVISORY_UNLOCK(:class_id, :object_id);',
169+
try {
170+
$sql = match ($accessMode) {
171+
PostgresLockAccessModeEnum::Exclusive
172+
=> 'SELECT PG_ADVISORY_UNLOCK(:class_id, :object_id);',
156173

157-
PostgresLockAccessModeEnum::Share
158-
=> 'SELECT PG_ADVISORY_UNLOCK_SHARED(:class_id, :object_id);',
159-
};
160-
$sql .= " -- $key->humanReadableValue";
174+
PostgresLockAccessModeEnum::Share
175+
=> 'SELECT PG_ADVISORY_UNLOCK_SHARED(:class_id, :object_id);',
176+
};
177+
$sql .= " -- $key->humanReadableValue";
161178

162-
return $dbConnection->fetchColumn($sql, [
163-
'class_id' => $key->classId,
164-
'object_id' => $key->objectId,
165-
]);
179+
return $dbConnection->fetchColumn($sql, [
180+
'class_id' => $key->classId,
181+
'object_id' => $key->objectId,
182+
]);
183+
} catch (\Exception $exception) {
184+
throw LockReleaseException::fromDatabaseError($key, $exception);
185+
}
166186
}
167187

168188
/**
@@ -209,25 +229,29 @@ private function tryAcquireLock(
209229
PostgresLockLevelEnum $level,
210230
PostgresLockAccessModeEnum $accessMode,
211231
): bool {
212-
$sql = match ([$level, $accessMode]) {
213-
[PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Exclusive]
214-
=> 'SELECT PG_TRY_ADVISORY_LOCK(:class_id, :object_id);',
232+
try {
233+
$sql = match ([$level, $accessMode]) {
234+
[PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Exclusive]
235+
=> 'SELECT PG_TRY_ADVISORY_LOCK(:class_id, :object_id);',
215236

216-
[PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Share]
217-
=> 'SELECT PG_TRY_ADVISORY_LOCK_SHARED(:class_id, :object_id);',
237+
[PostgresLockLevelEnum::Session, PostgresLockAccessModeEnum::Share]
238+
=> 'SELECT PG_TRY_ADVISORY_LOCK_SHARED(:class_id, :object_id);',
218239

219-
[PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Exclusive]
220-
=> 'SELECT PG_TRY_ADVISORY_XACT_LOCK(:class_id, :object_id);',
240+
[PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Exclusive]
241+
=> 'SELECT PG_TRY_ADVISORY_XACT_LOCK(:class_id, :object_id);',
221242

222-
[PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Share]
223-
=> 'SELECT PG_TRY_ADVISORY_XACT_LOCK_SHARED(:class_id, :object_id);',
224-
};
225-
$sql .= " -- $key->humanReadableValue";
243+
[PostgresLockLevelEnum::Transaction, PostgresLockAccessModeEnum::Share]
244+
=> 'SELECT PG_TRY_ADVISORY_XACT_LOCK_SHARED(:class_id, :object_id);',
245+
};
246+
$sql .= " -- $key->humanReadableValue";
226247

227-
return $dbConnection->fetchColumn($sql, [
228-
'class_id' => $key->classId,
229-
'object_id' => $key->objectId,
230-
]);
248+
return $dbConnection->fetchColumn($sql, [
249+
'class_id' => $key->classId,
250+
'object_id' => $key->objectId,
251+
]);
252+
} catch (\Exception $exception) {
253+
throw LockAcquireException::fromDatabaseError($key, $exception);
254+
}
231255
}
232256

233257
private function acquireLockWithTimeout(
@@ -276,32 +300,36 @@ private function acquireTransactionLockWithTimeout(
276300
PostgresLockKey $key,
277301
TimeoutDuration $timeoutDuration,
278302
): bool {
279-
$timeoutMs = $timeoutDuration->toMilliseconds();
280-
$dbConnection->execute("SET LOCAL lock_timeout = '$timeoutMs'");
303+
try {
304+
$timeoutMs = $timeoutDuration->toMilliseconds();
305+
$dbConnection->execute("SET LOCAL lock_timeout = '$timeoutMs'");
281306

282-
/**
283-
* Use a savepoint so that a lock_timeout error does not abort the entire transaction.
284-
* PostgreSQL handles same-name savepoints as a stack, so nested calls are safe.
285-
*/
286-
$dbConnection->execute('SAVEPOINT _lock_timeout_savepoint');
307+
/**
308+
* Use a savepoint so that a lock_timeout error does not abort the entire transaction.
309+
* PostgreSQL handles same-name savepoints as a stack, so nested calls are safe.
310+
*/
311+
$dbConnection->execute('SAVEPOINT _lock_timeout_savepoint');
287312

288-
try {
289-
$dbConnection->execute($sql, [
290-
'class_id' => $key->classId,
291-
'object_id' => $key->objectId,
292-
]);
313+
try {
314+
$dbConnection->execute($sql, [
315+
'class_id' => $key->classId,
316+
'object_id' => $key->objectId,
317+
]);
293318

294-
$dbConnection->execute('RELEASE SAVEPOINT _lock_timeout_savepoint');
319+
$dbConnection->execute('RELEASE SAVEPOINT _lock_timeout_savepoint');
295320

296-
return true;
297-
} catch (\Throwable $exception) {
298-
if ($dbConnection->isLockNotAvailable($exception)) {
299-
$dbConnection->execute('ROLLBACK TO SAVEPOINT _lock_timeout_savepoint');
321+
return true;
322+
} catch (\Exception $exception) {
323+
if ($dbConnection->isLockNotAvailable($exception)) {
324+
$dbConnection->execute('ROLLBACK TO SAVEPOINT _lock_timeout_savepoint');
300325

301-
return false;
302-
}
326+
return false;
327+
}
303328

304-
throw $exception;
329+
throw $exception;
330+
}
331+
} catch (\Exception $exception) {
332+
throw LockAcquireException::fromDatabaseError($key, $exception);
305333
}
306334
}
307335

@@ -311,26 +339,30 @@ private function acquireSessionLockWithTimeout(
311339
PostgresLockKey $key,
312340
TimeoutDuration $timeoutDuration,
313341
): bool {
314-
$timeoutMs = $timeoutDuration->toMilliseconds();
315-
$originalLockTimeout = $dbConnection->fetchColumn('SHOW lock_timeout');
316-
$dbConnection->execute("SET lock_timeout = '$timeoutMs'");
317-
318342
try {
319-
$dbConnection->execute($sql, [
320-
'class_id' => $key->classId,
321-
'object_id' => $key->objectId,
322-
]);
343+
$timeoutMs = $timeoutDuration->toMilliseconds();
344+
$originalLockTimeout = $dbConnection->fetchColumn('SHOW lock_timeout');
345+
$dbConnection->execute("SET lock_timeout = '$timeoutMs'");
346+
347+
try {
348+
$dbConnection->execute($sql, [
349+
'class_id' => $key->classId,
350+
'object_id' => $key->objectId,
351+
]);
352+
353+
return true;
354+
} catch (\Exception $exception) {
355+
if ($dbConnection->isLockNotAvailable($exception)) {
356+
return false;
357+
}
323358

324-
return true;
325-
} catch (\Throwable $exception) {
326-
if ($dbConnection->isLockNotAvailable($exception)) {
327-
return false;
359+
throw $exception;
328360
}
329-
330-
throw $exception;
331-
}
332-
finally {
333-
$dbConnection->execute("SET lock_timeout = '$originalLockTimeout'");
361+
finally {
362+
$dbConnection->execute("SET lock_timeout = '$originalLockTimeout'");
363+
}
364+
} catch (\Exception $exception) {
365+
throw LockAcquireException::fromDatabaseError($key, $exception);
334366
}
335367
}
336368
}

0 commit comments

Comments
 (0)