Merge pull request #468 from NDLANO/fix-500-errors

Fix some oembed-proxy 5xx errors
NDLANO · May 3, 2024 · 6b800ae · 6b800ae
2 parents 8a17f65 + ef3d3d1
commit 6b800ae
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 8 deletions.
diff --git a/network/src/main/scala/no/ndla/network/model/HttpRequestException.scala b/network/src/main/scala/no/ndla/network/model/HttpRequestException.scala
@@ -13,6 +13,7 @@ import sttp.client3.Response
 class HttpRequestException(message: String, val httpResponse: Option[Response[String]] = None)
     extends RuntimeException(message) {
   def is404: Boolean = httpResponse.exists(_.code.code == 404)
+  def is410: Boolean = httpResponse.exists(_.code.code == 410)
 }
 
 class AuthorizationException(message: String) extends RuntimeException(message)
diff --git a/oembed-proxy/src/main/scala/no/ndla/oembedproxy/controller/OEmbedProxyController.scala b/oembed-proxy/src/main/scala/no/ndla/oembedproxy/controller/OEmbedProxyController.scala
@@ -37,7 +37,7 @@ trait OEmbedProxyController {
         .in(query[String]("url").description("The URL to retrieve embedding information for"))
         .in(query[Option[String]]("maxwidth").description("The maximum width of the embedded resource"))
         .in(query[Option[String]]("maxheight").description("The maximum height of the embedded resource"))
-        .errorOut(errorOutputsFor(400, 404, 501, 502))
+        .errorOut(errorOutputsFor(400, 404, 410, 422, 502))
         .out(jsonBody[OEmbed])
         .serverLogicPure { case (url, maxWidth, maxHeight) =>
           oEmbedService.get(url, maxWidth, maxHeight) match {

diff --git a/oembed-proxy/src/main/scala/no/ndla/oembedproxy/model/NDLAErrors.scala b/oembed-proxy/src/main/scala/no/ndla/oembedproxy/model/NDLAErrors.scala
@@ -21,11 +21,15 @@ trait ErrorHelpers extends TapirErrorHelpers with StrictLogging {
 
   override def handleErrors: PartialFunction[Throwable, ErrorBody] = {
     case pnse: ProviderNotSupportedException =>
-      ErrorBody(PROVIDER_NOT_SUPPORTED, pnse.getMessage, clock.now(), 501)
+      ErrorBody(PROVIDER_NOT_SUPPORTED, pnse.getMessage, clock.now(), 422)
     case hre: HttpRequestException if hre.is404 =>
       val msg = hre.getMessage
       logger.info(s"Could not fetch remote: '$msg'")
       ErrorBody(REMOTE_ERROR, msg, clock.now(), 404)
+    case hre: HttpRequestException if hre.is410 =>
+      val msg = hre.getMessage
+      logger.info(s"Could not fetch remote: '$msg'")
+      ErrorBody(REMOTE_ERROR, msg, clock.now(), 410)
     case hre: HttpRequestException =>
       val msg = hre.httpResponse.map(response =>
         s": Received '${response.code}' '${response.statusText}'. Body was '${response.body}'"

diff --git a/oembed-proxy/src/main/scala/no/ndla/oembedproxy/service/OEmbedServiceComponent.scala b/oembed-proxy/src/main/scala/no/ndla/oembedproxy/service/OEmbedServiceComponent.scala
@@ -8,47 +8,69 @@
 
 package no.ndla.oembedproxy.service
 
+import com.typesafe.scalalogging.StrictLogging
 import no.ndla.network.NdlaClient
+import no.ndla.network.model.HttpRequestException
 import no.ndla.oembedproxy.model.{OEmbed, OEmbedProvider, ProviderNotSupportedException}
-import sttp.client3.quick._
+import sttp.client3.quick.*
 
+import scala.annotation.tailrec
 import scala.concurrent.duration.DurationInt
-import scala.util.{Failure, Try}
+import scala.util.{Failure, Success, Try}
 import scala.concurrent.duration.FiniteDuration
 
 trait OEmbedServiceComponent {
   this: NdlaClient with ProviderService =>
   val oEmbedService: OEmbedService
 
-  class OEmbedService(optionalProviders: Option[List[OEmbedProvider]] = None) {
+  class OEmbedService(optionalProviders: Option[List[OEmbedProvider]] = None) extends StrictLogging {
     val remoteTimeout: FiniteDuration = 10.seconds
 
     private lazy val providers = optionalProviders.toList.flatten ++ providerService
       .loadProviders()
     private def getProvider(url: String): Option[OEmbedProvider] =
       providers.find(_.supports(url))
 
+    private val MaxFetchOembedRetries: Int = 3
+    @tailrec
     private def fetchOembedFromProvider(
         provider: OEmbedProvider,
         url: String,
         maxWidth: Option[String],
-        maxHeight: Option[String]
+        maxHeight: Option[String],
+        retryCount: Int
     ): Try[OEmbed] = {
       val uri = uri"${provider.requestUrl(url, maxWidth, maxHeight)}"
       ndlaClient.fetch[OEmbed](
         quickRequest
           .get(uri)
           .followRedirects(true)
           .readTimeout(remoteTimeout)
-      )
+      ) match {
+        case Success(oembed)                   => Success(oembed)
+        case Failure(ex: HttpRequestException) => Failure(ex)
+        case Failure(ex) if retryCount < MaxFetchOembedRetries =>
+          logger.error(
+            s"Failed to fetch oembed from provider ${provider.providerName} for url $url. Retrying ${retryCount + 1}/$MaxFetchOembedRetries.",
+            ex
+          )
+          fetchOembedFromProvider(
+            provider,
+            url,
+            maxWidth,
+            maxHeight,
+            retryCount + 1
+          )
+        case Failure(ex) => Failure(ex)
+      }
     }
 
     def get(url: String, maxWidth: Option[String], maxHeight: Option[String]): Try[OEmbed] = {
       getProvider(url) match {
         case None =>
           Failure(ProviderNotSupportedException(s"Could not find an oembed-provider for the url '$url'"))
         case Some(provider) =>
-          fetchOembedFromProvider(provider, url, maxWidth, maxHeight).map(provider.postProcessor(url, _))
+          fetchOembedFromProvider(provider, url, maxWidth, maxHeight, 0).map(provider.postProcessor(url, _))
       }
     }