Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: inspect servant errors to shortcut retries #653

Merged
merged 3 commits into from
Jun 3, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 30 additions & 37 deletions cachix/src/Cachix/Client/Retry.hs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module Cachix.Client.Retry
( retryAll,
retryAllWithPolicy,
retryAllWithLogging,
retryHttp,
retryHttpWith,
endlessRetryPolicy,
Expand All @@ -11,10 +10,7 @@ where

import Cachix.Client.Exception (CachixException (..))
import qualified Control.Concurrent.Async as Async
import Control.Exception.Safe
( Handler (..),
isSyncException,
)
import Control.Exception.Safe (Handler (..))
import Control.Monad.Catch (MonadCatch, MonadMask, handleJust, throwM)
import Control.Retry
import Data.List (lookup)
Expand All @@ -32,6 +28,8 @@ import qualified Network.HTTP.Client as HTTP
import Network.HTTP.Types.Header (hRetryAfter)
import qualified Network.HTTP.Types.Status as HTTP
import Protolude hiding (Handler (..), handleJust)
import Servant.Client (ClientError (..))
import qualified Servant.Client as Servant
import qualified Text.ParserCombinators.ReadPrec as ReadPrec (lift)

defaultRetryPolicy :: RetryPolicy
Expand Down Expand Up @@ -65,25 +63,6 @@ retryAllWithPolicy policy f =
-- Retry everything else
allHandler _ = Handler $ \(_ :: SomeException) -> return True

-- Catches all exceptions except async exceptions with logging support
retryAllWithLogging ::
(MonadIO m, MonadMask m) =>
RetryPolicyM m ->
(Bool -> SomeException -> RetryStatus -> m ()) ->
m a ->
m a
retryAllWithLogging policy logger f =
recovering policy handlers $
const (rethrowLinkedThreadExceptions f)
where
handlers = skipAsyncExceptions ++ [exitCodeHandler, loggingHandler]

-- Skip over exitSuccess/exitFailure
exitCodeHandler _ = Handler $ \(_ :: ExitCode) -> return False

-- Log and retry everything else
loggingHandler = logRetries (return . isSyncException) logger

-- | Unwrap 'Async.ExceptionInLinkedThread' exceptions and rethrow the inner exception.
rethrowLinkedThreadExceptions :: (MonadCatch m) => m a -> m a
rethrowLinkedThreadExceptions =
Expand All @@ -105,20 +84,27 @@ retryHttpWith policy = recoveringDynamic policy handlers . const
where
handlers :: [RetryStatus -> Handler m RetryAction]
handlers =
skipAsyncExceptions' ++ [retryHttpExceptions, retrySyncExceptions]
skipAsyncExceptions' ++ [retryHttpExceptions, retryClientExceptions]

skipAsyncExceptions' = map (fmap toRetryAction .) skipAsyncExceptions

retryHttpExceptions _ = Handler httpExceptionToRetryAction
retrySyncExceptions _ = Handler $ \(_ :: SomeException) -> return ConsultPolicy
retryHttpExceptions _ = Handler httpExceptionToRetryHandler
retryClientExceptions _ = Handler clientExceptionToRetryHandler

httpExceptionToRetryAction :: HTTP.HttpException -> m RetryAction
httpExceptionToRetryAction (HTTP.HttpExceptionRequest _ (HTTP.StatusCodeException response _))
httpExceptionToRetryHandler :: HTTP.HttpException -> m RetryAction
httpExceptionToRetryHandler (HTTP.HttpExceptionRequest _ (HTTP.StatusCodeException response _))
| statusMayHaveRetryHeader (HTTP.responseStatus response) = overrideDelayWithRetryAfter response
httpExceptionToRetryAction ex = return . toRetryAction . shouldRetryHttpException $ ex

statusMayHaveRetryHeader :: HTTP.Status -> Bool
statusMayHaveRetryHeader = flip elem [HTTP.tooManyRequests429, HTTP.serviceUnavailable503]
httpExceptionToRetryHandler ex = return . toRetryAction . shouldRetryHttpException $ ex

clientExceptionToRetryHandler :: ClientError -> m RetryAction
clientExceptionToRetryHandler (FailureResponse _req res)
| shouldRetryHttpStatusCode (Servant.responseStatusCode res) =
return ConsultPolicy
clientExceptionToRetryHandler (ConnectionError ex) =
case fromException ex of
Just httpException -> httpExceptionToRetryHandler httpException
Nothing -> return DontRetry
clientExceptionToRetryHandler _ = return DontRetry

data RetryAfter
= RetryAfterDate UTCTime
Expand Down Expand Up @@ -171,9 +157,16 @@ shouldRetryHttpException (HTTP.HttpExceptionRequest _ reason) =
| HTTP.statusIsServerError status -> True
HTTP.ResponseBodyTooShort _ _ -> True
HTTP.ResponseTimeout -> True
HTTP.StatusCodeException response _
| HTTP.responseStatus response == HTTP.tooManyRequests429 -> True
HTTP.StatusCodeException response _
| HTTP.statusIsServerError (HTTP.responseStatus response) -> True
HTTP.StatusCodeException response _ ->
shouldRetryHttpStatusCode (HTTP.responseStatus response)
HTTP.HttpZlibException _ -> True
_ -> False

-- | Determine whether the HTTP status code is worth retrying.
shouldRetryHttpStatusCode :: HTTP.Status -> Bool
shouldRetryHttpStatusCode code | code == HTTP.tooManyRequests429 = True
shouldRetryHttpStatusCode code | HTTP.statusIsServerError code = True
shouldRetryHttpStatusCode _ = False

statusMayHaveRetryHeader :: HTTP.Status -> Bool
statusMayHaveRetryHeader = flip elem [HTTP.tooManyRequests429, HTTP.serviceUnavailable503]