feat: Add reachability request timeout to improve handling of bad net…

…work connections (#302 by @davidstritzl) This fixes an issue where a reachability check would not produce a result until a response from the reachability request is received. The fetch API, which is used for this purpose, does not have a built-in timeout mechanism, therefore requests can, in theory, stay pending forever. In case of a bad connection with packet loss, this can mean that no reachability issues are detected, even though there is effectively no internet connection. This commit aims to fix this issue by implementing a user-configurable request timeout.
react-native-netinfo · Feb 12, 2020 · 9eb1531 · 9eb1531
1 parent 0413eee
commit 9eb1531
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 30 deletions.
diff --git a/README.md b/README.md
@@ -284,12 +284,13 @@ Describes the current generation of the `cellular` connection. It is an enum wit
 #### `NetInfoConfiguration`
 The configuration options for the library.
 
-| Property                   | Type                              | Description                                                                                                                                                                                                                               |
-| -------------------------- | --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `reachabilityUrl`          | `string`                          | The URL to call to test if the internet is reachable. Only used on platforms which do not supply internet reachability natively.                                                                                                          |
-| `reachabilityTest`         | `(response: Response) => boolean` | A function which is passed the `Response` from calling the reachability URL. It should return `true` if the response indicates that the internet is reachable. Only used on platforms which do not supply internet reachability natively. |
-| `reachabilityShortTimeout` | `number`                          | The number of seconds between internet reachability checks when the internet was not previously detected. Only used on platforms which do not supply internet reachability natively.                                                      |
-| `reachabilityLongTimeout`  | `number`                          | The number of seconds between internet reachability checks when the internet was previously detected. Only used on platforms which do not supply internet reachability natively.                                                          |
+| Property                     | Type                              | Description                                                                                                                                                                                                                               |
+| ---------------------------- | --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `reachabilityUrl`            | `string`                          | The URL to call to test if the internet is reachable. Only used on platforms which do not supply internet reachability natively.                                                                                                          |
+| `reachabilityTest`           | `(response: Response) => boolean` | A function which is passed the `Response` from calling the reachability URL. It should return `true` if the response indicates that the internet is reachable. Only used on platforms which do not supply internet reachability natively. |
+| `reachabilityShortTimeout`   | `number`                          | The number of milliseconds between internet reachability checks when the internet was not previously detected. Only used on platforms which do not supply internet reachability natively.                                                 |
+| `reachabilityLongTimeout`    | `number`                          | The number of milliseconds between internet reachability checks when the internet was previously detected. Only used on platforms which do not supply internet reachability natively.                                                     |
+| `reachabilityRequestTimeout` | `number`                          | The number of milliseconds that a reachability check is allowed to take before failing. Only used on platforms which do not supply internet reachability natively.                                                                        |
 
 ### Methods
 
@@ -306,6 +307,7 @@ NetInfo.configure({
   reachabilityTest: async (response) => response.status === 204,
   reachabilityLongTimeout: 60 * 1000, // 60s
   reachabilityShortTimeout: 5 * 1000, // 5s
+  reachabilityRequestTimeout: 15 * 1000, // 15s
 });
 ```
 
@@ -358,6 +360,7 @@ const YourComponent = () => {
     reachabilityTest: async (response) => response.status === 204,
     reachabilityLongTimeout: 60 * 1000, // 60s
     reachabilityShortTimeout: 5 * 1000, // 5s
+    reachabilityRequestTimeout: 15 * 1000, // 15s
   });
 
   // ...

diff --git a/src/index.ts b/src/index.ts
@@ -18,6 +18,7 @@ const DEFAULT_CONFIGURATION = {
     Promise.resolve(response.status === 204),
   reachabilityLongTimeout: 60 * 1000, // 60s
   reachabilityShortTimeout: 5 * 1000, // 5s
+  reachabilityRequestTimeout: 15 * 1000, // 15s
 };
 
 // Stores the currently used configuration

diff --git a/src/internal/internetReachability.ts b/src/internal/internetReachability.ts
@@ -68,48 +68,69 @@ export default class InternetReachability {
   };
 
   private _checkInternetReachability = (): InternetReachabilityCheckHandler => {
-    // We wrap the promise to allow us to cancel the pending request, if needed
-    let hasCanceled = false;
+    const responsePromise = fetch(this._configuration.reachabilityUrl);
 
-    const promise = fetch(this._configuration.reachabilityUrl)
+    // Create promise that will reject after the request timeout has been reached
+    let timeoutHandle: ReturnType<typeof setTimeout>;
+    const timeoutPromise = new Promise<Response>(
+      (_, reject): void => {
+        timeoutHandle = setTimeout(
+          (): void => reject('timedout'),
+          this._configuration.reachabilityRequestTimeout,
+        );
+      },
+    );
+
+    // Create promise that makes it possible to cancel a pending request through a reject
+    let cancel: () => void = (): void => {};
+    const cancelPromise = new Promise<Response>(
+      (_, reject): void => {
+        cancel = (): void => reject('canceled');
+      },
+    );
+
+    const promise = Promise.race([
+      responsePromise,
+      timeoutPromise,
+      cancelPromise,
+    ])
       .then(
-        (response): Promise<boolean | 'canceled'> => {
-          if (!hasCanceled) {
-            return this._configuration.reachabilityTest(response);
-          } else {
-            return Promise.resolve('canceled');
-          }
+        (response): Promise<boolean> => {
+          return this._configuration.reachabilityTest(response);
         },
       )
       .then(
         (result): void => {
-          if (result !== 'canceled') {
-            this._setIsInternetReachable(result);
-            const nextTimeoutInterval = this._isInternetReachable
-              ? this._configuration.reachabilityLongTimeout
-              : this._configuration.reachabilityShortTimeout;
+          this._setIsInternetReachable(result);
+          const nextTimeoutInterval = this._isInternetReachable
+            ? this._configuration.reachabilityLongTimeout
+            : this._configuration.reachabilityShortTimeout;
+          this._currentTimeoutHandle = setTimeout(
+            this._checkInternetReachability,
+            nextTimeoutInterval,
+          );
+        },
+      )
+      .catch(
+        (error: Error | 'timedout' | 'canceled'): void => {
+          if (error !== 'canceled') {
+            this._setIsInternetReachable(false);
             this._currentTimeoutHandle = setTimeout(
               this._checkInternetReachability,
-              nextTimeoutInterval,
+              this._configuration.reachabilityShortTimeout,
             );
           }
         },
       )
-      .catch(
+      .finally(
         (): void => {
-          this._setIsInternetReachable(false);
-          this._currentTimeoutHandle = setTimeout(
-            this._checkInternetReachability,
-            this._configuration.reachabilityShortTimeout,
-          );
+          clearTimeout(timeoutHandle);
         },
       );
 
     return {
       promise,
-      cancel: (): void => {
-        hasCanceled = true;
-      },
+      cancel,
     };
   };
 

diff --git a/src/internal/types.ts b/src/internal/types.ts
@@ -99,4 +99,5 @@ export interface NetInfoConfiguration {
   reachabilityTest: (response: Response) => Promise<boolean>;
   reachabilityLongTimeout: number;
   reachabilityShortTimeout: number;
+  reachabilityRequestTimeout: number;
 }