Skip to content

Commit faeb5bf

Browse files
committed
fix: surface count()/sum() timeouts and stop clearTimeout leaking on SQL adapters
- Mongo count() swallowed every client exception and returned 0, so a maxTimeMS expiry never surfaced; rethrow timeouts via processException. - Mongo sum() threw the raw client exception; route it through processException so timeouts map to the Timeout exception. - Remove count()'s dead early $options['limit'] assignment (the limit is applied via the $limit pipeline stage). - Adapter::clearTimeout() now resets the timeout property. Postgres applies the property on every statement (SET statement_timeout), so a cleared timeout kept cancelling all subsequent heavy statements on the connection. - Harden testCountTimeout: cleanup in finally, fixture read once, and the assertion moved out of the catch so a swallowed timeout fails clearly.
1 parent a7b99ed commit faeb5bf

3 files changed

Lines changed: 30 additions & 15 deletions

File tree

‎src/Database/Adapter.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,11 @@ public function clearTimeout(string $event): void
349349
{
350350
// Clear existing callback
351351
$this->before($event, 'timeout');
352+
353+
// Adapters that apply the timeout from this property on every statement
354+
// (e.g. Postgres SET statement_timeout) would otherwise keep enforcing a
355+
// cleared timeout on all subsequent queries.
356+
$this->timeout = 0;
352357
}
353358

354359
/**

‎src/Database/Adapter/Mongo.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,11 +2712,6 @@ public function count(Document $collection, array $queries = [], ?int $max = nul
27122712
$this->escapeQueryAttributes($collection, $queries);
27132713

27142714
$filters = [];
2715-
$options = [];
2716-
2717-
if (!\is_null($max) && $max > 0) {
2718-
$options['limit'] = $max;
2719-
}
27202715

27212716
// Build filters from queries
27222717
$filters = $this->buildFilters($queries);
@@ -2791,6 +2786,11 @@ public function count(Document $collection, array $queries = [], ?int $max = nul
27912786

27922787
return 0;
27932788
} catch (MongoException $e) {
2789+
$processed = $this->processException($e);
2790+
if ($processed instanceof TimeoutException) {
2791+
throw $processed;
2792+
}
2793+
27942794
return 0;
27952795
}
27962796
}
@@ -2854,7 +2854,11 @@ public function sum(Document $collection, string $attribute, array $queries = []
28542854
$options['maxTimeMS'] = $this->timeout;
28552855
}
28562856

2857-
return $this->client->aggregate($name, $pipeline, $options)->cursor->firstBatch[0]->total ?? 0;
2857+
try {
2858+
return $this->client->aggregate($name, $pipeline, $options)->cursor->firstBatch[0]->total ?? 0;
2859+
} catch (MongoException $e) {
2860+
throw $this->processException($e);
2861+
}
28582862
}
28592863

28602864
/**

‎tests/e2e/Adapter/Scopes/GeneralTests.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,10 @@ public function testCountTimeout(): void
109109
)
110110
);
111111

112+
$longtext = file_get_contents(__DIR__ . '/../../../resources/longtext.txt');
112113
for ($i = 0; $i < 20; $i++) {
113114
$database->createDocument('count-timeouts', new Document([
114-
'longtext' => file_get_contents(__DIR__ . '/../../../resources/longtext.txt'),
115+
'longtext' => $longtext,
115116
'$permissions' => [
116117
Permission::read(Role::any()),
117118
Permission::update(Role::any()),
@@ -120,17 +121,22 @@ public function testCountTimeout(): void
120121
]));
121122
}
122123

123-
$database->setTimeout(1);
124-
125124
try {
126-
$database->count('count-timeouts', [
127-
Query::notEqual('longtext', 'appwrite'),
128-
]);
129-
$this->fail('count() failed to throw a timeout exception');
130-
} catch (\Exception $e) {
125+
$database->setTimeout(1);
126+
127+
$thrown = null;
128+
try {
129+
$database->count('count-timeouts', [
130+
Query::notEqual('longtext', 'appwrite'),
131+
]);
132+
} catch (\Exception $e) {
133+
$thrown = $e;
134+
}
135+
136+
$this->assertInstanceOf(TimeoutException::class, $thrown, 'count() must throw a timeout exception');
137+
} finally {
131138
$database->clearTimeout();
132139
$database->deleteCollection('count-timeouts');
133-
$this->assertInstanceOf(TimeoutException::class, $e);
134140
}
135141
}
136142

0 commit comments

Comments
 (0)