Implement the ability to set a timeout on rest requests.

I've tested this using CL 251703099, which is a work-in-progress CL to add timeouts to the Functions C++ SDK. I assume I should also add some tests under firebase/app/client/cpp/rest/, but I wanted to get feedback on this API before spending any more time on it.

It seems I don't have permission to run the Guitar presubmits. :/

PiperOrigin-RevId: 252854599

Author: Googler

app/instance_id/instance_id_desktop_impl.cc

@@ -51,8 +51,8 @@ class SignalSemaphoreResponse : public rest::Response {
     complete_->Post();
   }
 
-  void MarkCanceled() override {
-    rest::Response::MarkCompleted();
+  void MarkFailed() override {
+    rest::Response::MarkFailed();
     complete_->Post();
   }
 

app/rest/request.h

@@ -81,8 +81,8 @@ class Request : public Transfer {
   // Mark the transfer completed.
   void MarkCompleted() override { completed_ = true; }
 
-  // Mark the transfer canceled.
-  void MarkCanceled() override { completed_ = false; }
+  // Mark the transfer failed, usually from cancellation or timeout.
+  void MarkFailed() override { completed_ = false; }
 
   bool completed() const { return completed_; }
 

app/rest/request_options.h

@@ -27,8 +27,11 @@ namespace rest {
 // transport_interface.h. The actual HTTP transporter could be either library
 // Curl or a test mock.
 struct RequestOptions {
-  RequestOptions() :
-      method("GET"), stream_post_fields(false), verbose(false) {}
+  RequestOptions()
+      : method("GET"),
+        stream_post_fields(false),
+        timeout_ms(300000),  // Same timeout used by Chromium.
+        verbose(false) {}
 
   // The URL to use in the request.
   std::string url;
@@ -42,6 +45,8 @@ struct RequestOptions {
   bool stream_post_fields;
   // Stores key-value pairs in header.
   std::map<std::string, std::string> header;
+  // The maximum time in milliseconds to allow the request and response.
+  int64_t timeout_ms;
 
   // Set true to make the library display more verbose info to help debug. Does
   // not really affect the connection.

app/rest/response.h

@@ -69,12 +69,11 @@ class Response : public Transfer {
     body_completed_ = true;
   }
 
-  // Marks the response as canceled.  There will never be a response, so stop
+  // Marks the response as failed. There will never be a response, so stop
   // waiting for one.
-  void MarkCanceled() override {
+  void MarkFailed() override {
     header_completed_ = false;
     body_completed_ = false;
-    status_ = rest::util::HttpNoContent;
   }
 
   // Getters.
@@ -85,6 +84,8 @@ class Response : public Transfer {
   std::time_t fetch_time() const { return fetch_time_; }
 
   // Setters.
+  void set_status(int status) { status_ = status; }
+
   void set_sdk_error_code(int sdk_error_code) {
     sdk_error_code_ = sdk_error_code;
   }

app/rest/transfer_interface.h

@@ -27,8 +27,8 @@ class Transfer {
   // Mark the transfer completed.
   virtual void MarkCompleted() = 0;
 
-  // Mark the transfer canceled.
-  virtual void MarkCanceled() = 0;
+  // Mark the transfer failed, usually from cancellation or timeout.
+  virtual void MarkFailed() = 0;
 };
 
 }  // namespace rest

app/rest/transport_curl.cc

@@ -168,7 +168,8 @@ class BackgroundTransportCurl {
 
   CURL* curl() const { return curl_; }
   Response* response() const { return response_; }
-  void set_canceled(bool canceled) { canceled_ = true; }
+  void set_canceled(bool canceled) { canceled_ = canceled; }
+  void set_timed_out(bool timed_out) { timed_out_ = timed_out; }
   ControllerCurl* controller() const { return controller_; }
   TransportCurl* transport_curl() const { return transport_curl_; }
 
@@ -203,6 +204,8 @@ class BackgroundTransportCurl {
   void* complete_data_;
   // Whether the operation has been canceled.
   bool canceled_;
+  // Whether the operation timed out.
+  bool timed_out_;
 };
 
 // The data common to both threads. This is used to communicate when the
@@ -358,7 +361,8 @@ BackgroundTransportCurl::BackgroundTransportCurl(
       transport_curl_(transport_curl),
       complete_(complete),
       complete_data_(complete_data),
-      canceled_(false) {
+      canceled_(false),
+      timed_out_(false) {
   assert(curl_multi_);
   assert(curl_);
   assert(transport_curl);
@@ -389,15 +393,15 @@ BackgroundTransportCurl::~BackgroundTransportCurl() {
     request_header_ = nullptr;
   }
 
-  // If this is an asynchronous operation, MarkCanceled() or MarkCompleted()
+  // If this is an asynchronous operation, MarkFailed() or MarkCompleted()
   // could end up attempting to tear down TransportCurl so we signal
   // completion here.
   if (transport_curl_->is_async()) {
     transport_curl_->SignalTransferComplete();
     CompleteOperation();
   } else {
     // Synchronous operations need all data present in the response before
-    // Perform() returns so signal complete after MarkCanceled() or
+    // Perform() returns so signal complete after MarkFailed() or
     // MarkCompleted().
     CompleteOperation();
     transport_curl_->SignalTransferComplete();
@@ -407,8 +411,13 @@ BackgroundTransportCurl::~BackgroundTransportCurl() {
 void BackgroundTransportCurl::CompleteOperation() {
   if (complete_) complete_(this, complete_data_);
   if (canceled_) {
-    request_->MarkCanceled();
-    response_->MarkCanceled();
+    response_->set_status(rest::util::HttpNoContent);
+    request_->MarkFailed();
+    response_->MarkFailed();
+  } else if (timed_out_) {
+    response_->set_status(rest::util::HttpRequestTimeout);
+    request_->MarkFailed();
+    response_->MarkFailed();
   } else {
     request_->MarkCompleted();
     response_->MarkCompleted();
@@ -456,6 +465,8 @@ bool BackgroundTransportCurl::PerformBackground(Request* request) {
           "set http body read callback");
   CheckOk(curl_easy_setopt(curl_, CURLOPT_READDATA, request),
           "set http body read callback data");
+  CheckOk(curl_easy_setopt(curl_, CURLOPT_TIMEOUT_MS, options.timeout_ms),
+          "set http timeout milliseconds");
 
   // SDK error in initialization stage is not recoverable.
   FIREBASE_ASSERT(err_code_ == CURLE_OK);
@@ -872,9 +883,16 @@ void CurlThread::ProcessRequests() {
             char* char_pointer;
             curl_easy_getinfo(handle, CURLINFO_PRIVATE, &char_pointer);
             curl_multi_remove_handle(curl_multi, handle);
+            BackgroundTransportCurl* transport =
+                reinterpret_cast<BackgroundTransportCurl*>(char_pointer);
+
+            // Determine if the request timed out.
+            if (message->data.result == CURLE_OPERATION_TIMEDOUT) {
+              transport->set_timed_out(true);
+            }
 
             // Mark the response complete.
-            delete reinterpret_cast<BackgroundTransportCurl*>(char_pointer);
+            delete transport;
             expected_running_handles--;
             break;
           }

app/rest/util.h

@@ -51,7 +51,8 @@ enum HttpStatusCode {
   HttpPaymentRequired = 402,
   HttpUnauthorized = 401,
   HttpForbidden = 403,
-  HttpNotFound = 404
+  HttpNotFound = 404,
+  HttpRequestTimeout = 408
 };
 
 // Initialize utilities.  This must be called before any functions that

functions/src/desktop/callable_reference_desktop.cc

@@ -97,8 +97,8 @@ void HttpsCallableRequest::MarkCompleted() {
                                                 response_);
 }
 
-void HttpsCallableRequest::MarkCanceled() {
-  rest::Request::MarkCanceled();
+void HttpsCallableRequest::MarkFailed() {
+  rest::Request::MarkFailed();
   HttpsCallableReferenceInternal::ResolveFuture(future_impl_, future_handle_,
                                                 response_);
 }

functions/src/desktop/callable_reference_desktop.h

@@ -31,8 +31,8 @@ class HttpsCallableRequest : public rest::Request {
   // Mark the transfer completed.
   void MarkCompleted() override;
 
-  // Mark the transfer canceled.
-  void MarkCanceled() override;
+  // Mark the transfer failed.
+  void MarkFailed() override;
 
   void set_future_impl(ReferenceCountedFutureImpl* impl) {
     future_impl_ = impl;

storage/src/desktop/curl_requests.cc

@@ -12,14 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "storage/src/desktop/curl_requests.h"
+
 #include <stdio.h>
-#include <string>
 
-#include "storage/src/desktop/curl_requests.h"
+#include <string>
 
 #include "app/rest/util.h"
 #include "storage/src/desktop/metadata_desktop.h"
 #include "storage/src/desktop/rest_operation.h"
+#include "storage/src/include/firebase/storage/common.h"
 #include "storage/src/include/firebase/storage/metadata.h"
 
 namespace firebase {
@@ -64,24 +66,28 @@ BlockingResponse::BlockingResponse(FutureHandle handle,
 
 BlockingResponse::~BlockingResponse() {
   // If the response isn't complete, cancel it.
-  if (status() == rest::util::HttpInvalid) MarkCanceled();
+  if (status() == rest::util::HttpInvalid) {
+    set_status(rest::util::HttpNoContent);
+    MarkFailed();
+  }
 }
 
 void BlockingResponse::MarkCompleted() { rest::Response::MarkCompleted(); }
 
-void BlockingResponse::MarkCanceled() {
-  rest::Response::MarkCanceled();
-  ref_future_->Complete(SafeFutureHandle<void>(handle_), kErrorCancelled);
-  NotifyCanceled();
+void BlockingResponse::MarkFailed() {
+  rest::Response::MarkFailed();
+  if (status() == rest::util::HttpRequestTimeout) {
+    ref_future_->Complete(SafeFutureHandle<void>(handle_),
+                          kErrorRetryLimitExceeded);
+  } else {
+    ref_future_->Complete(SafeFutureHandle<void>(handle_), kErrorCancelled);
+  }
+  NotifyFailed();
 }
 
-void BlockingResponse::NotifyComplete() {
-  notifier_.NotifyComplete();
-}
+void BlockingResponse::NotifyComplete() { notifier_.NotifyComplete(); }
 
-void BlockingResponse::NotifyCanceled() {
-  notifier_.NotifyCanceled();
-}
+void BlockingResponse::NotifyFailed() { notifier_.NotifyFailed(); }
 
 // Read in the raw response string, and try to make sense of it.
 bool StorageNetworkError::Parse(const char* json_txt) {