From 847ba46864289eca80b685dcb04437210a991e3e Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 20 Jan 2021 16:43:50 -0800 Subject: [PATCH 01/11] Add support for request timeout Wire up a timer to perform the appropriate type of aborting and resource cleanup for each client. Handles canceling the timer when the request completes. --- lib/http.dart | 60 ++++++++++++++++++++++++------------- lib/src/base_client.dart | 58 +++++++++++++++++++++++------------ lib/src/base_request.dart | 4 +-- lib/src/browser_client.dart | 37 +++++++++++++++++------ lib/src/client.dart | 34 +++++++++++++++------ lib/src/io_client.dart | 59 ++++++++++++++++++++++++++++++------ lib/src/mock_client.dart | 3 +- 7 files changed, 186 insertions(+), 69 deletions(-) diff --git a/lib/http.dart b/lib/http.dart index 1ea751eab1..cefdcd29a0 100644 --- a/lib/http.dart +++ b/lib/http.dart @@ -32,8 +32,10 @@ export 'src/streamed_response.dart'; /// the same server, you should use a single [Client] for all of those requests. /// /// For more fine-grained control over the request, use [Request] instead. -Future head(Uri url, {Map? headers}) => - _withClient((client) => client.head(url, headers: headers)); +Future head(Uri url, + {Map? headers, Duration? timeout}) => + _withClient( + (client) => client.head(url, headers: headers, timeout: timeout)); /// Sends an HTTP GET request with the given headers to the given URL. /// @@ -42,8 +44,10 @@ Future head(Uri url, {Map? headers}) => /// the same server, you should use a single [Client] for all of those requests. /// /// For more fine-grained control over the request, use [Request] instead. -Future get(Uri url, {Map? headers}) => - _withClient((client) => client.get(url, headers: headers)); +Future get(Uri url, + {Map? headers, Duration? timeout}) => + _withClient( + (client) => client.get(url, headers: headers, timeout: timeout)); /// Sends an HTTP POST request with the given headers and body to the given URL. /// @@ -64,9 +68,12 @@ Future get(Uri url, {Map? headers}) => /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future post(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _withClient((client) => - client.post(url, headers: headers, body: body, encoding: encoding)); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _withClient((client) => client.post(url, + headers: headers, body: body, encoding: encoding, timeout: timeout)); /// Sends an HTTP PUT request with the given headers and body to the given URL. /// @@ -87,9 +94,12 @@ Future post(Uri url, /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future put(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _withClient((client) => - client.put(url, headers: headers, body: body, encoding: encoding)); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _withClient((client) => client.put(url, + headers: headers, body: body, encoding: encoding, timeout: timeout)); /// Sends an HTTP PATCH request with the given headers and body to the given /// URL. @@ -111,9 +121,12 @@ Future put(Uri url, /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future patch(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _withClient((client) => - client.patch(url, headers: headers, body: body, encoding: encoding)); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _withClient((client) => client.patch(url, + headers: headers, body: body, encoding: encoding, timeout: timeout)); /// Sends an HTTP DELETE request with the given headers to the given URL. /// @@ -123,9 +136,12 @@ Future patch(Uri url, /// /// For more fine-grained control over the request, use [Request] instead. Future delete(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _withClient((client) => - client.delete(url, headers: headers, body: body, encoding: encoding)); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _withClient((client) => client.delete(url, + headers: headers, body: body, encoding: encoding, timeout: timeout)); /// Sends an HTTP GET request with the given headers to the given URL and /// returns a Future that completes to the body of the response as a [String]. @@ -139,8 +155,10 @@ Future delete(Uri url, /// /// For more fine-grained control over the request and response, use [Request] /// instead. -Future read(Uri url, {Map? headers}) => - _withClient((client) => client.read(url, headers: headers)); +Future read(Uri url, + {Map? headers, Duration? timeout}) => + _withClient( + (client) => client.read(url, headers: headers, timeout: timeout)); /// Sends an HTTP GET request with the given headers to the given URL and /// returns a Future that completes to the body of the response as a list of @@ -155,8 +173,10 @@ Future read(Uri url, {Map? headers}) => /// /// For more fine-grained control over the request and response, use [Request] /// instead. -Future readBytes(Uri url, {Map? headers}) => - _withClient((client) => client.readBytes(url, headers: headers)); +Future readBytes(Uri url, + {Map? headers, Duration? timeout}) => + _withClient( + (client) => client.readBytes(url, headers: headers, timeout: timeout)); Future _withClient(Future Function(Client) fn) async { var client = Client(); diff --git a/lib/src/base_client.dart b/lib/src/base_client.dart index efb065f70e..dd4f7112c1 100644 --- a/lib/src/base_client.dart +++ b/lib/src/base_client.dart @@ -19,43 +19,63 @@ import 'streamed_response.dart'; /// maybe [close], and then they get various convenience methods for free. abstract class BaseClient implements Client { @override - Future head(Uri url, {Map? headers}) => - _sendUnstreamed('HEAD', url, headers); + Future head(Uri url, + {Map? headers, Duration? timeout}) => + _sendUnstreamed('HEAD', url, headers, timeout: timeout); @override - Future get(Uri url, {Map? headers}) => - _sendUnstreamed('GET', url, headers); + Future get(Uri url, + {Map? headers, Duration? timeout}) => + _sendUnstreamed('GET', url, headers, timeout: timeout); @override Future post(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _sendUnstreamed('POST', url, headers, body, encoding); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _sendUnstreamed('POST', url, headers, + body: body, encoding: encoding, timeout: timeout); @override Future put(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _sendUnstreamed('PUT', url, headers, body, encoding); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _sendUnstreamed('PUT', url, headers, + body: body, encoding: encoding, timeout: timeout); @override Future patch(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _sendUnstreamed('PATCH', url, headers, body, encoding); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _sendUnstreamed('PATCH', url, headers, + body: body, encoding: encoding, timeout: timeout); @override Future delete(Uri url, - {Map? headers, Object? body, Encoding? encoding}) => - _sendUnstreamed('DELETE', url, headers, body, encoding); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}) => + _sendUnstreamed('DELETE', url, headers, + body: body, encoding: encoding, timeout: timeout); @override - Future read(Uri url, {Map? headers}) async { - final response = await get(url, headers: headers); + Future read(Uri url, + {Map? headers, Duration? timeout}) async { + final response = await get(url, headers: headers, timeout: timeout); _checkResponseSuccess(url, response); return response.body; } @override - Future readBytes(Uri url, {Map? headers}) async { - final response = await get(url, headers: headers); + Future readBytes(Uri url, + {Map? headers, Duration? timeout}) async { + final response = await get(url, headers: headers, timeout: timeout); _checkResponseSuccess(url, response); return response.bodyBytes; } @@ -68,12 +88,12 @@ abstract class BaseClient implements Client { /// later point, or it could already be closed when it's returned. Any /// internal HTTP errors should be wrapped as [ClientException]s. @override - Future send(BaseRequest request); + Future send(BaseRequest request, {Duration? timeout}); /// Sends a non-streaming [Request] and returns a non-streaming [Response]. Future _sendUnstreamed( String method, Uri url, Map? headers, - [body, Encoding? encoding]) async { + {dynamic body, Encoding? encoding, Duration? timeout}) async { var request = Request(method, url); if (headers != null) request.headers.addAll(headers); @@ -90,7 +110,7 @@ abstract class BaseClient implements Client { } } - return Response.fromStream(await send(request)); + return Response.fromStream(await send(request, timeout: timeout)); } /// Throws an error if [response] is not successful. diff --git a/lib/src/base_request.dart b/lib/src/base_request.dart index 6380cb0bc9..f4ddac68c9 100644 --- a/lib/src/base_request.dart +++ b/lib/src/base_request.dart @@ -117,11 +117,11 @@ abstract class BaseRequest { /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those /// requests. - Future send() async { + Future send({Duration? timeout}) async { var client = Client(); try { - var response = await client.send(this); + var response = await client.send(this, timeout: timeout); var stream = onDone(response.stream, client.close); return StreamedResponse(ByteStream(stream), response.statusCode, contentLength: response.contentLength, diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index d1eb35c1f5..7a411ff8e5 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -41,7 +41,24 @@ class BrowserClient extends BaseClient { /// Sends an HTTP request and asynchronously returns the response. @override - Future send(BaseRequest request) async { + Future send(BaseRequest request, {Duration? timeout}) { + final completer = Completer(); + _send(request, timeout, completer); + return completer.future; + } + + Future _send(BaseRequest request, Duration? timeout, + Completer completer) async { + Timer? timer; + late void Function() onTimeout; + if (timeout != null) { + timer = Timer(timeout, () { + onTimeout(); + }); + onTimeout = () { + completer.completeError(TimeoutException('Request aborted', timeout)); + }; + } var bytes = await request.finalize().toBytes(); var xhr = HttpRequest(); _xhrs.add(xhr); @@ -50,34 +67,36 @@ class BrowserClient extends BaseClient { ..responseType = 'arraybuffer' ..withCredentials = withCredentials; request.headers.forEach(xhr.setRequestHeader); - - var completer = Completer(); + if (timeout != null) { + onTimeout = () { + xhr.abort(); + completer.completeError(TimeoutException('Request aborted', timeout)); + }; + } unawaited(xhr.onLoad.first.then((_) { var body = (xhr.response as ByteBuffer).asUint8List(); + timer?.cancel(); completer.complete(StreamedResponse( ByteStream.fromBytes(body), xhr.status!, contentLength: body.length, request: request, headers: xhr.responseHeaders, reasonPhrase: xhr.statusText)); + _xhrs.remove(xhr); })); unawaited(xhr.onError.first.then((_) { // Unfortunately, the underlying XMLHttpRequest API doesn't expose any // specific information about the error itself. + timer?.cancel(); completer.completeError( ClientException('XMLHttpRequest error.', request.url), StackTrace.current); + _xhrs.remove(xhr); })); xhr.send(bytes); - - try { - return await completer.future; - } finally { - _xhrs.remove(xhr); - } } /// Closes the client. diff --git a/lib/src/client.dart b/lib/src/client.dart index 12695e7123..09a1f568cc 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -34,12 +34,14 @@ abstract class Client { /// Sends an HTTP HEAD request with the given headers to the given URL. /// /// For more fine-grained control over the request, use [send] instead. - Future head(Uri url, {Map? headers}); + Future head(Uri url, + {Map? headers, Duration? timeout}); /// Sends an HTTP GET request with the given headers to the given URL. /// /// For more fine-grained control over the request, use [send] instead. - Future get(Uri url, {Map? headers}); + Future get(Uri url, + {Map? headers, Duration? timeout}); /// Sends an HTTP POST request with the given headers and body to the given /// URL. @@ -60,7 +62,10 @@ abstract class Client { /// /// For more fine-grained control over the request, use [send] instead. Future post(Uri url, - {Map? headers, Object? body, Encoding? encoding}); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}); /// Sends an HTTP PUT request with the given headers and body to the given /// URL. @@ -81,7 +86,10 @@ abstract class Client { /// /// For more fine-grained control over the request, use [send] instead. Future put(Uri url, - {Map? headers, Object? body, Encoding? encoding}); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}); /// Sends an HTTP PATCH request with the given headers and body to the given /// URL. @@ -102,13 +110,19 @@ abstract class Client { /// /// For more fine-grained control over the request, use [send] instead. Future patch(Uri url, - {Map? headers, Object? body, Encoding? encoding}); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}); /// Sends an HTTP DELETE request with the given headers to the given URL. /// /// For more fine-grained control over the request, use [send] instead. Future delete(Uri url, - {Map? headers, Object? body, Encoding? encoding}); + {Map? headers, + Object? body, + Encoding? encoding, + Duration? timeout}); /// Sends an HTTP GET request with the given headers to the given URL and /// returns a Future that completes to the body of the response as a String. @@ -118,7 +132,8 @@ abstract class Client { /// /// For more fine-grained control over the request and response, use [send] or /// [get] instead. - Future read(Uri url, {Map? headers}); + Future read(Uri url, + {Map? headers, Duration? timeout}); /// Sends an HTTP GET request with the given headers to the given URL and /// returns a Future that completes to the body of the response as a list of @@ -129,10 +144,11 @@ abstract class Client { /// /// For more fine-grained control over the request and response, use [send] or /// [get] instead. - Future readBytes(Uri url, {Map? headers}); + Future readBytes(Uri url, + {Map? headers, Duration? timeout}); /// Sends an HTTP request and asynchronously returns the response. - Future send(BaseRequest request); + Future send(BaseRequest request, {Duration? timeout}); /// Closes the client and cleans up any resources associated with it. /// diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 5ee66db0f8..46fd815664 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:async'; import 'dart:io'; import 'base_client.dart'; @@ -23,32 +24,68 @@ class IOClient extends BaseClient { /// Sends an HTTP request and asynchronously returns the response. @override - Future send(BaseRequest request) async { + Future send(BaseRequest request, {Duration? timeout}) { + final completer = Completer(); + _send(request, timeout, completer); + return completer.future; + } + + Future _send(BaseRequest request, Duration? timeout, + Completer completer) async { var stream = request.finalize(); + Timer? timer; + late void Function() onTimeout; + if (timeout != null) { + timer = Timer(timeout, () { + onTimeout(); + }); + onTimeout = () { + completer.completeError(TimeoutException('Request aborted', timeout)); + }; + } try { var ioRequest = (await _inner!.openUrl(request.method, request.url)) ..followRedirects = request.followRedirects ..maxRedirects = request.maxRedirects ..contentLength = (request.contentLength ?? -1) ..persistentConnection = request.persistentConnection; + if (completer.isCompleted) return; request.headers.forEach((name, value) { ioRequest.headers.set(name, value); }); + if (timeout != null) { + onTimeout = () { + ioRequest.abort(); + completer.completeError(TimeoutException('Request aborted', timeout)); + }; + } + var response = await stream.pipe(ioRequest) as HttpClientResponse; + if (completer.isCompleted) return; var headers = {}; response.headers.forEach((key, values) { headers[key] = values.join(','); }); + var responseStream = response.handleError((error) { + final httpException = error as HttpException; + throw ClientException(httpException.message, httpException.uri); + }, test: (error) => error is HttpException).transform>( + StreamTransformer.fromHandlers(handleDone: (sink) { + timer?.cancel(); + sink.close(); + })); + + if (timeout != null) { + onTimeout = () { + // TODO, is this necessary? How will it surface? + response.detachSocket().then((socket) => socket.destroy()); + }; + } - return IOStreamedResponse( - response.handleError((error) { - final httpException = error as HttpException; - throw ClientException(httpException.message, httpException.uri); - }, test: (error) => error is HttpException), - response.statusCode, + completer.complete(IOStreamedResponse(responseStream, response.statusCode, contentLength: response.contentLength == -1 ? null : response.contentLength, request: request, @@ -56,9 +93,13 @@ class IOClient extends BaseClient { isRedirect: response.isRedirect, persistentConnection: response.persistentConnection, reasonPhrase: response.reasonPhrase, - inner: response); + inner: response)); } on HttpException catch (error) { - throw ClientException(error.message, error.uri); + if (completer.isCompleted) return; + completer.completeError(ClientException(error.message, error.uri)); + } catch (error, stackTrace) { + if (completer.isCompleted) return; + completer.completeError(error, stackTrace); } } diff --git a/lib/src/mock_client.dart b/lib/src/mock_client.dart index abfcde20e3..731f0b1378 100644 --- a/lib/src/mock_client.dart +++ b/lib/src/mock_client.dart @@ -65,7 +65,8 @@ class MockClient extends BaseClient { }); @override - Future send(BaseRequest request) async { + Future send(BaseRequest request, + {Duration? timeout}) async { var bodyStream = request.finalize(); return await _handler(request, bodyStream); } From e135dcabee1ddb297aec3d2328f888f0a3cc4d11 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 21 Jan 2021 17:42:26 -0800 Subject: [PATCH 02/11] Remove xhrs in one place --- lib/src/browser_client.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index 7a411ff8e5..cbde7f82a3 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -62,6 +62,9 @@ class BrowserClient extends BaseClient { var bytes = await request.finalize().toBytes(); var xhr = HttpRequest(); _xhrs.add(xhr); + unawaited(completer.future.whenComplete(() { + _xhrs.remove(xhr); + })); xhr ..open(request.method, '${request.url}', async: true) ..responseType = 'arraybuffer' @@ -83,7 +86,6 @@ class BrowserClient extends BaseClient { request: request, headers: xhr.responseHeaders, reasonPhrase: xhr.statusText)); - _xhrs.remove(xhr); })); unawaited(xhr.onError.first.then((_) { @@ -93,7 +95,6 @@ class BrowserClient extends BaseClient { completer.completeError( ClientException('XMLHttpRequest error.', request.url), StackTrace.current); - _xhrs.remove(xhr); })); xhr.send(bytes); From 202af71b43d0711e705fa011e29500bd93cde8c7 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 27 Jan 2021 15:06:26 -0800 Subject: [PATCH 03/11] Complete with error for timeout during detach --- lib/src/io_client.dart | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 46fd815664..4d4dd92476 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -69,22 +69,25 @@ class IOClient extends BaseClient { response.headers.forEach((key, values) { headers[key] = values.join(','); }); + var wasTimedOut = false; + if (timeout != null) { + onTimeout = () { + wasTimedOut = true; + response.detachSocket().then((socket) => socket.destroy()); + }; + } var responseStream = response.handleError((error) { final httpException = error as HttpException; throw ClientException(httpException.message, httpException.uri); }, test: (error) => error is HttpException).transform>( StreamTransformer.fromHandlers(handleDone: (sink) { timer?.cancel(); + if (wasTimedOut) { + sink.addError(TimeoutException('Request aborted', timeout)); + } sink.close(); })); - if (timeout != null) { - onTimeout = () { - // TODO, is this necessary? How will it surface? - response.detachSocket().then((socket) => socket.destroy()); - }; - } - completer.complete(IOStreamedResponse(responseStream, response.statusCode, contentLength: response.contentLength == -1 ? null : response.contentLength, From 55d4cec4684ab745b4172e6f51f12b44c9d16555 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 27 Jan 2021 15:15:03 -0800 Subject: [PATCH 04/11] Docs --- lib/http.dart | 33 +++++++++++++++++++++++++++++++++ lib/src/client.dart | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/lib/http.dart b/lib/http.dart index cefdcd29a0..efc6cbd9ff 100644 --- a/lib/http.dart +++ b/lib/http.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. /// A composable, [Future]-based library for making HTTP requests. +import 'dart:async'; import 'dart:convert'; import 'dart:typed_data'; @@ -27,6 +28,10 @@ export 'src/streamed_response.dart'; /// Sends an HTTP HEAD request with the given headers to the given URL. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// This automatically initializes a new [Client] and closes that client once /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those requests. @@ -39,6 +44,10 @@ Future head(Uri url, /// Sends an HTTP GET request with the given headers to the given URL. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// This automatically initializes a new [Client] and closes that client once /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those requests. @@ -65,6 +74,10 @@ Future get(Uri url, /// /// [encoding] defaults to [utf8]. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future post(Uri url, @@ -91,6 +104,10 @@ Future post(Uri url, /// /// [encoding] defaults to [utf8]. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future put(Uri url, @@ -118,6 +135,10 @@ Future put(Uri url, /// /// [encoding] defaults to [utf8]. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// For more fine-grained control over the request, use [Request] or /// [StreamedRequest] instead. Future patch(Uri url, @@ -130,6 +151,10 @@ Future patch(Uri url, /// Sends an HTTP DELETE request with the given headers to the given URL. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// This automatically initializes a new [Client] and closes that client once /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those requests. @@ -149,6 +174,10 @@ Future delete(Uri url, /// The Future will emit a [ClientException] if the response doesn't have a /// success status code. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// This automatically initializes a new [Client] and closes that client once /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those requests. @@ -167,6 +196,10 @@ Future read(Uri url, /// The Future will emit a [ClientException] if the response doesn't have a /// success status code. /// +/// If [timeout] is not null the request will be aborted if it takes longer than +/// the given duration to complete, and the returned future will complete as an +/// error with a [TimeoutException]. +/// /// This automatically initializes a new [Client] and closes that client once /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those requests. diff --git a/lib/src/client.dart b/lib/src/client.dart index 09a1f568cc..291e39b48f 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:async'; import 'dart:convert'; import 'dart:typed_data'; @@ -33,12 +34,20 @@ abstract class Client { /// Sends an HTTP HEAD request with the given headers to the given URL. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future head(Uri url, {Map? headers, Duration? timeout}); /// Sends an HTTP GET request with the given headers to the given URL. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future get(Uri url, {Map? headers, Duration? timeout}); @@ -60,6 +69,10 @@ abstract class Client { /// /// [encoding] defaults to [utf8]. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future post(Uri url, {Map? headers, @@ -84,6 +97,10 @@ abstract class Client { /// /// [encoding] defaults to [utf8]. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future put(Uri url, {Map? headers, @@ -108,6 +125,10 @@ abstract class Client { /// /// [encoding] defaults to [utf8]. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future patch(Uri url, {Map? headers, @@ -117,6 +138,10 @@ abstract class Client { /// Sends an HTTP DELETE request with the given headers to the given URL. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request, use [send] instead. Future delete(Uri url, {Map? headers, @@ -130,6 +155,10 @@ abstract class Client { /// The Future will emit a [ClientException] if the response doesn't have a /// success status code. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request and response, use [send] or /// [get] instead. Future read(Uri url, @@ -142,12 +171,23 @@ abstract class Client { /// The Future will emit a [ClientException] if the response doesn't have a /// success status code. /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete, and the returned future will complete + /// as an error with a [TimeoutException]. + /// /// For more fine-grained control over the request and response, use [send] or /// [get] instead. Future readBytes(Uri url, {Map? headers, Duration? timeout}); /// Sends an HTTP request and asynchronously returns the response. + /// + /// If [timeout] is not null the request will be aborted if it takes longer + /// than the given duration to complete. If the timeout occurs before any + /// reply is received from the server the returned future will as an error + /// with a [TimeoutException]. If the timout occurs after the reply has been + /// started but before the entire body has been read the response stream will + /// emit a [TimeoutException] and close. Future send(BaseRequest request, {Duration? timeout}); /// Closes the client and cleans up any resources associated with it. From a8c31e57ec191566ea5ab2de01f6bb8a43f85db5 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 27 Jan 2021 15:22:55 -0800 Subject: [PATCH 05/11] Avoid late, use nullable request to abort --- lib/src/browser_client.dart | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index cbde7f82a3..3463821309 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -50,17 +50,15 @@ class BrowserClient extends BaseClient { Future _send(BaseRequest request, Duration? timeout, Completer completer) async { Timer? timer; - late void Function() onTimeout; + HttpRequest? toAbort; if (timeout != null) { timer = Timer(timeout, () { - onTimeout(); - }); - onTimeout = () { + toAbort?.abort(); completer.completeError(TimeoutException('Request aborted', timeout)); - }; + }); } var bytes = await request.finalize().toBytes(); - var xhr = HttpRequest(); + var xhr = toAbort = HttpRequest(); _xhrs.add(xhr); unawaited(completer.future.whenComplete(() { _xhrs.remove(xhr); @@ -70,12 +68,6 @@ class BrowserClient extends BaseClient { ..responseType = 'arraybuffer' ..withCredentials = withCredentials; request.headers.forEach(xhr.setRequestHeader); - if (timeout != null) { - onTimeout = () { - xhr.abort(); - completer.completeError(TimeoutException('Request aborted', timeout)); - }; - } unawaited(xhr.onLoad.first.then((_) { var body = (xhr.response as ByteBuffer).asUint8List(); From c742e59746e480d1b6142426ae52080b50154518 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 27 Jan 2021 15:34:58 -0800 Subject: [PATCH 06/11] All positional parameters --- lib/src/base_client.dart | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/src/base_client.dart b/lib/src/base_client.dart index dd4f7112c1..fb5bd91c44 100644 --- a/lib/src/base_client.dart +++ b/lib/src/base_client.dart @@ -21,12 +21,12 @@ abstract class BaseClient implements Client { @override Future head(Uri url, {Map? headers, Duration? timeout}) => - _sendUnstreamed('HEAD', url, headers, timeout: timeout); + _sendUnstreamed('HEAD', url, headers, null, null, timeout); @override Future get(Uri url, {Map? headers, Duration? timeout}) => - _sendUnstreamed('GET', url, headers, timeout: timeout); + _sendUnstreamed('GET', url, headers, null, null, timeout); @override Future post(Uri url, @@ -34,8 +34,7 @@ abstract class BaseClient implements Client { Object? body, Encoding? encoding, Duration? timeout}) => - _sendUnstreamed('POST', url, headers, - body: body, encoding: encoding, timeout: timeout); + _sendUnstreamed('POST', url, headers, body, encoding, timeout); @override Future put(Uri url, @@ -43,8 +42,7 @@ abstract class BaseClient implements Client { Object? body, Encoding? encoding, Duration? timeout}) => - _sendUnstreamed('PUT', url, headers, - body: body, encoding: encoding, timeout: timeout); + _sendUnstreamed('PUT', url, headers, body, encoding, timeout); @override Future patch(Uri url, @@ -52,8 +50,7 @@ abstract class BaseClient implements Client { Object? body, Encoding? encoding, Duration? timeout}) => - _sendUnstreamed('PATCH', url, headers, - body: body, encoding: encoding, timeout: timeout); + _sendUnstreamed('PATCH', url, headers, body, encoding, timeout); @override Future delete(Uri url, @@ -61,8 +58,7 @@ abstract class BaseClient implements Client { Object? body, Encoding? encoding, Duration? timeout}) => - _sendUnstreamed('DELETE', url, headers, - body: body, encoding: encoding, timeout: timeout); + _sendUnstreamed('DELETE', url, headers, body, encoding, timeout); @override Future read(Uri url, @@ -92,8 +88,12 @@ abstract class BaseClient implements Client { /// Sends a non-streaming [Request] and returns a non-streaming [Response]. Future _sendUnstreamed( - String method, Uri url, Map? headers, - {dynamic body, Encoding? encoding, Duration? timeout}) async { + String method, + Uri url, + Map? headers, + dynamic body, + Encoding? encoding, + Duration? timeout) async { var request = Request(method, url); if (headers != null) request.headers.addAll(headers); From b94a86a95bfc4b166252d47888757d3f870dedef Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 27 Jan 2021 17:00:36 -0800 Subject: [PATCH 07/11] Ignore future errors --- lib/src/browser_client.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index 3463821309..b77da8dacd 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -60,9 +60,12 @@ class BrowserClient extends BaseClient { var bytes = await request.finalize().toBytes(); var xhr = toAbort = HttpRequest(); _xhrs.add(xhr); - unawaited(completer.future.whenComplete(() { - _xhrs.remove(xhr); - })); + unawaited(completer.future + .whenComplete(() { + _xhrs.remove(xhr); + }) + .then((_) {}) + .catchError((_) {})); xhr ..open(request.method, '${request.url}', async: true) ..responseType = 'arraybuffer' From 0fe2253f87d2b953b0dc760ed4a3635cd33ba078 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 28 Jan 2021 08:58:16 -0800 Subject: [PATCH 08/11] Avoid late --- lib/src/io_client.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 4d4dd92476..fe2f84938d 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -35,14 +35,14 @@ class IOClient extends BaseClient { var stream = request.finalize(); Timer? timer; - late void Function() onTimeout; + void Function() onTimeout; if (timeout != null) { - timer = Timer(timeout, () { - onTimeout(); - }); onTimeout = () { completer.completeError(TimeoutException('Request aborted', timeout)); }; + timer = Timer(timeout, () { + onTimeout(); + }); } try { var ioRequest = (await _inner!.openUrl(request.method, request.url)) From 4f3dd7150086fc76c2d4b420def0ca70fc11bbad Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 28 Jan 2021 11:23:58 -0800 Subject: [PATCH 09/11] Rename to contentTimeout --- lib/src/base_client.dart | 5 +++-- lib/src/base_request.dart | 12 ++++++++++-- lib/src/browser_client.dart | 5 +++-- lib/src/client.dart | 15 ++++++++------- lib/src/io_client.dart | 23 +++++++++++++---------- lib/src/mock_client.dart | 2 +- 6 files changed, 38 insertions(+), 24 deletions(-) diff --git a/lib/src/base_client.dart b/lib/src/base_client.dart index fb5bd91c44..8e1445a1f7 100644 --- a/lib/src/base_client.dart +++ b/lib/src/base_client.dart @@ -84,7 +84,8 @@ abstract class BaseClient implements Client { /// later point, or it could already be closed when it's returned. Any /// internal HTTP errors should be wrapped as [ClientException]s. @override - Future send(BaseRequest request, {Duration? timeout}); + Future send(BaseRequest request, + {Duration? contentTimeout}); /// Sends a non-streaming [Request] and returns a non-streaming [Response]. Future _sendUnstreamed( @@ -110,7 +111,7 @@ abstract class BaseClient implements Client { } } - return Response.fromStream(await send(request, timeout: timeout)); + return Response.fromStream(await send(request, contentTimeout: timeout)); } /// Throws an error if [response] is not successful. diff --git a/lib/src/base_request.dart b/lib/src/base_request.dart index f4ddac68c9..156e20d85e 100644 --- a/lib/src/base_request.dart +++ b/lib/src/base_request.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:async'; import 'dart:collection'; import 'package:meta/meta.dart'; @@ -117,11 +118,18 @@ abstract class BaseRequest { /// the request is complete. If you're planning on making multiple requests to /// the same server, you should use a single [Client] for all of those /// requests. - Future send({Duration? timeout}) async { + /// + /// If [contentTimeout] is not null the request will be aborted if it takes + /// longer than the given duration to receive the entire response. If the + /// timeout occurs before any reply is received from the server the returned + /// future will as an error with a [TimeoutException]. If the timout occurs + /// after the reply has been started but before the entire body has been read + /// the response stream will emit a [TimeoutException] and close. + Future send({Duration? contentTimeout}) async { var client = Client(); try { - var response = await client.send(this, timeout: timeout); + var response = await client.send(this, contentTimeout: contentTimeout); var stream = onDone(response.stream, client.close); return StreamedResponse(ByteStream(stream), response.statusCode, contentLength: response.contentLength, diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index b77da8dacd..46ad363e0a 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -41,9 +41,10 @@ class BrowserClient extends BaseClient { /// Sends an HTTP request and asynchronously returns the response. @override - Future send(BaseRequest request, {Duration? timeout}) { + Future send(BaseRequest request, + {Duration? contentTimeout}) { final completer = Completer(); - _send(request, timeout, completer); + _send(request, contentTimeout, completer); return completer.future; } diff --git a/lib/src/client.dart b/lib/src/client.dart index 291e39b48f..ba162dbb2f 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -182,13 +182,14 @@ abstract class Client { /// Sends an HTTP request and asynchronously returns the response. /// - /// If [timeout] is not null the request will be aborted if it takes longer - /// than the given duration to complete. If the timeout occurs before any - /// reply is received from the server the returned future will as an error - /// with a [TimeoutException]. If the timout occurs after the reply has been - /// started but before the entire body has been read the response stream will - /// emit a [TimeoutException] and close. - Future send(BaseRequest request, {Duration? timeout}); + /// If [contentTimeout] is not null the request will be aborted if it takes + /// longer than the given duration to receive the entire response. If the + /// timeout occurs before any reply is received from the server the returned + /// future will as an error with a [TimeoutException]. If the timout occurs + /// after the reply has been started but before the entire body has been read + /// the response stream will emit a [TimeoutException] and close. + Future send(BaseRequest request, {Duration? + contentTimeout}); /// Closes the client and cleans up any resources associated with it. /// diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index fe2f84938d..90809fe53f 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -24,23 +24,25 @@ class IOClient extends BaseClient { /// Sends an HTTP request and asynchronously returns the response. @override - Future send(BaseRequest request, {Duration? timeout}) { + Future send(BaseRequest request, + {Duration? contentTimeout}) { final completer = Completer(); - _send(request, timeout, completer); + _send(request, contentTimeout, completer); return completer.future; } - Future _send(BaseRequest request, Duration? timeout, + Future _send(BaseRequest request, Duration? contentTimeout, Completer completer) async { var stream = request.finalize(); Timer? timer; void Function() onTimeout; - if (timeout != null) { + if (contentTimeout != null) { onTimeout = () { - completer.completeError(TimeoutException('Request aborted', timeout)); + completer + .completeError(TimeoutException('Request aborted', contentTimeout)); }; - timer = Timer(timeout, () { + timer = Timer(contentTimeout, () { onTimeout(); }); } @@ -55,10 +57,11 @@ class IOClient extends BaseClient { ioRequest.headers.set(name, value); }); - if (timeout != null) { + if (contentTimeout != null) { onTimeout = () { ioRequest.abort(); - completer.completeError(TimeoutException('Request aborted', timeout)); + completer.completeError( + TimeoutException('Request aborted', contentTimeout)); }; } @@ -70,7 +73,7 @@ class IOClient extends BaseClient { headers[key] = values.join(','); }); var wasTimedOut = false; - if (timeout != null) { + if (contentTimeout != null) { onTimeout = () { wasTimedOut = true; response.detachSocket().then((socket) => socket.destroy()); @@ -83,7 +86,7 @@ class IOClient extends BaseClient { StreamTransformer.fromHandlers(handleDone: (sink) { timer?.cancel(); if (wasTimedOut) { - sink.addError(TimeoutException('Request aborted', timeout)); + sink.addError(TimeoutException('Request aborted', contentTimeout)); } sink.close(); })); diff --git a/lib/src/mock_client.dart b/lib/src/mock_client.dart index 731f0b1378..09312062c0 100644 --- a/lib/src/mock_client.dart +++ b/lib/src/mock_client.dart @@ -66,7 +66,7 @@ class MockClient extends BaseClient { @override Future send(BaseRequest request, - {Duration? timeout}) async { + {Duration? contentTimeout}) async { var bodyStream = request.finalize(); return await _handler(request, bodyStream); } From 329f6f47ff800b59b6c4aa4e0245189711fb23fc Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 28 Jan 2021 12:21:31 -0800 Subject: [PATCH 10/11] Guard all completions with checks --- lib/src/io_client.dart | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/src/io_client.dart b/lib/src/io_client.dart index 90809fe53f..590b831e88 100644 --- a/lib/src/io_client.dart +++ b/lib/src/io_client.dart @@ -39,8 +39,10 @@ class IOClient extends BaseClient { void Function() onTimeout; if (contentTimeout != null) { onTimeout = () { - completer - .completeError(TimeoutException('Request aborted', contentTimeout)); + if (!completer.isCompleted) { + completer.completeError( + TimeoutException('Request aborted', contentTimeout)); + } }; timer = Timer(contentTimeout, () { onTimeout(); @@ -60,8 +62,10 @@ class IOClient extends BaseClient { if (contentTimeout != null) { onTimeout = () { ioRequest.abort(); - completer.completeError( - TimeoutException('Request aborted', contentTimeout)); + if (!completer.isCompleted) { + completer.completeError( + TimeoutException('Request aborted', contentTimeout)); + } }; } @@ -91,15 +95,18 @@ class IOClient extends BaseClient { sink.close(); })); - completer.complete(IOStreamedResponse(responseStream, response.statusCode, - contentLength: - response.contentLength == -1 ? null : response.contentLength, - request: request, - headers: headers, - isRedirect: response.isRedirect, - persistentConnection: response.persistentConnection, - reasonPhrase: response.reasonPhrase, - inner: response)); + if (!completer.isCompleted) { + completer.complete(IOStreamedResponse( + responseStream, response.statusCode, + contentLength: + response.contentLength == -1 ? null : response.contentLength, + request: request, + headers: headers, + isRedirect: response.isRedirect, + persistentConnection: response.persistentConnection, + reasonPhrase: response.reasonPhrase, + inner: response)); + } } on HttpException catch (error) { if (completer.isCompleted) return; completer.completeError(ClientException(error.message, error.uri)); From bea9f640fda70db7defa2e8e3adc4275d871408d Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 28 Jan 2021 12:22:46 -0800 Subject: [PATCH 11/11] Guard all completions with checks --- lib/src/browser_client.dart | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/src/browser_client.dart b/lib/src/browser_client.dart index 46ad363e0a..123b51ec76 100644 --- a/lib/src/browser_client.dart +++ b/lib/src/browser_client.dart @@ -55,7 +55,9 @@ class BrowserClient extends BaseClient { if (timeout != null) { timer = Timer(timeout, () { toAbort?.abort(); - completer.completeError(TimeoutException('Request aborted', timeout)); + if (!completer.isCompleted) { + completer.completeError(TimeoutException('Request aborted', timeout)); + } }); } var bytes = await request.finalize().toBytes(); @@ -76,21 +78,25 @@ class BrowserClient extends BaseClient { unawaited(xhr.onLoad.first.then((_) { var body = (xhr.response as ByteBuffer).asUint8List(); timer?.cancel(); - completer.complete(StreamedResponse( - ByteStream.fromBytes(body), xhr.status!, - contentLength: body.length, - request: request, - headers: xhr.responseHeaders, - reasonPhrase: xhr.statusText)); + if (!completer.isCompleted) { + completer.complete(StreamedResponse( + ByteStream.fromBytes(body), xhr.status!, + contentLength: body.length, + request: request, + headers: xhr.responseHeaders, + reasonPhrase: xhr.statusText)); + } })); unawaited(xhr.onError.first.then((_) { // Unfortunately, the underlying XMLHttpRequest API doesn't expose any // specific information about the error itself. timer?.cancel(); - completer.completeError( - ClientException('XMLHttpRequest error.', request.url), - StackTrace.current); + if (!completer.isCompleted) { + completer.completeError( + ClientException('XMLHttpRequest error.', request.url), + StackTrace.current); + } })); xhr.send(bytes);