Skip to content

✨ Introduce ClosableBaseClient #1795

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkgs/cronet_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.5.0-wip

* Implements `ClosableBaseClient`.
* Expose `CronetEngine.close` and `CronetClient.close`.

## 1.4.0

* Add a new `CronetStreamedResponse` class that provides additional information
Expand Down
9 changes: 7 additions & 2 deletions pkgs/cronet_http/lib/src/cronet_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ enum CacheMode {
/// An environment that can be used to make HTTP requests.
class CronetEngine {
late final jb.CronetEngine _engine;

bool get isClosed => _isClosed;
bool _isClosed = false;

CronetEngine._(this._engine);
Expand Down Expand Up @@ -351,9 +353,12 @@ jb.UrlRequestCallbackProxy$UrlRequestCallbackInterface _urlRequestCallbacks(
/// A HTTP [Client] based on the
/// [Cronet](https://developer.android.com/guide/topics/connectivity/cronet)
/// network stack.
class CronetClient extends BaseClient {
class CronetClient extends ClosableBaseClient {
static final _executor = jb.Executors.newCachedThreadPool();
CronetEngine? _engine;

@override
bool get isClosed => _isClosed;
bool _isClosed = false;

/// Indicates that [CronetClient] is responsible for closing [_engine].
Expand Down Expand Up @@ -414,7 +419,7 @@ class CronetClient extends BaseClient {
final engine = _engine ?? CronetEngine.build();
_engine = engine;

if (engine._isClosed) {
if (engine.isClosed) {
throw ClientException(
'HTTP request failed. CronetEngine is already closed.', request.url);
}
Expand Down
5 changes: 5 additions & 0 deletions pkgs/cupertino_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 2.4.0-wip

* Implements `ClosableBaseClient`.
* Expose `CronetEngine.close` and `CronetClient.close`.

## 2.3.0-wip

* Add the ability to abort requests.
Expand Down
4 changes: 3 additions & 1 deletion pkgs/cupertino_http/lib/src/cupertino_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,11 @@ class _TaskTracker {
/// }
/// }
/// ```
class CupertinoClient extends BaseClient {
class CupertinoClient extends ClosableBaseClient {
static final Map<URLSessionTask, _TaskTracker> _tasks = {};

@override
bool get isClosed => _urlSession == null;
URLSession? _urlSession;

CupertinoClient._(this._urlSession);
Expand Down
5 changes: 5 additions & 0 deletions pkgs/http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.5.0-wip

* Introduce `ClosableBaseClient` specifically for clients that can be closed.## 0.2.0-wip
* Expose `IOClient.close` and `BrowserClient.close`.

## 1.5.0-beta.2

* Fixed a bug in `IOClient` where the `HttpClient`'s response stream was
Expand Down
7 changes: 7 additions & 0 deletions pkgs/http/lib/src/base_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,10 @@ abstract mixin class BaseClient implements Client {
@override
void close() {}
}

abstract class ClosableBaseClient extends BaseClient {
bool get isClosed;

@override
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need to be here?

void close();
}
5 changes: 4 additions & 1 deletion pkgs/http/lib/src/browser_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,17 @@ external JSPromise<Response> _fetch(
///
/// Responses are streamed but requests are not. A request will only be sent
/// once all the data is available.
class BrowserClient extends BaseClient {
class BrowserClient extends ClosableBaseClient {
/// Whether to send credentials such as cookies or authorization headers for
/// cross-site requests.
///
/// Defaults to `false`.
bool withCredentials = false;

@override
bool get isClosed => _isClosed;
bool _isClosed = false;

final _openRequestAbortControllers = <AbortController>[];

/// Sends an HTTP request and asynchronously returns the response.
Expand Down
9 changes: 7 additions & 2 deletions pkgs/http/lib/src/io_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,14 @@ class _IOStreamedResponseV2 extends IOStreamedResponse
/// // would be caught by this handler.
/// }
/// ```
class IOClient extends BaseClient {
class IOClient extends ClosableBaseClient {
/// The underlying `dart:io` HTTP client.
HttpClient? _inner;

@override
bool get isClosed => _isClosed;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't this just be => _inner == null?

bool _isClosed = false;

/// Create a new `dart:io`-based HTTP [Client].
///
/// If [inner] is provided then it can be used to provide configuration
Expand All @@ -103,7 +107,7 @@ class IOClient extends BaseClient {
/// Sends an HTTP request and asynchronously returns the response.
@override
Future<IOStreamedResponse> send(BaseRequest request) async {
if (_inner == null) {
if (_inner == null || _isClosed) {
throw ClientException(
'HTTP request failed. Client is already closed.', request.url);
}
Expand Down Expand Up @@ -243,5 +247,6 @@ class IOClient extends BaseClient {
_inner!.close(force: true);
_inner = null;
}
_isClosed = true;
}
}
5 changes: 5 additions & 0 deletions pkgs/ok_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.2.0-wip

* Implements `ClosableBaseClient`.
* Expose `OkHttpClient.close`.

## 0.1.1-wip

- `OkHttpClient` now receives an `OkHttpClientConfiguration` to configure the client on a per-call basis.
Expand Down
5 changes: 4 additions & 1 deletion pkgs/ok_http/lib/src/ok_http_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,11 @@ Future<String?> choosePrivateKeyAlias({
/// }
/// }
/// ```
class OkHttpClient extends BaseClient {
class OkHttpClient extends ClosableBaseClient {
late bindings.OkHttpClient _client;

@override
bool get isClosed => _isClosed;
bool _isClosed = false;

/// The configuration for this client, applied on a per-call basis.
Expand Down
Loading