diff --git a/change/react-native-windows-42dec614-af90-4b29-b8c7-f266415ea240.json b/change/react-native-windows-42dec614-af90-4b29-b8c7-f266415ea240.json new file mode 100644 index 00000000000..5937254e3f5 --- /dev/null +++ b/change/react-native-windows-42dec614-af90-4b29-b8c7-f266415ea240.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Implement HTTP client timeout", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/vnext/Desktop.IntegrationTests/HttpOriginPolicyIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/HttpOriginPolicyIntegrationTest.cpp index 640383f71e6..40c4fde98da 100644 --- a/vnext/Desktop.IntegrationTests/HttpOriginPolicyIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/HttpOriginPolicyIntegrationTest.cpp @@ -147,7 +147,7 @@ TEST_CLASS(HttpOriginPolicyIntegrationTest) {}, /*data*/ "text", false, /*useIncrementalUpdates*/ - 1000, /*timeout*/ + 0, /*timeout*/ clientArgs.WithCredentials, /*withCredentials*/ [](int64_t){} /*reactCallback*/ ); @@ -201,7 +201,7 @@ TEST_CLASS(HttpOriginPolicyIntegrationTest) {}, /*data*/ "text", false, /*useIncrementalUpdates*/ - 1000, /*timeout*/ + 0, /*timeout*/ clientArgs.WithCredentials, /*withCredentials*/ [](int64_t) {} /*reactCallback*/ ); @@ -300,7 +300,7 @@ TEST_CLASS(HttpOriginPolicyIntegrationTest) //{} /*bodyData*/, "text", false /*useIncrementalUpdates*/, - 1000 /*timeout*/, + 0 /*timeout*/, false /*withCredentials*/, [](int64_t) {} /*callback*/ ); diff --git a/vnext/Desktop.IntegrationTests/HttpResourceIntegrationTests.cpp b/vnext/Desktop.IntegrationTests/HttpResourceIntegrationTests.cpp index 2d2e7dac901..8cafbade184 100644 --- a/vnext/Desktop.IntegrationTests/HttpResourceIntegrationTests.cpp +++ b/vnext/Desktop.IntegrationTests/HttpResourceIntegrationTests.cpp @@ -78,7 +78,7 @@ TEST_CLASS (HttpResourceIntegrationTest) { {}, /*data*/ "text", false, - 1000 /*timeout*/, + 0 /*timeout*/, false /*withCredentials*/, [](int64_t) {}); @@ -139,7 +139,7 @@ TEST_CLASS (HttpResourceIntegrationTest) { {}, /*data*/ "text", false, - 1000 /*timeout*/, + 0 /*timeout*/, false /*withCredentials*/, [](int64_t) {}); //clang-format on @@ -172,7 +172,7 @@ TEST_CLASS (HttpResourceIntegrationTest) { promise.set_value(); }); - resource->SendRequest("GET", "http://nonexistinghost", 0, {}, {}, "text", false, 1000, false, [](int64_t) {}); + resource->SendRequest("GET", "http://nonexistinghost", 0, {}, {}, "text", false, 0, false, [](int64_t) {}); promise.get_future().wait(); @@ -256,7 +256,7 @@ TEST_CLASS (HttpResourceIntegrationTest) { {}, /*data*/ "text", false, - 1000 /*timeout*/, + 0 /*timeout*/, false /*withCredentials*/, [](int64_t) {}); //clang-format on @@ -339,7 +339,7 @@ TEST_CLASS (HttpResourceIntegrationTest) { {}, /*data*/ "text", false, /*useIncrementalUpdates*/ - 1000 /*timeout*/, + 0 /*timeout*/, false /*withCredentials*/, [](int64_t) {}); //clang-format on @@ -354,6 +354,104 @@ TEST_CLASS (HttpResourceIntegrationTest) { Assert::AreEqual(static_cast(200), getResponse.StatusCode); Assert::AreEqual({"Redirect Content"}, content); } + + TEST_METHOD(TimeoutSucceeds) { + auto port = s_port; + string url = "http://localhost:" + std::to_string(port); + + promise getPromise; + string error; + int statusCode = 0; + + auto server = std::make_shared(s_port); + server->Callbacks().OnGet = [](const DynamicRequest &) -> ResponseWrapper { + DynamicResponse response; + response.result(http::status::ok); + + // Hold response to test client timeout + promise timer; + timer.get_future().wait_for(std::chrono::milliseconds(2000)); + + return {std::move(response)}; + }; + server->Start(); + + auto resource = IHttpResource::Make(); + resource->SetOnResponse([&getPromise, &statusCode](int64_t, IHttpResource::Response response) { + statusCode = static_cast(response.StatusCode); + getPromise.set_value(); + }); + resource->SetOnError([&getPromise, &error](int64_t, string &&errorMessage) { + error = std::move(errorMessage); + getPromise.set_value(); + }); + resource->SendRequest( + "GET", + std::move(url), + 0, /*requestId*/ + {}, /*headers*/ + {}, /*data*/ + "text", /*responseType*/ + false, /*useIncrementalUpdates*/ + 6000, /*timeout*/ + false, /*withCredentials*/ + [](int64_t) {} /*callback*/); + + getPromise.get_future().wait(); + server->Stop(); + + Assert::AreEqual({}, error); + Assert::AreEqual(200, statusCode); + } + + TEST_METHOD(TimeoutFails) { + auto port = s_port; + string url = "http://localhost:" + std::to_string(port); + + promise getPromise; + string error; + int statusCode = 0; + + auto server = std::make_shared(s_port); + server->Callbacks().OnGet = [](const DynamicRequest &) -> ResponseWrapper { + DynamicResponse response; + response.result(http::status::ok); + + // Hold response to test client timeout + promise timer; + timer.get_future().wait_for(std::chrono::milliseconds(4000)); + + return {std::move(response)}; + }; + server->Start(); + + auto resource = IHttpResource::Make(); + resource->SetOnResponse([&getPromise, &statusCode](int64_t, IHttpResource::Response response) { + statusCode = static_cast(response.StatusCode); + getPromise.set_value(); + }); + resource->SetOnError([&getPromise, &error](int64_t, string &&errorMessage) { + error = std::move(errorMessage); + getPromise.set_value(); + }); + resource->SendRequest( + "GET", + std::move(url), + 0, /*requestId*/ + {}, /*headers*/ + {}, /*data*/ + "text", /*responseType*/ + false, /*useIncrementalUpdates*/ + 2000, /*timeout*/ + false, /*withCredentials*/ + [](int64_t) {} /*callback*/); + + getPromise.get_future().wait(); + server->Stop(); + + Assert::AreEqual({"[0x800705b4] This operation returned because the timeout period expired."}, error); + Assert::AreEqual(0, statusCode); + } }; /*static*/ uint16_t HttpResourceIntegrationTest::s_port = 4444; diff --git a/vnext/Shared/Networking/IHttpResource.h b/vnext/Shared/Networking/IHttpResource.h index 7a8434e9f6f..173c79cda6e 100644 --- a/vnext/Shared/Networking/IHttpResource.h +++ b/vnext/Shared/Networking/IHttpResource.h @@ -71,6 +71,7 @@ struct IHttpResource { /// /// /// Request timeout in miliseconds. + /// Note: A value of 0 means no timeout. The resource will await the response indefinitely. /// /// /// Allow including credentials in request. diff --git a/vnext/Shared/Networking/WinRTHttpResource.cpp b/vnext/Shared/Networking/WinRTHttpResource.cpp index a496186d769..49b1d0e3875 100644 --- a/vnext/Shared/Networking/WinRTHttpResource.cpp +++ b/vnext/Shared/Networking/WinRTHttpResource.cpp @@ -297,7 +297,32 @@ fire_and_forget WinRTHttpResource::PerformSendRequest(HttpRequestMessage &&reque auto sendRequestOp = self->m_client.SendRequestAsync(coRequest); self->TrackResponse(coReqArgs->RequestId, sendRequestOp); - co_await lessthrow_await_adapter{sendRequestOp}; + if (coReqArgs->Timeout > 0) { + // See https://devblogs.microsoft.com/oldnewthing/20220415-00/?p=106486 + auto timedOut = std::make_shared(false); + auto sendRequestTimeout = [](auto timedOut, auto milliseconds) -> ResponseOperation { + // Convert milliseconds to "ticks" (10^-7 seconds) + co_await winrt::resume_after(winrt::Windows::Foundation::TimeSpan{milliseconds * 10000}); + *timedOut = true; + co_return nullptr; + }(timedOut, coReqArgs->Timeout); + + co_await lessthrow_await_adapter{winrt::when_any(sendRequestOp, sendRequestTimeout)}; + + // Cancel either still unfinished coroutine. + sendRequestTimeout.Cancel(); + sendRequestOp.Cancel(); + + if (*timedOut) { + if (self->m_onError) { + self->m_onError(coReqArgs->RequestId, Utilities::HResultToString(HRESULT_FROM_WIN32(ERROR_TIMEOUT))); + } + co_return self->UntrackResponse(coReqArgs->RequestId); + } + } else { + co_await lessthrow_await_adapter{sendRequestOp}; + } + auto result = sendRequestOp.ErrorCode(); if (result < 0) { if (self->m_onError) {