Fix Scala-js fetch manual abort v4 (#2073)

softwaremill · Feb 8, 2024 · 3e5d3f6 · 3e5d3f6
1 parent aa37fbf
commit 3e5d3f6
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 24 deletions.
diff --git a/core/src/main/scalajs/sttp/client4/fetch/AbstractFetchBackend.scala b/core/src/main/scalajs/sttp/client4/fetch/AbstractFetchBackend.scala
@@ -84,20 +84,21 @@ abstract class AbstractFetchBackend[F[_], S <: Streams[S]](
   private def sendRegular[T](request: GenericRequest[T, R]): F[Response[T]] = {
     // https://stackoverflow.com/q/31061838/4094860
     val readTimeout = request.options.readTimeout
-    val (signal, cancelTimeout) = readTimeout match {
+    val controller = new AbortController()
+    val signal = controller.signal
+    val cancelTimeout = readTimeout match {
       case timeout: FiniteDuration =>
-        val controller = new AbortController()
-        val signal = controller.signal
-
         val timeoutHandle = setTimeout(timeout) {
           controller.abort()
         }
-        (Some(signal), () => clearTimeout(timeoutHandle))
+        () => clearTimeout(timeoutHandle)
 
       case _ =>
-        (None, () => ())
+        () => ()
     }
 
+    val cancel = () => controller.abort()
+
     val rheaders = new JSHeaders()
     request.headers.foreach { header =>
       // for multipart/form-data requests dom.FormData is responsible for setting the Content-Type header
@@ -113,7 +114,7 @@ abstract class AbstractFetchBackend[F[_], S <: Streams[S]](
     val req = createBody(request.body).map { rbody =>
       // use manual so we can return a specific error instead of the generic "TypeError: Failed to fetch"
       val rredirect = if (request.options.followRedirects) RequestRedirect.follow else RequestRedirect.manual
-      val rsignal = signal.orUndefined
+      val rsignal = signal
 
       val requestInitStatic = new RequestInit() {
         this.method = request.method.method.asInstanceOf[HttpMethod]
@@ -132,7 +133,7 @@ abstract class AbstractFetchBackend[F[_], S <: Streams[S]](
       }
 
       val requestInitDynamic = requestInitStatic.asInstanceOf[js.Dynamic]
-      signal.foreach(s => requestInitDynamic.updateDynamic("signal")(s))
+      requestInitDynamic.updateDynamic("signal")(signal)
       requestInitDynamic.updateDynamic("redirect")(rredirect) // named wrong in RequestInit
       val requestInit = requestInitDynamic.asInstanceOf[RequestInit]
 
@@ -165,10 +166,10 @@ abstract class AbstractFetchBackend[F[_], S <: Streams[S]](
           )
         }
       }
-    addCancelTimeoutHook(result, cancelTimeout)
+    addCancelTimeoutHook(result, cancel, cancelTimeout)
   }
 
-  protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit): F[T]
+  protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit, cleanup: () => Unit): F[T]
 
   private def convertResponseHeaders(headers: JSHeaders): Seq[Header] =
     headers

diff --git a/core/src/main/scalajs/sttp/client4/fetch/FetchBackend.scala b/core/src/main/scalajs/sttp/client4/fetch/FetchBackend.scala
@@ -16,8 +16,12 @@ class FetchBackend private (fetchOptions: FetchOptions, customizeRequest: FetchR
 
   override val streams: NoStreams = NoStreams
 
-  override protected def addCancelTimeoutHook[T](result: Future[T], cancel: () => Unit): Future[T] = {
-    result.onComplete(_ => cancel())
+  override protected def addCancelTimeoutHook[T](
+      result: Future[T],
+      cancel: () => Unit,
+      cleanup: () => Unit
+  ): Future[T] = {
+    result.onComplete(_ => cleanup())
     result
   }
 

diff --git a/effects/cats-ce2/src/main/scalajs/sttp/client4/impl/cats/FetchCatsBackend.scala b/effects/cats-ce2/src/main/scalajs/sttp/client4/impl/cats/FetchCatsBackend.scala
@@ -19,9 +19,10 @@ class FetchCatsBackend[F[_]: Concurrent: ContextShift] private (
 
   override val streams: NoStreams = NoStreams
 
-  override protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit): F[T] = {
+  override protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit, cleanup: () => Unit): F[T] = {
     val doCancel = Sync[F].delay(cancel())
-    result.guarantee(doCancel)
+    val doCleanup = Sync[F].delay(cleanup())
+    result.onCancel(doCancel).guarantee(doCleanup)
   }
 
   override protected def handleStreamBody(s: Nothing): F[js.UndefOr[BodyInit]] = s

diff --git a/effects/cats/src/main/scalajs/sttp/client4/impl/cats/FetchCatsBackend.scala b/effects/cats/src/main/scalajs/sttp/client4/impl/cats/FetchCatsBackend.scala
@@ -19,9 +19,10 @@ class FetchCatsBackend[F[_]: Async] private (
 
   override val streams: NoStreams = NoStreams
 
-  override protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit): F[T] = {
+  override protected def addCancelTimeoutHook[T](result: F[T], cancel: () => Unit, cleanup: () => Unit): F[T] = {
     val doCancel = Async[F].delay(cancel())
-    result.guarantee(doCancel)
+    val doCleanup = Async[F].delay(cleanup())
+    result.onCancel(doCancel).guarantee(doCleanup)
   }
 
   override protected def handleStreamBody(s: Nothing): F[js.UndefOr[BodyInit]] = s

diff --git a/effects/monix/src/main/scalajs/sttp/client4/impl/monix/FetchMonixBackend.scala b/effects/monix/src/main/scalajs/sttp/client4/impl/monix/FetchMonixBackend.scala
@@ -29,9 +29,10 @@ class FetchMonixBackend private (fetchOptions: FetchOptions, customizeRequest: F
 
   override val streams: MonixStreams = MonixStreams
 
-  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit): Task[T] = {
+  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit, cleanup: () => Unit): Task[T] = {
     val doCancel = Task.delay(cancel())
-    result.doOnCancel(doCancel).doOnFinish(_ => doCancel)
+    val doCleanup = Task.delay(cleanup())
+    result.doOnCancel(doCancel).doOnFinish(_ => doCleanup)
   }
 
   override protected def handleStreamBody(s: Observable[Array[Byte]]): Task[js.UndefOr[BodyInit]] = {

diff --git a/effects/zio/src/main/scalajs/sttp/client4/impl/zio/FetchZioBackend.scala b/effects/zio/src/main/scalajs/sttp/client4/impl/zio/FetchZioBackend.scala
@@ -32,9 +32,10 @@ class FetchZioBackend private (fetchOptions: FetchOptions, customizeRequest: Fet
 
   override val streams: ZioStreams = ZioStreams
 
-  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit): Task[T] = {
-    val doCancel = ZIO.attempt(cancel())
-    result.onInterrupt(doCancel.catchAll(_ => ZIO.unit)).tap(_ => doCancel)
+  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit, cleanup: () => Unit): Task[T] = {
+    val doCancel = ZIO.attempt(cancel()).ignore
+    val doCleanup = ZIO.attempt(cleanup()).ignore
+    result.onInterrupt(doCancel).onExit(_ => doCleanup)
   }
 
   override protected def handleStreamBody(s: Observable[Byte]): Task[js.UndefOr[BodyInit]] = {

diff --git a/effects/zio1/src/main/scalajs/sttp/client4/impl/zio/FetchZioBackend.scala b/effects/zio1/src/main/scalajs/sttp/client4/impl/zio/FetchZioBackend.scala
@@ -37,9 +37,10 @@ class FetchZioBackend private (fetchOptions: FetchOptions, customizeRequest: Fet
 
   override val streams: ZioStreams = ZioStreams
 
-  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit): Task[T] = {
-    val doCancel = ZIO.effect(cancel())
-    result.onInterrupt(doCancel.catchAll(_ => ZIO.unit)).tap(_ => doCancel)
+  override protected def addCancelTimeoutHook[T](result: Task[T], cancel: () => Unit, cleanup: () => Unit): Task[T] = {
+    val doCancel = ZIO.effect(cancel()).ignore
+    val doCleanup = ZIO.effect(cleanup()).ignore
+    result.onInterrupt(doCancel).onExit(_ => doCleanup)
   }
 
   override protected def handleStreamBody(s: Observable[Byte]): Task[js.UndefOr[BodyInit]] = {