⚡ Ignore zero duration timeout (#2029)

Resolves #2022. ### New Pull Request Checklist - [x] I have read the [Documentation](https://pub.dev/documentation/dio/latest/) - [x] I have searched for a similar pull request in the [project](https://github.com/cfug/dio/pulls) and found none - [x] I have updated this branch with the latest `main` branch to avoid conflicts (via merge from master or rebase) - [x] I have added the required tests to prove the fix/feature I'm adding - [x] I have updated the documentation (if necessary) - [x] I have run the tests without failures - [x] I have updated the `CHANGELOG.md` in the corresponding package ### Additional context and info (if any) I've found that the `IOHttpClientAdapter` uses `Stopwatch` to integrate with `receiveTimeout`, which might cause infinity awaits if the stream has no response forever. This might be the root cause of #1739. https://github.com/cfug/dio/blob/78f3813a8d8948887198ef628e5a2e2039489b03/dio/lib/src/adapters/io_adapter.dart#L194-L200 --------- Signed-off-by: Alex Li <[email protected]>
cfug · Nov 21, 2023 · 3595442 · 3595442
1 parent bedcc54
commit 3595442
Show file tree

Hide file tree

Showing 8 changed files with 103 additions and 56 deletions.
diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md
@@ -6,8 +6,9 @@ See the [Migration Guide][] for the complete breaking changes list.**
 ## Unreleased
 
 - Raise warning for `Map`s other than `Map<String, dynamic>` when encoding request data.
-- Improve exception messages
-- Allow `ResponseDecoder` and `RequestEncoder` to be async
+- Improve exception messages.
+- Allow `ResponseDecoder` and `RequestEncoder` to be async.
+- Ignores `Duration.zero` timeouts.
 
 ## 5.3.3
 

diff --git a/dio/lib/src/adapters/browser_adapter.dart b/dio/lib/src/adapters/browser_adapter.dart
@@ -58,15 +58,11 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
       }
     });
 
-    final connectTimeout = options.connectTimeout;
-    final receiveTimeout = options.receiveTimeout;
-    int xhrTimeout = 0;
-    if (connectTimeout != null &&
-        receiveTimeout != null &&
-        receiveTimeout > Duration.zero) {
-      xhrTimeout = (connectTimeout + receiveTimeout).inMilliseconds;
-      xhr.timeout = xhrTimeout;
-    }
+    final sendTimeout = options.sendTimeout ?? Duration.zero;
+    final connectTimeout = options.connectTimeout ?? Duration.zero;
+    final receiveTimeout = options.receiveTimeout ?? Duration.zero;
+    final xhrTimeout = (connectTimeout + receiveTimeout).inMilliseconds;
+    xhr.timeout = xhrTimeout;
 
     final completer = Completer<ResponseBody>();
 
@@ -86,22 +82,20 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
     });
 
     Timer? connectTimeoutTimer;
-
-    final connectionTimeout = options.connectTimeout;
-    if (connectionTimeout != null) {
+    if (connectTimeout > Duration.zero) {
       connectTimeoutTimer = Timer(
-        connectionTimeout,
+        connectTimeout,
         () {
+          connectTimeoutTimer = null;
           if (completer.isCompleted) {
             // connectTimeout is triggered after the fetch has been completed.
             return;
           }
-
           xhr.abort();
           completer.completeError(
             DioException.connectionTimeout(
               requestOptions: options,
-              timeout: connectionTimeout,
+              timeout: connectTimeout,
             ),
             StackTrace.current,
           );
@@ -123,14 +117,12 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
         });
       }
 
-      final sendTimeout = options.sendTimeout;
-      if (sendTimeout != null) {
+      if (sendTimeout > Duration.zero) {
         final uploadStopwatch = Stopwatch();
         xhr.upload.onProgress.listen((event) {
           if (!uploadStopwatch.isRunning) {
             uploadStopwatch.start();
           }
-
           final duration = uploadStopwatch.elapsed;
           if (duration > sendTimeout) {
             uploadStopwatch.stop();
@@ -155,7 +147,7 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
         });
       }
     } else {
-      if (options.sendTimeout != null) {
+      if (sendTimeout > Duration.zero) {
         debugLog(
           'sendTimeout cannot be used without a request body to send',
           StackTrace.current,
@@ -176,25 +168,24 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
         connectTimeoutTimer = null;
       }
 
-      final receiveTimeout = options.receiveTimeout;
-      if (receiveTimeout != null) {
+      if (receiveTimeout > Duration.zero) {
         if (!downloadStopwatch.isRunning) {
           downloadStopwatch.start();
         }
-
         final duration = downloadStopwatch.elapsed;
         if (duration > receiveTimeout) {
           downloadStopwatch.stop();
           completer.completeError(
             DioException.receiveTimeout(
-              timeout: options.receiveTimeout!,
+              timeout: receiveTimeout,
               requestOptions: options,
             ),
             StackTrace.current,
           );
           xhr.abort();
         }
       }
+
       if (options.onReceiveProgress != null) {
         if (event.loaded != null && event.total != null) {
           options.onReceiveProgress!(event.loaded!, event.total!);
@@ -218,17 +209,27 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
     });
 
     xhr.onTimeout.first.then((_) {
+      final isConnectTimeout = connectTimeoutTimer != null;
       if (connectTimeoutTimer != null) {
         connectTimeoutTimer?.cancel();
       }
       if (!completer.isCompleted) {
-        completer.completeError(
-          DioException.receiveTimeout(
-            timeout: Duration(milliseconds: xhrTimeout),
-            requestOptions: options,
-          ),
-          StackTrace.current,
-        );
+        if (isConnectTimeout) {
+          completer.completeError(
+            DioException.connectionTimeout(
+              timeout: connectTimeout,
+              requestOptions: options,
+            ),
+          );
+        } else {
+          completer.completeError(
+            DioException.receiveTimeout(
+              timeout: Duration(milliseconds: xhrTimeout),
+              requestOptions: options,
+            ),
+            StackTrace.current,
+          );
+        }
       }
     });
 

diff --git a/dio/lib/src/adapters/io_adapter.dart b/dio/lib/src/adapters/io_adapter.dart
@@ -92,7 +92,7 @@ class IOHttpClientAdapter implements HttpClientAdapter {
     late HttpClientRequest request;
     try {
       final connectionTimeout = options.connectTimeout;
-      if (connectionTimeout != null) {
+      if (connectionTimeout != null && connectionTimeout > Duration.zero) {
         request = await reqFuture.timeout(
           connectionTimeout,
           onTimeout: () {
@@ -116,11 +116,19 @@ class IOHttpClientAdapter implements HttpClientAdapter {
       });
     } on SocketException catch (e) {
       if (e.message.contains('timed out')) {
+        final Duration effectiveTimeout;
+        if (options.connectTimeout != null &&
+            options.connectTimeout! > Duration.zero) {
+          effectiveTimeout = options.connectTimeout!;
+        } else if (httpClient.connectionTimeout != null &&
+            httpClient.connectionTimeout! > Duration.zero) {
+          effectiveTimeout = httpClient.connectionTimeout!;
+        } else {
+          effectiveTimeout = Duration.zero;
+        }
         throw DioException.connectionTimeout(
           requestOptions: options,
-          timeout: options.connectTimeout ??
-              httpClient.connectionTimeout ??
-              Duration.zero,
+          timeout: effectiveTimeout,
           error: e,
         );
       }
@@ -139,7 +147,7 @@ class IOHttpClientAdapter implements HttpClientAdapter {
       // Transform the request data.
       Future<dynamic> future = request.addStream(requestStream);
       final sendTimeout = options.sendTimeout;
-      if (sendTimeout != null) {
+      if (sendTimeout != null && sendTimeout > Duration.zero) {
         future = future.timeout(
           sendTimeout,
           onTimeout: () {
@@ -157,7 +165,7 @@ class IOHttpClientAdapter implements HttpClientAdapter {
     final stopwatch = Stopwatch()..start();
     Future<HttpClientResponse> future = request.close();
     final receiveTimeout = options.receiveTimeout;
-    if (receiveTimeout != null) {
+    if (receiveTimeout != null && receiveTimeout > Duration.zero) {
       future = future.timeout(
         receiveTimeout,
         onTimeout: () {
@@ -195,7 +203,9 @@ class IOHttpClientAdapter implements HttpClientAdapter {
           stopwatch.stop();
           final duration = stopwatch.elapsed;
           final receiveTimeout = options.receiveTimeout;
-          if (receiveTimeout != null && duration > receiveTimeout) {
+          if (receiveTimeout != null &&
+              receiveTimeout > Duration.zero &&
+              duration > receiveTimeout) {
             sink.addError(
               DioException.receiveTimeout(
                 timeout: receiveTimeout,
@@ -228,8 +238,11 @@ class IOHttpClientAdapter implements HttpClientAdapter {
   }
 
   HttpClient _configHttpClient(Duration? connectionTimeout) {
-    return (_cachedHttpClient ??= _createHttpClient())
-      ..connectionTimeout = connectionTimeout;
+    _cachedHttpClient ??= _createHttpClient();
+    if (connectionTimeout != null && connectionTimeout > Duration.zero) {
+      _cachedHttpClient!.connectionTimeout = connectionTimeout;
+    }
+    return _cachedHttpClient!;
   }
 
   @override

diff --git a/dio/lib/src/options.dart b/dio/lib/src/options.dart
@@ -107,7 +107,7 @@ mixin OptionsMixin {
   /// [Dio] will throw the [DioException] with
   /// [DioExceptionType.connectionTimeout] type when time out.
   ///
-  /// `null` meanings no timeout limit.
+  /// `null` or `Duration.zero` meanings no timeout limit.
   Duration? get connectTimeout => _connectTimeout;
   Duration? _connectTimeout;
 
@@ -363,7 +363,7 @@ class Options {
   /// [Dio] will throw the [DioException] with
   /// [DioExceptionType.sendTimeout] type when timed out.
   ///
-  /// `null` meanings no timeout limit.
+  /// `null` or `Duration.zero` meanings no timeout limit.
   Duration? get sendTimeout => _sendTimeout;
   Duration? _sendTimeout;
 
@@ -382,7 +382,7 @@ class Options {
   /// [Dio] will throw the [DioException] with
   /// [DioExceptionType.receiveTimeout] type when time out.
   ///
-  /// `null` meanings no timeout limit.
+  /// `null` or `Duration.zero` meanings no timeout limit.
   Duration? get receiveTimeout => _receiveTimeout;
   Duration? _receiveTimeout;
 

diff --git a/dio/test/timeout_test.dart b/dio/test/timeout_test.dart
@@ -70,4 +70,31 @@ void main() {
     } on DioException catch (_) {}
     expect(http.connectionTimeout?.inSeconds == 1, isTrue);
   }, testOn: 'vm');
+
+  test('ignores zero duration timeouts', () async {
+    final dio = Dio(
+      BaseOptions(
+        baseUrl: 'https://httpbun.com/',
+        connectTimeout: Duration.zero,
+        sendTimeout: Duration.zero,
+        receiveTimeout: Duration.zero,
+      ),
+    );
+    // Ignores zero duration timeouts from the base options.
+    await dio.get('/drip-lines?delay=1');
+    // Reset the base options.
+    dio.options
+      ..connectTimeout = Duration(seconds: 10)
+      ..sendTimeout = Duration(seconds: 10)
+      ..receiveTimeout = Duration(seconds: 10);
+    // Override with request options.
+    await dio.get(
+      '/drip-lines?delay=1',
+      options: Options(
+        sendTimeout: Duration.zero,
+        receiveTimeout: Duration.zero,
+      ),
+    );
+    await dio.get('/drip-lines?delay=1');
+  });
 }
diff --git a/plugins/http2_adapter/CHANGELOG.md b/plugins/http2_adapter/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Implement `sendTimeout` and `receiveTimeout` for the adapter.
 - Fix redirect not working when requestStream is null.
+- Ignores `Duration.zero` timeouts.
 
 ## 2.3.1+1
 

diff --git a/plugins/http2_adapter/lib/src/connection_manager_imp.dart b/plugins/http2_adapter/lib/src/connection_manager_imp.dart
@@ -85,7 +85,7 @@ class _ConnectionManager implements ConnectionManager {
       if (e.osError == null) {
         if (e.message.contains('timed out')) {
           throw DioException.connectionTimeout(
-            timeout: options.connectTimeout!,
+            timeout: options.connectTimeout ?? Duration.zero,
             requestOptions: options,
           );
         }
@@ -135,25 +135,29 @@ class _ConnectionManager implements ConnectionManager {
     RequestOptions options,
     ClientSetting clientConfig,
   ) async {
-    if (clientConfig.proxy == null) {
+    final timeout = (options.connectTimeout ?? Duration.zero) > Duration.zero
+        ? options.connectTimeout!
+        : null;
+    final proxy = clientConfig.proxy;
+
+    if (proxy == null) {
       return SecureSocket.connect(
         target.host,
         target.port,
-        timeout: options.connectTimeout,
+        timeout: timeout,
         context: clientConfig.context,
         onBadCertificate: clientConfig.onBadCertificate,
         supportedProtocols: ['h2'],
       );
     }
 
     final proxySocket = await Socket.connect(
-      clientConfig.proxy!.host,
-      clientConfig.proxy!.port,
-      timeout: options.connectTimeout,
+      proxy.host,
+      proxy.port,
+      timeout: timeout,
     );
 
-    final String credentialsProxy =
-        base64Encode(utf8.encode(clientConfig.proxy!.userInfo));
+    final String credentialsProxy = base64Encode(utf8.encode(proxy.userInfo));
 
     // Create http tunnel proxy https://www.ietf.org/rfc/rfc2817.txt
 

diff --git a/plugins/http2_adapter/lib/src/http2_adapter.dart b/plugins/http2_adapter/lib/src/http2_adapter.dart
@@ -92,8 +92,8 @@ class Http2Adapter implements HttpClientAdapter {
       final requestStreamFuture = requestStream!.listen((data) {
         stream.outgoingMessages.add(DataStreamMessage(data));
       }).asFuture();
-      final sendTimeout = options.sendTimeout;
-      if (sendTimeout != null) {
+      final sendTimeout = options.sendTimeout ?? Duration.zero;
+      if (sendTimeout > Duration.zero) {
         await requestStreamFuture.timeout(
           sendTimeout,
           onTimeout: () {
@@ -158,8 +158,8 @@ class Http2Adapter implements HttpClientAdapter {
       cancelOnError: true,
     );
 
-    final receiveTimeout = options.receiveTimeout;
-    if (receiveTimeout != null) {
+    final receiveTimeout = options.receiveTimeout ?? Duration.zero;
+    if (receiveTimeout > Duration.zero) {
       await completer.future.timeout(
         receiveTimeout,
         onTimeout: () {