From 1303e6d0392e8bc4da1198c98fc8cd7ad488b81c Mon Sep 17 00:00:00 2001 From: Alex Roman Date: Mon, 18 Jan 2016 07:47:04 -0800 Subject: [PATCH] Implement XHR timeout for Android and IOS natively. Summary: Opening this in a separate PR but the discussion can be viewed on #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 https://github.com/facebook/react-native/pull/5038 Reviewed By: svcscm Differential Revision: D2838743 Pulled By: nicklockwood fb-gh-sync-id: 774f864ac35082bf522f7665f4311bd3affbe82c --- Libraries/Network/RCTNetworking.android.js | 5 ++-- Libraries/Network/RCTNetworking.m | 2 +- Libraries/Network/XMLHttpRequest.android.js | 6 ++--- Libraries/Network/XMLHttpRequest.ios.js | 3 ++- Libraries/Network/XMLHttpRequestBase.js | 6 +++-- .../modules/network/NetworkingModule.java | 10 +++++++- .../modules/network/NetworkingModuleTest.java | 24 ++++++++++++------- 7 files changed, 38 insertions(+), 18 deletions(-) diff --git a/Libraries/Network/RCTNetworking.android.js b/Libraries/Network/RCTNetworking.android.js index 351524eb991..38f694d5452 100644 --- a/Libraries/Network/RCTNetworking.android.js +++ b/Libraries/Network/RCTNetworking.android.js @@ -25,7 +25,7 @@ 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, @@ -33,7 +33,8 @@ class RCTNetworking { requestId, headers, data, - useIncrementalUpdates); + useIncrementalUpdates, + timeout); return requestId; } diff --git a/Libraries/Network/RCTNetworking.m b/Libraries/Network/RCTNetworking.m index 74954199100..5e30813430d 100644 --- a/Libraries/Network/RCTNetworking.m +++ b/Libraries/Network/RCTNetworking.m @@ -210,7 +210,7 @@ RCT_EXPORT_MODULE() 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 *data = [RCTConvert NSDictionary:RCTNilIfNull(query[@"data"])]; return [self processDataForHTTPQuery:data callback:^(NSError *error, NSDictionary *result) { if (error) { diff --git a/Libraries/Network/XMLHttpRequest.android.js b/Libraries/Network/XMLHttpRequest.android.js index e0ebc77d0af..165b6d316e2 100644 --- a/Libraries/Network/XMLHttpRequest.android.js +++ b/Libraries/Network/XMLHttpRequest.android.js @@ -26,7 +26,7 @@ function convertHeadersMapToArray(headers: Object): Array
{ } 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); } diff --git a/Libraries/Network/XMLHttpRequest.ios.js b/Libraries/Network/XMLHttpRequest.ios.js index 21614ff3616..d79e38019f4 100644 --- a/Libraries/Network/XMLHttpRequest.ios.js +++ b/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) ); diff --git a/Libraries/Network/XMLHttpRequestBase.js b/Libraries/Network/XMLHttpRequestBase.js index f4c8bc5e246..1fffb713f97 100644 --- a/Libraries/Network/XMLHttpRequestBase.js +++ b/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 { diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java index d1f32616e6a..9374683b9d8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java @@ -15,6 +15,8 @@ import java.io.IOException; 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 final class NetworkingModule extends ReactContextBaseJavaModule { } @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"); diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java index 454a3cb986a..64884aabdb5 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java @@ -93,7 +93,8 @@ public class NetworkingModuleTest { 0, SimpleArray.of(), null, - true); + true, + 0); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); @@ -122,7 +123,8 @@ public class NetworkingModuleTest { 0, SimpleArray.from(invalidHeaders), null, - true); + true, + 0); verifyErrorEmit(emitter, 0); } @@ -147,7 +149,8 @@ public class NetworkingModuleTest { 0, SimpleArray.of(), body, - true); + true, + 0); verifyErrorEmit(emitter, 0); } @@ -202,7 +205,8 @@ public class NetworkingModuleTest { 0, SimpleArray.of(SimpleArray.of("Content-Type", "text/plain")), body, - true); + true, + 0); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); @@ -238,7 +242,8 @@ public class NetworkingModuleTest { 0, SimpleArray.from(headers), null, - true); + true, + 0); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); Headers requestHeaders = argumentCaptor.getValue().headers(); @@ -284,7 +289,8 @@ public class NetworkingModuleTest { 0, new SimpleArray(), body, - true); + true, + 0); // verify url, method, headers ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); @@ -341,7 +347,8 @@ public class NetworkingModuleTest { 0, SimpleArray.from(headers), body, - true); + true, + 0); // verify url, method, headers ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); @@ -435,7 +442,8 @@ public class NetworkingModuleTest { 0, SimpleArray.from(headers), body, - true); + true, + 0); // verify RequestBodyPart for image PowerMockito.verifyStatic(times(1));