Fixed retry and duplicated replacements

modules/core/src/main/scala/org/scalasteward/core/application/Context.scala

@@ -95,7 +95,7 @@ object Context {
       userAgent <- Resource.eval(F.fromEither(`User-Agent`.parse(1)(userAgentString)))
       middleware = ClientConfiguration
         .setUserAgent[F](userAgent)
-        .andThen(ClientConfiguration.retryAfter[F](maxAttempts = 5))
+        .andThen(ClientConfiguration.withRetry[F](maxAttempts = 5))
       defaultClient <- ClientConfiguration.build(
         ClientConfiguration.BuilderMiddleware.default,
         middleware

modules/core/src/main/scala/org/scalasteward/core/client/ClientConfiguration.scala

@@ -22,11 +22,12 @@ import eu.timepit.refined.auto._
 import eu.timepit.refined.types.numeric.PosInt
 import java.net.http.HttpClient
 import java.net.http.HttpClient.Builder
-import org.http4s.Response
+import org.http4s.{Headers, Response}
 import org.http4s.client._
 import org.http4s.headers.`User-Agent`
 import org.http4s.jdkhttpclient.JdkHttpClient
 import org.typelevel.ci._
+import java.time.Instant
 import scala.concurrent.duration._
 
 object ClientConfiguration {
@@ -65,20 +66,38 @@ object ClientConfiguration {
 
   private val RetryAfterStatuses = Set(403, 429, 503)
 
+  implicit class FunctionExtender[T, U](f: T => Option[U]) {
+    def orElse(g: T => Option[U])(t: T): Option[U] = f(t).orElse(g(t))
+  }
+
+  private val retryAfter: Headers => Option[Int] =
+    _.get(ci"Retry-After").flatMap(_.head.value.toIntOption)
+
+  private val retryWaitMax60Seconds: Long => Int = {
+    case i if i > 0 && i < 60 => i.toInt
+    case i if i > 60          => 60
+    case _                    => 1
+  }
+
+  private val rateLimitReset: Headers => Option[Int] = _.get(ci"X-Ratelimit-Reset")
+    .flatMap(_.head.value.toLongOption)
+    .map(_ - Instant.now.getEpochSecond)
+    .map(retryWaitMax60Seconds)
+
+  private val retryInterval: Headers => Option[Int] = retryAfter.orElse(rateLimitReset)
+
   /** @param maxAttempts
     *   max number times the HTTP request should be sent useful to avoid unexpected cloud provider
     *   costs
     */
-  def retryAfter[F[_]: Temporal](maxAttempts: PosInt = 5): Middleware[F] = { client =>
+  def withRetry[F[_]: Temporal](maxAttempts: PosInt = 5): Middleware[F] = { client =>
     Client[F] { req =>
       def run(attempt: Int = 1): Resource[F, Response[F]] = client
         .run(req.putHeaders("X-Attempt" -> attempt.toString))
         .flatMap { response =>
           val maybeRetried = for {
-            header <- response.headers.get(ci"Retry-After")
-            seconds <- header.head.value.toIntOption
-            if seconds > 0
-            duration = seconds.seconds
+            interval <- retryInterval(response.headers)
+            duration = interval.seconds
             if RetryAfterStatuses.contains(response.status.code)
             if attempt < maxAttempts.value
           } yield Resource

modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala

@@ -5,24 +5,30 @@ import cats.implicits._
 import eu.timepit.refined.auto._
 import eu.timepit.refined.types.numeric.PosInt
 import munit.CatsEffectSuite
-import org.http4s.HttpRoutes
+import org.http4s._
 import org.http4s.client._
+import org.http4s.client.dsl.io._
+import org.http4s.dsl.io._
+import org.http4s.ember.server.EmberServerBuilder
 import org.http4s.headers.{`Retry-After`, `User-Agent`, Location}
 import org.http4s.implicits._
 import org.typelevel.ci._
 import org.typelevel.log4cats.LoggerFactory
 import org.typelevel.log4cats.slf4j.Slf4jFactory
 
+import java.time.Instant
 import scala.concurrent.duration._
+import scala.util.Try
 
 class ClientConfigurationTest extends CatsEffectSuite {
 
+  private val previousEpochSecond = Instant.now().minusSeconds(1).getEpochSecond
+  private val nextEpochSecond = Instant.now().plusSeconds(1).getEpochSecond
   private val userAgentValue = "my-user-agent"
   private val dummyUserAgent =
     `User-Agent`.parse(1)(userAgentValue).getOrElse(fail("unable to create user agent"))
 
-  private val routes: HttpRoutes[IO] = {
-    import org.http4s.dsl.io._
+  private val routes: HttpRoutes[IO] =
     HttpRoutes.of[IO] {
       case req @ GET -> Root / "user-agent" =>
         req.headers.get(ci"user-agent") match {
@@ -48,15 +54,22 @@ class ClientConfigurationTest extends CatsEffectSuite {
           case Some(attempt) if attempt >= 2 =>
             Ok()
           case _ =>
-            Forbidden().map(_.putHeaders(`Retry-After`.fromLong(1)))
+            val resetHeader =
+              ParseResult.success(Header.Raw(ci"X-Ratelimit-Reset", s"$nextEpochSecond"))
+            Forbidden().map(_.putHeaders(`Retry-After`.fromLong(1), resetHeader))
+        }
+      case req @ GET -> Root / "rate-limit-reset" / epochSecondsParam =>
+        req.headers.get(ci"X-Attempt").flatMap(_.head.value.toIntOption) match {
+          case Some(attempt) if attempt >= 2 => Ok()
+          case _ =>
+            val seconds = Try(epochSecondsParam.toLong).getOrElse(0L)
+            val resetHeader =
+              ParseResult.success(Header.Raw(ci"X-Ratelimit-Reset", seconds.toString))
+            Forbidden().map(_.putHeaders(resetHeader))
         }
     }
-  }
 
   test("setUserAgent add a specific user agent to requests") {
-    import org.http4s.Method._
-    import org.http4s.client.dsl.io._
-
     val initialClient = Client.fromHttpApp[IO](routes.orNotFound)
     val setUserAgent = ClientConfiguration.setUserAgent[IO](dummyUserAgent)
     val newClient = setUserAgent(initialClient)
@@ -106,26 +119,32 @@ class ClientConfigurationTest extends CatsEffectSuite {
     test.assertEquals((400, 302))
   }
 
-  test("retries on retry-after response header") {
-    import org.http4s.Method._
-    import org.http4s.client.dsl.io._
+  test("retries on retry-after response header even though 'X-Ratelimit-Reset' exists") {
+    val notEnoughRetries = request(uri"/retry-after", 1).assertEquals(403)
+    val exactlyEnoughRetries = request(uri"/retry-after", 2).assertEquals(200)
+    notEnoughRetries.flatMap(_ => exactlyEnoughRetries)
+  }
 
-    def clientWithMaxAttempts(maxAttempts: PosInt): Client[IO] = {
-      val initialClient = Client.fromHttpApp[IO](routes.orNotFound)
-      val retryAfter = ClientConfiguration.retryAfter[IO](maxAttempts)
-      retryAfter(initialClient)
-    }
+  test("retries with the value mentioned in 'X-Ratelimit-Reset' in the absense of 'Retry-After'") {
+    val uri = Uri.unsafeFromString(s"/rate-limit-reset/$nextEpochSecond")
+    val notEnoughRetries = request(uri, 1).assertEquals(403)
+    val exactlyEnoughRetries = request(uri, 2).assertEquals(200)
+    notEnoughRetries.flatMap(_ => exactlyEnoughRetries)
+  }
 
-    val notEnoughRetries = clientWithMaxAttempts(1)
-      .run(GET(uri"/retry-after"))
-      .use(r => r.status.code.pure[IO])
-      .assertEquals(403)
+  test("retries after 1 second when the given value in 'X-Ratelimit-Reset' elapsed") {
+    val uri: Uri = Uri.unsafeFromString(s"/rate-limit-reset/$previousEpochSecond")
+    val notEnoughRetries = request(uri, 1).assertEquals(403)
+    val exactlyEnoughRetries = request(uri, 2).assertEquals(200)
+    notEnoughRetries.flatMap(_ => exactlyEnoughRetries)
+  }
 
-    val exactlyEnoughRetries = clientWithMaxAttempts(2)
-      .run(GET(uri"/retry-after"))
-      .use(r => r.status.code.pure[IO])
-      .assertEquals(200)
+  private def request(uri: Uri, attempts: PosInt): IO[Int] =
+    clientWithMaxAttempts(attempts).run(GET(uri)).use(r => r.status.code.pure[IO])
 
-    notEnoughRetries.flatMap(_ => exactlyEnoughRetries)
+  private def clientWithMaxAttempts(maxAttempts: PosInt): Client[IO] = {
+    val initialClient = Client.fromHttpApp[IO](routes.orNotFound)
+    val withRetry = ClientConfiguration.withRetry[IO](maxAttempts)
+    withRetry(initialClient)
   }
 }