From 38222af5e49bbc878dfbbc3ed75b58afa0b72eb0 Mon Sep 17 00:00:00 2001 From: Taylor Jasko Date: Wed, 24 Apr 2024 15:45:29 -0500 Subject: [PATCH] fix: updating `SafeToRetry()` function to retry on wrapped errors There is a rather unique bug when a single connection is being slammed with queries. When statement caching is enabled, and the `deallocateInvalidatedCachedStatements()` method is called to clean up, the `Pipeline.Sync()` method has the ability to error out due to the connection being in-use. `connLockError` already implements the inexplicit `SafeToRetry() bool()` interface, which is then checked by the `SafeToRetry()` function. In the event the connection is locked due to being in use, the `deallocateInvalidatedCachedStatements()` method can return a wrapped error that looks like: ``` failed to deallocate cached statement(s): conn busy ``` When the `stdlib` PGX wrapper is used, it is possible for this to not be retried, as it is also taking advantage of this `SafeToRetry()` function. In order to correct this and prevent future similar errors, the `SafeToRetry()` function has been updated to check against the wrapped errors as well. --- errors.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index 66d3558..8854177 100644 --- a/errors.go +++ b/errors.go @@ -12,8 +12,9 @@ import ( // SafeToRetry checks if the err is guaranteed to have occurred before sending any data to the server. func SafeToRetry(err error) bool { - if e, ok := err.(interface{ SafeToRetry() bool }); ok { - return e.SafeToRetry() + var retryableErr interface{ SafeToRetry() bool } + if errors.As(err, &retryableErr) { + return retryableErr.SafeToRetry() } return false }