From 9eb15311d2ee9e615a3bdf3f14b955ec4c4102f8 Mon Sep 17 00:00:00 2001 From: David Stritzl <437018+davidstritzl@users.noreply.github.com> Date: Wed, 12 Feb 2020 19:04:06 +0100 Subject: [PATCH] feat: Add reachability request timeout to improve handling of bad network 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. --- README.md | 15 +++--- src/index.ts | 1 + src/internal/internetReachability.ts | 69 ++++++++++++++++++---------- src/internal/types.ts | 1 + 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 64236a10..8ee31369 100644 --- 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 index 5435dab7..442174d0 100644 --- 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 index c9d97e85..2ad70e72 100644 --- 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; + const timeoutPromise = new Promise( + (_, 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( + (_, reject): void => { + cancel = (): void => reject('canceled'); + }, + ); + + const promise = Promise.race([ + responsePromise, + timeoutPromise, + cancelPromise, + ]) .then( - (response): Promise => { - if (!hasCanceled) { - return this._configuration.reachabilityTest(response); - } else { - return Promise.resolve('canceled'); - } + (response): Promise => { + 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 index ed7bcbc7..48fcfd0d 100644 --- a/src/internal/types.ts +++ b/src/internal/types.ts @@ -99,4 +99,5 @@ export interface NetInfoConfiguration { reachabilityTest: (response: Response) => Promise; reachabilityLongTimeout: number; reachabilityShortTimeout: number; + reachabilityRequestTimeout: number; }