Skip to content
Merged
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
1 change: 1 addition & 0 deletions plugins/native_dio_adapter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- Support request cancellation for native HTTP clients via use of `AbortableRequest` (introduced in http package from version 1.5.0)
- Add timeout handling for `sendTimeout`, `connectTimeout`, and `receiveTimeout` in `ConversionLayerAdapter`

## 1.5.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,57 @@ class ConversionLayerAdapter implements HttpClientAdapter {
Future<ResponseBody> fetch(
RequestOptions options,
Stream<Uint8List>? requestStream,
Future<dynamic>? cancelFuture,
Future<void>? cancelFuture,
) async {
final request = await _fromOptionsAndStream(
final timeoutCompleter = Completer<void>();
Copy link
Member

Choose a reason for hiding this comment

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

Since all onTimeout throws DioException, is the completer still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's required. The completer is used to trigger the AbortableRequest's cancellation logic. The throw just handles reporting the error to the caller. We must signal the cancellation first, then throw.


final cancelToken = cancelFuture != null
? Future.any([cancelFuture, timeoutCompleter.future])
: timeoutCompleter.future;
final requestFuture = _fromOptionsAndStream(
options,
requestStream,
cancelFuture,
cancelToken,
);
final response = await client.send(request);

final sendTimeout = options.sendTimeout ?? Duration.zero;
final BaseRequest request;
if (sendTimeout == Duration.zero) {
request = await requestFuture;
} else {
request = await requestFuture.timeout(
sendTimeout,
onTimeout: () {
timeoutCompleter.complete();
throw DioException.sendTimeout(
timeout: sendTimeout,
requestOptions: options,
);
},
);
}

// http package doesn't separate connect and receive phases,
// so we combine both timeouts for client.send()
final connectTimeout = options.connectTimeout ?? Duration.zero;
final receiveTimeout = options.receiveTimeout ?? Duration.zero;
final totalTimeout = connectTimeout + receiveTimeout;
Comment on lines +54 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, it might be good to exclude connectionTimeout from support.

final StreamedResponse response;
if (totalTimeout == Duration.zero) {
response = await client.send(request);
} else {
response = await client.send(request).timeout(
totalTimeout,
onTimeout: () {
timeoutCompleter.complete();
throw DioException.receiveTimeout(
timeout: totalTimeout,
requestOptions: options,
);
},
);
}

return response.toDioResponseBody(options);
}

Expand All @@ -38,7 +81,7 @@ class ConversionLayerAdapter implements HttpClientAdapter {
Future<BaseRequest> _fromOptionsAndStream(
RequestOptions options,
Stream<Uint8List>? requestStream,
Future<dynamic>? cancelFuture,
Future<void> cancelFuture,
) async {
final request = AbortableRequest(
options.method,
Expand Down
23 changes: 23 additions & 0 deletions plugins/native_dio_adapter/test/client_mock.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,26 @@ class AbortClientMock implements Client {
}

class AbortedError extends Error {}

class DelayedClientMock implements Client {
DelayedClientMock({
required this.duration,
});

final Duration duration;

@override
Future<StreamedResponse> send(BaseRequest request) async {
await Future<void>.delayed(duration);

return StreamedResponse(
Stream.fromIterable([]),
200,
);
}

@override
void noSuchMethod(Invocation invocation) {
throw UnimplementedError();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void main() {
expect(await resp.stream.length, 5);
});

test('request cancellation', () {
test('request cancellation', () async {
final mock = AbortClientMock();
final cla = ConversionLayerAdapter(mock);
final cancelToken = CancelToken();
Expand All @@ -74,7 +74,7 @@ void main() {
},
);

expectLater(
await expectLater(
() => cla.fetch(
RequestOptions(path: ''),
null,
Expand Down Expand Up @@ -110,4 +110,99 @@ void main() {
);
expect(mock.isRequestCanceled, true);
});

group('Timeout tests', () {
test('sendTimeout throws DioException.sendTimeout', () async {
final mock = ClientMock()
..response = StreamedResponse(const Stream.empty(), 200);
final cla = ConversionLayerAdapter(mock);

final delayedStream = Stream<Uint8List>.periodic(
const Duration(milliseconds: 10),
(count) => Uint8List.fromList([count]),
);

try {
await cla.fetch(
RequestOptions(
path: '',
sendTimeout: const Duration(milliseconds: 1),
),
delayedStream,
null,
);
fail('Should have thrown DioException');
} on DioException catch (e) {
expect(e.type, DioExceptionType.sendTimeout);
expect(e.message, contains('1'));
}
});

test('receiveTimeout throws DioException.receiveTimeout', () async {
final mock = DelayedClientMock(
duration: const Duration(milliseconds: 10),
);
final cla = ConversionLayerAdapter(mock);

try {
await cla.fetch(
RequestOptions(
path: '',
receiveTimeout: const Duration(milliseconds: 1),
),
null,
null,
);
fail('Should have thrown DioException');
} on DioException catch (e) {
expect(e.type, DioExceptionType.receiveTimeout);
expect(e.message, contains('1'));
}
});

test('connectTimeout and receiveTimeout are combined', () async {
final mock = DelayedClientMock(
duration: const Duration(milliseconds: 10),
);
final cla = ConversionLayerAdapter(mock);

try {
await cla.fetch(
RequestOptions(
path: '',
connectTimeout: const Duration(milliseconds: 1),
receiveTimeout: const Duration(milliseconds: 1),
),
null,
null,
);
fail('Should have thrown DioException');
} on DioException catch (e) {
expect(e.type, DioExceptionType.receiveTimeout);
expect(e.message, contains('2'));
}
});

test('AbortableRequest is triggered on receiveTimeout', () async {
final mock = AbortClientMock();
final cla = ConversionLayerAdapter(mock);

try {
await cla.fetch(
RequestOptions(
path: '',
receiveTimeout: const Duration(milliseconds: 1),
),
null,
null,
);
fail('Should have thrown DioException');
} on DioException catch (e) {
expect(e.type, DioExceptionType.receiveTimeout);
// Give delay for the abortTrigger callback to execute
await Future<void>.delayed(Duration.zero);
expect(mock.isRequestCanceled, isTrue);
}
});
});
}