Implement XHR timeout for Android and IOS natively.

Summary:
Opening this in a separate PR but the discussion can be viewed on facebook#4832.

Basically, this is a native implementation and is a bit more elegant. The consensus on my previous PR was that it should be done natively rather than in JS.

There's now no maximum valid timeout value and a timeout of 0 will never time out.

ontimeout isn't implemented (yet) in this PR.

cc nicklockwood ide philikon
Closes facebook#5038

Reviewed By: svcscm

Differential Revision: D2838743

Pulled By: nicklockwood

fb-gh-sync-id: 774f864ac35082bf522f7665f4311bd3affbe82c

Libraries/Network/RCTNetworking.android.js

@@ -25,15 +25,16 @@ var generateRequestId = function() {
  */
 class RCTNetworking {
 
-  static sendRequest(method, url, headers, data, useIncrementalUpdates) {
+  static sendRequest(method, url, headers, data, useIncrementalUpdates, timeout) {
     var requestId = generateRequestId();
     RCTNetworkingNative.sendRequest(
       method,
       url,
       requestId,
       headers,
       data,
-      useIncrementalUpdates);
+      useIncrementalUpdates,
+      timeout);
     return requestId;
   }
 

Libraries/Network/RCTNetworking.m

@@ -210,7 +210,7 @@ - (RCTURLRequestCancellationBlock)buildRequest:(NSDictionary<NSString *, id> *)q
   NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL];
   request.HTTPMethod = [RCTConvert NSString:RCTNilIfNull(query[@"method"])].uppercaseString ?: @"GET";
   request.allHTTPHeaderFields = [RCTConvert NSDictionary:query[@"headers"]];
-
+  request.timeoutInterval = [RCTConvert NSTimeInterval:query[@"timeout"]];
   NSDictionary<NSString *, id> *data = [RCTConvert NSDictionary:RCTNilIfNull(query[@"data"])];
   return [self processDataForHTTPQuery:data callback:^(NSError *error, NSDictionary<NSString *, id> *result) {
     if (error) {

Libraries/Network/XMLHttpRequest.android.js

@@ -26,7 +26,7 @@ function convertHeadersMapToArray(headers: Object): Array<Header> {
 }
 
 class XMLHttpRequest extends XMLHttpRequestBase {
-  sendImpl(method: ?string, url: ?string, headers: Object, data: any): void {
+  sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void {
     var body;
     if (typeof data === 'string') {
       body = {string: data};
@@ -40,14 +40,14 @@ class XMLHttpRequest extends XMLHttpRequestBase {
     } else {
       body = data;
     }
-
     var useIncrementalUpdates = this.onreadystatechange ? true : false;
     var requestId = RCTNetworking.sendRequest(
       method,
       url,
       convertHeadersMapToArray(headers),
       body,
-      useIncrementalUpdates
+      useIncrementalUpdates,
+      timeout
     );
     this.didCreateRequest(requestId);
   }

Libraries/Network/XMLHttpRequest.ios.js

@@ -23,7 +23,7 @@ class XMLHttpRequest extends XMLHttpRequestBase {
     this.upload = {};
   }
 
-  sendImpl(method: ?string, url: ?string, headers: Object, data: any): void {
+  sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void {
     if (typeof data === 'string') {
       data = {string: data};
     } else if (data instanceof FormData) {
@@ -36,6 +36,7 @@ class XMLHttpRequest extends XMLHttpRequestBase {
         data,
         headers,
         incrementalUpdates: this.onreadystatechange ? true : false,
+        timeout
       },
       this.didCreateRequest.bind(this)
     );

Libraries/Network/XMLHttpRequestBase.js

@@ -32,6 +32,7 @@ class XMLHttpRequestBase {
   responseHeaders: ?Object;
   responseText: ?string;
   status: number;
+  timeout: number;
   responseURL: ?string;
 
   upload: ?{
@@ -58,6 +59,7 @@ class XMLHttpRequestBase {
     this.onreadystatechange = null;
     this.onload = null;
     this.upload = undefined; /* Upload not supported yet */
+    this.timeout = 0;
 
     this._reset();
     this._method = null;
@@ -196,7 +198,7 @@ class XMLHttpRequestBase {
     this.setReadyState(this.OPENED);
   }
 
-  sendImpl(method: ?string, url: ?string, headers: Object, data: any): void {
+  sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void {
     throw new Error('Subclass must define sendImpl method');
   }
 
@@ -208,7 +210,7 @@ class XMLHttpRequestBase {
       throw new Error('Request has already been sent');
     }
     this._sent = true;
-    this.sendImpl(this._method, this._url, this._headers, data);
+    this.sendImpl(this._method, this._url, this._headers, data, this.timeout);
   }
 
   abort(): void {

ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java

@@ -15,6 +15,8 @@
 import java.io.InputStream;
 import java.io.Reader;
 
+import java.util.concurrent.TimeUnit;
+
 import com.facebook.react.bridge.Arguments;
 import com.facebook.react.bridge.GuardedAsyncTask;
 import com.facebook.react.bridge.ReactApplicationContext;
@@ -113,19 +115,25 @@ public void onCatalystInstanceDestroy() {
   }
 
   @ReactMethod
+  /**
+  * @param timeout value of 0 results in no timeout
+  */
   public void sendRequest(
       String method,
       String url,
       final int requestId,
       ReadableArray headers,
       ReadableMap data,
-      final boolean useIncrementalUpdates) {
+      final boolean useIncrementalUpdates,
+      int timeout) {
     Request.Builder requestBuilder = new Request.Builder().url(url);
 
     if (requestId != 0) {
       requestBuilder.tag(requestId);
     }
 
+    mClient.setConnectTimeout(timeout, TimeUnit.MILLISECONDS);
+
     Headers requestHeaders = extractHeaders(headers, data);
     if (requestHeaders == null) {
       onRequestError(requestId, "Unrecognized headers format");

ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java

@@ -93,7 +93,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         SimpleArray.of(),
         null,
-        true);
+        true,
+        0);
 
     ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
     verify(httpClient).newCall(argumentCaptor.capture());
@@ -122,7 +123,8 @@ public void testFailGetWithInvalidHeadersStruct() throws Exception {
         0,
         SimpleArray.from(invalidHeaders),
         null,
-        true);
+        true,
+        0);
 
     verifyErrorEmit(emitter, 0);
   }
@@ -147,7 +149,8 @@ public void testFailPostWithoutContentType() throws Exception {
         0,
         SimpleArray.of(),
         body,
-        true);
+        true,
+        0);
 
     verifyErrorEmit(emitter, 0);
   }
@@ -202,7 +205,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         SimpleArray.of(SimpleArray.of("Content-Type", "text/plain")),
         body,
-        true);
+        true,
+        0);
 
     ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
     verify(httpClient).newCall(argumentCaptor.capture());
@@ -238,7 +242,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         SimpleArray.from(headers),
         null,
-        true);
+        true,
+        0);
     ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
     verify(httpClient).newCall(argumentCaptor.capture());
     Headers requestHeaders = argumentCaptor.getValue().headers();
@@ -284,7 +289,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         new SimpleArray(),
         body,
-        true);
+        true,
+        0);
 
     // verify url, method, headers
     ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
@@ -341,7 +347,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         SimpleArray.from(headers),
         body,
-        true);
+        true,
+        0);
 
     // verify url, method, headers
     ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
@@ -435,7 +442,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         0,
         SimpleArray.from(headers),
         body,
-        true);
+        true,
+        0);
 
     // verify RequestBodyPart for image
     PowerMockito.verifyStatic(times(1));