From 8f047eabba5d309d20b186e0d520003854d40463 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Tue, 14 Feb 2023 19:26:28 +0000 Subject: [PATCH 1/4] Add broken tests for canceling in-flight requests --- .../org/http4s/dom/FetchServiceWorkerSuite.scala | 11 +++++++++++ .../scala/org/http4s/dom/NodeJSFetchSuite.scala | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/testsBrowser/src/test/scala/org/http4s/dom/FetchServiceWorkerSuite.scala b/testsBrowser/src/test/scala/org/http4s/dom/FetchServiceWorkerSuite.scala index 0173ca0c..3057edcd 100644 --- a/testsBrowser/src/test/scala/org/http4s/dom/FetchServiceWorkerSuite.scala +++ b/testsBrowser/src/test/scala/org/http4s/dom/FetchServiceWorkerSuite.scala @@ -110,6 +110,17 @@ class FetchServiceWorkerSuite extends CatsEffectSuite { } } + test("Cancel an in-flight request") { + client + .expect[String](GET(baseUrl / "delayed")) + .timeoutTo(100.millis, IO.unit) + .timed + .flatMap { + case (duration, _) => + IO(assert(clue(duration) < 500.millis)) + } + } + GetRoutes.getPaths.toList.foreach { case (path, expected) => test(s"Execute GET $path") { diff --git a/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala b/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala index bdea5225..cb19ebfa 100644 --- a/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala +++ b/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala @@ -22,6 +22,20 @@ import cats.effect.Resource import org.http4s.client.Client import org.http4s.client.testkit.ClientRouteTestBattery +import scala.concurrent.duration._ + class NodeJSFetchSuite extends ClientRouteTestBattery("FetchClient") { def clientResource: Resource[IO, Client[IO]] = FetchClientBuilder[IO].resource + + test("Cancel an in-flight request") { + val address = server().addresses.head + client() + .expect[String](s"http://$address/delayed") + .timeoutTo(100.millis, IO.unit) + .timed + .flatMap { + case (duration, _) => + IO(assert(clue(duration) < 500.millis)) + } + } } From d3b3455fe2f21c5b39a887810a04abb43144f4d5 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Tue, 14 Feb 2023 19:42:06 +0000 Subject: [PATCH 2/4] Tidying --- dom/src/main/scala/org/http4s/dom/FetchClient.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dom/src/main/scala/org/http4s/dom/FetchClient.scala b/dom/src/main/scala/org/http4s/dom/FetchClient.scala index f887613c..3e195a81 100644 --- a/dom/src/main/scala/org/http4s/dom/FetchClient.scala +++ b/dom/src/main/scala/org/http4s/dom/FetchClient.scala @@ -17,7 +17,6 @@ package org.http4s package dom -import cats.data.OptionT import cats.effect.Async import cats.effect.Poll import cats.effect.Resource @@ -115,7 +114,7 @@ private[dom] object FetchClient { } } { case (r, exitCase) => - OptionT.fromOption(Option(r.body)).foreachF(cancelReadableStream(_, exitCase)) + Option(r.body).traverse_(cancelReadableStream(_, exitCase)) } .evalMap(fromDomResponse[F]) From e66aa7d1ed8c6b6ad236c88d1894db9477d2a910 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Tue, 14 Feb 2023 20:16:42 +0000 Subject: [PATCH 3/4] Fix timeout handling --- .../main/scala/org/http4s/dom/FetchClient.scala | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/dom/src/main/scala/org/http4s/dom/FetchClient.scala b/dom/src/main/scala/org/http4s/dom/FetchClient.scala index 3e195a81..9f74143f 100644 --- a/dom/src/main/scala/org/http4s/dom/FetchClient.scala +++ b/dom/src/main/scala/org/http4s/dom/FetchClient.scala @@ -98,19 +98,15 @@ private[dom] object FetchClient { mergedOptions.referrerPolicy.foreach(init.referrerPolicy = _) val fetch = - poll(F.fromPromise(F.delay(Fetch.fetch(req.uri.renderString, init)))) + F.fromPromise(F.delay(Fetch.fetch(req.uri.renderString, init))) .onCancel(F.delay(abortController.abort())) - - requestTimeout match { - case d: FiniteDuration => - fetch.timeoutTo( - d, + .timeoutTo( + requestTimeout, F.raiseError[FetchResponse](new TimeoutException( - s"Request to ${req.uri.renderString} timed out after ${d.toMillis} ms")) + s"Request to ${req.uri.renderString} timed out after ${requestTimeout.toMillis} ms")) ) - case _ => - fetch - } + + poll(fetch) } } { case (r, exitCase) => From c53044854447d6401f9d035efdb7f40cbe503b7a Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Tue, 14 Feb 2023 20:23:57 +0000 Subject: [PATCH 4/4] Add streaming request test to Node.js suite --- .../test/scala/org/http4s/dom/NodeJSFetchSuite.scala | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala b/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala index cb19ebfa..e095df62 100644 --- a/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala +++ b/testsNodeJS/src/test/scala/org/http4s/dom/NodeJSFetchSuite.scala @@ -19,6 +19,8 @@ package dom import cats.effect.IO import cats.effect.Resource +import fs2.Stream +import org.http4s.Method._ import org.http4s.client.Client import org.http4s.client.testkit.ClientRouteTestBattery @@ -27,6 +29,16 @@ import scala.concurrent.duration._ class NodeJSFetchSuite extends ClientRouteTestBattery("FetchClient") { def clientResource: Resource[IO, Client[IO]] = FetchClientBuilder[IO].resource + test("POST a chunked body with streaming requests") { + val address = server().addresses.head + val baseUrl = Uri.fromString(s"http://$address/").toOption.get + FetchClientBuilder[IO] + .withStreamingRequests + .create + .expect[String](POST(Stream("This is chunked.").covary[IO], baseUrl / "echo")) + .assertEquals("This is chunked.") + } + test("Cancel an in-flight request") { val address = server().addresses.head client()