Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for request timeout #521

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Add support for request timeout #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
847ba46
Add support for request timeout
natebosch Jan 21, 2021
e135dca
Remove xhrs in one place
natebosch Jan 22, 2021
202af71
Complete with error for timeout during detach
natebosch Jan 27, 2021
55d4cec
Docs
natebosch Jan 27, 2021
a8c31e5
Avoid late, use nullable request to abort
natebosch Jan 27, 2021
c742e59
All positional parameters
natebosch Jan 27, 2021
b94a86a
Ignore future errors
natebosch Jan 28, 2021
0fe2253
Avoid late
natebosch Jan 28, 2021
4f3dd71
Rename to contentTimeout
natebosch Jan 28, 2021
329f6f4
Guard all completions with checks
natebosch Jan 28, 2021
bea9f64
Guard all completions with checks
natebosch Jan 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
93 changes: 73 additions & 20 deletions lib/http.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -27,23 +28,35 @@ 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.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> head(Uri url, {Map<String, String>? headers}) =>
_withClient((client) => client.head(url, headers: headers));
Future<Response> head(Uri url,
{Map<String, String>? headers, Duration? timeout}) =>
natebosch marked this conversation as resolved.
Show resolved Hide resolved
_withClient(
(client) => client.head(url, headers: headers, timeout: 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].
///
/// 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.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> get(Uri url, {Map<String, String>? headers}) =>
_withClient((client) => client.get(url, headers: headers));
Future<Response> get(Uri url,
{Map<String, String>? 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.
///
Expand All @@ -61,12 +74,19 @@ Future<Response> get(Uri url, {Map<String, String>? headers}) =>
///
/// [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<Response> post(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.post(url, headers: headers, body: body, encoding: encoding));
{Map<String, String>? 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.
///
Expand All @@ -84,12 +104,19 @@ Future<Response> 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<Response> put(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.put(url, headers: headers, body: body, encoding: encoding));
{Map<String, String>? 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.
Expand All @@ -108,39 +135,59 @@ Future<Response> 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<Response> patch(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.patch(url, headers: headers, body: body, encoding: encoding));
{Map<String, String>? 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.
///
/// 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.
///
/// For more fine-grained control over the request, use [Request] instead.
Future<Response> delete(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_withClient((client) =>
client.delete(url, headers: headers, body: body, encoding: encoding));
{Map<String, String>? 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].
///
/// 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.
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<String> read(Uri url, {Map<String, String>? headers}) =>
_withClient((client) => client.read(url, headers: headers));
Future<String> read(Uri url,
{Map<String, String>? 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
Expand All @@ -149,14 +196,20 @@ Future<String> read(Uri url, {Map<String, String>? headers}) =>
/// 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.
///
/// For more fine-grained control over the request and response, use [Request]
/// instead.
Future<Uint8List> readBytes(Uri url, {Map<String, String>? headers}) =>
_withClient((client) => client.readBytes(url, headers: headers));
Future<Uint8List> readBytes(Uri url,
{Map<String, String>? headers, Duration? timeout}) =>
_withClient(
(client) => client.readBytes(url, headers: headers, timeout: timeout));

Future<T> _withClient<T>(Future<T> Function(Client) fn) async {
var client = Client();
Expand Down
61 changes: 41 additions & 20 deletions lib/src/base_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,59 @@ import 'streamed_response.dart';
/// maybe [close], and then they get various convenience methods for free.
abstract class BaseClient implements Client {
@override
Future<Response> head(Uri url, {Map<String, String>? headers}) =>
_sendUnstreamed('HEAD', url, headers);
Future<Response> head(Uri url,
{Map<String, String>? headers, Duration? timeout}) =>
_sendUnstreamed('HEAD', url, headers, null, null, timeout);

@override
Future<Response> get(Uri url, {Map<String, String>? headers}) =>
_sendUnstreamed('GET', url, headers);
Future<Response> get(Uri url,
{Map<String, String>? headers, Duration? timeout}) =>
_sendUnstreamed('GET', url, headers, null, null, timeout);

@override
Future<Response> post(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('POST', url, headers, body, encoding);
{Map<String, String>? headers,
Object? body,
Encoding? encoding,
Duration? timeout}) =>
_sendUnstreamed('POST', url, headers, body, encoding, timeout);

@override
Future<Response> put(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('PUT', url, headers, body, encoding);
{Map<String, String>? headers,
Object? body,
Encoding? encoding,
Duration? timeout}) =>
_sendUnstreamed('PUT', url, headers, body, encoding, timeout);

@override
Future<Response> patch(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('PATCH', url, headers, body, encoding);
{Map<String, String>? headers,
Object? body,
Encoding? encoding,
Duration? timeout}) =>
_sendUnstreamed('PATCH', url, headers, body, encoding, timeout);

@override
Future<Response> delete(Uri url,
{Map<String, String>? headers, Object? body, Encoding? encoding}) =>
_sendUnstreamed('DELETE', url, headers, body, encoding);
{Map<String, String>? headers,
Object? body,
Encoding? encoding,
Duration? timeout}) =>
_sendUnstreamed('DELETE', url, headers, body, encoding, timeout);

@override
Future<String> read(Uri url, {Map<String, String>? headers}) async {
final response = await get(url, headers: headers);
Future<String> read(Uri url,
{Map<String, String>? headers, Duration? timeout}) async {
final response = await get(url, headers: headers, timeout: timeout);
_checkResponseSuccess(url, response);
return response.body;
}

@override
Future<Uint8List> readBytes(Uri url, {Map<String, String>? headers}) async {
final response = await get(url, headers: headers);
Future<Uint8List> readBytes(Uri url,
{Map<String, String>? headers, Duration? timeout}) async {
final response = await get(url, headers: headers, timeout: timeout);
_checkResponseSuccess(url, response);
return response.bodyBytes;
}
Expand All @@ -68,12 +84,17 @@ 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<StreamedResponse> send(BaseRequest request);
Future<StreamedResponse> send(BaseRequest request,
{Duration? contentTimeout});

/// Sends a non-streaming [Request] and returns a non-streaming [Response].
Future<Response> _sendUnstreamed(
String method, Uri url, Map<String, String>? headers,
[body, Encoding? encoding]) async {
String method,
Uri url,
Map<String, String>? headers,
dynamic body,
Encoding? encoding,
Duration? timeout) async {
var request = Request(method, url);

if (headers != null) request.headers.addAll(headers);
Expand All @@ -90,7 +111,7 @@ abstract class BaseClient implements Client {
}
}

return Response.fromStream(await send(request));
return Response.fromStream(await send(request, contentTimeout: timeout));
}

/// Throws an error if [response] is not successful.
Expand Down
12 changes: 10 additions & 2 deletions lib/src/base_request.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<StreamedResponse> send() async {
///
/// If [contentTimeout] is not null the request will be aborted if it takes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd usually say "If [contentTimeout] is provided, the ...."
I know you can technically provide a null argument, but users will understand it anyway. After all, you talk about the duration afterwards, and null is not a duration.

/// longer than the given duration to receive the entire response. If the
/// timeout occurs before any reply is received from the server the returned
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comma after "server"?

/// future will as an error with a [TimeoutException]. If the timout occurs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a "will complete as" here?

I'd usually write "the returned future will complete with a [TimeoutException] error."

/// after the reply has been started but before the entire body has been read
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comma before "but"? (And then probably also after "read").

Maybe "has been started" -> "has started".

Copy link

@DeD1rk DeD1rk Jul 13, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comma before "but"? (And then probably also after "read").

Don't think so, the '... but ...' is a single description of a condition, whereas a comma would suggest the two parts being separate sentences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read "but before the entire body has been read" as parenthetical.
If it's not a parenthetical clause, it deserves to be equally prominent to the main clause, and I'd change but to and.
I'd still have a comma after read in either case.

/// the response stream will emit a [TimeoutException] and close.
Future<StreamedResponse> send({Duration? contentTimeout}) async {
var client = Client();

try {
var response = await client.send(this);
var response = await client.send(this, contentTimeout: contentTimeout);
var stream = onDone(response.stream, client.close);
return StreamedResponse(ByteStream(stream), response.statusCode,
contentLength: response.contentLength,
Expand Down