Repository navigation
fix(firestore): retry reads on transient connection-closed errors - #294
Conversation
Coverage Report✅ Coverage 75.48% meets 40% threshold Total Coverage: 75.48% Package Breakdown
Minimum threshold: 40% |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements retries with exponential backoff on transient connection errors (http.ClientException) for Firestore query and aggregation reads, adding corresponding unit tests. The review feedback points out that the retry helper introduces an unnecessary delay on the first attempt and loses the original stack trace. Additionally, the query and aggregation readers should reset the retrieved transaction ID at the start of each attempt to ensure clean state isolation between retries.
9b41cd7 to
93e825b
Compare
f6c7f9a to
227c221
Compare
| results[entry.key] = value.doubleValue; | ||
| } else if (value.nullValue != null) { | ||
| results[entry.key] = null; | ||
| return retryOnConnectionError(() async { |
There was a problem hiding this comment.
https://pub.dev/packages/retry would rather use this Google package that actually does exponential backoff
There was a problem hiding this comment.
i'll look into it, there's already a pre-existing backoff implementation the retryOnConnectionError just wraps it, see
973e26e to
0cc084a
Compare
Fixes #291.
under high concurrency,
Query.get(),AggregateQuery.get(),Transaction.getQuery(), andTransaction.getAggregateQuery()could throwClientException: Connection closed before full header was received— a knowndart:io HttpClientkeep-alive race with no client-side fix except retrying. Added a sharedretryOnConnectionErrorhelper that retries with backoff when no partial results were received yet and the read isn't inside an active transaction.