Skip to content

Commit

Permalink
refactor(neon_framework): let NeonClient handle the http timeouts
Browse files Browse the repository at this point in the history 
Signed-off-by: Nikolas Rimikis <leptopoda@users.noreply.github.com>
  • Loading branch information
@Leptopoda
Leptopoda committed Jul 23, 2024
1 parent 17e46ff commit bc87322
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 186 deletions.
1 change: 1 addition & 0 deletions packages/neon_framework/lib/src/models/account.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ abstract class Account implements Credentials, Findable, Built<Account, AccountB
cookieStore: cookieStore,
userAgent: userAgent,
client: this.httpClient,
timeLimit: kDefaultTimeout,
);

return NextcloudClient(
Expand Down
50 changes: 16 additions & 34 deletions packages/neon_framework/lib/src/utils/request_manager.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:neon_framework/models.dart';
import 'package:neon_framework/src/bloc/result.dart';
import 'package:neon_framework/src/models/account.dart';
import 'package:neon_framework/storage.dart';
import 'package:neon_http_client/neon_http_client.dart';
import 'package:nextcloud/utils.dart';
import 'package:nextcloud/webdav.dart';
import 'package:rxdart/rxdart.dart';
Expand Down Expand Up @@ -38,11 +39,6 @@ typedef DeserializeCallback<T> = T Function(Uint8List);
/// range or if the request has timed out.
const kMaxTries = 3;

/// The default timeout for requests.
///
/// Requests that take longer than this duration will be canceled.
const kDefaultTimeout = Duration(seconds: 30);

/// A singleton class that handles requests to the Nextcloud API.
///
/// Requests need to be made through the [nextcloud](https://pub.dev/packages/nextcloud)
Expand Down Expand Up @@ -81,8 +77,6 @@ class RequestManager {
required Converter<http.Response, R> converter,
required UnwrapCallback<T, R> unwrap,
AsyncValueGetter<Map<String, String>>? getCacheHeaders,
bool disableTimeout = false,
Duration timeLimit = kDefaultTimeout,
}) async {
if (subject.isClosed) {
return;
Expand Down Expand Up @@ -125,10 +119,7 @@ class RequestManager {
// Save the new cache parameters and return.
if (parameters case CacheParameters(:final etag) when etag != null && getCacheHeaders != null) {
try {
final newHeaders = await timeout(
getCacheHeaders.call,
timeLimit: timeLimit,
);
final newHeaders = await getCacheHeaders.call();
if (subject.isClosed) {
return;
}
Expand All @@ -146,7 +137,7 @@ class RequestManager {
subject.add(Result(unwrapped, null, isLoading: false, isCached: true));
return;
}
} on TimeoutException catch (error) {
} on HttpTimeoutException catch (error) {
_log.info(
'Fetching cache parameters timed out. Assuming expired.',
error,
Expand All @@ -171,24 +162,19 @@ class RequestManager {
subject.add(subject.value.asLoading());
}

var client = httpClient;
// Assume the request is for WebDAV if the Content-Type is application/xml,
if (request.headers['content-type']?.split(';').first == 'application/xml') {
client ??= account.client.webdav.csrfClient;
} else {
client ??= account.client;
}

for (var i = 0; i < kMaxTries; i++) {
try {
final response = await timeout(
() async {
var client = httpClient;
// Assume the request is for WebDAV if the Content-Type is application/xml,
if (request.headers['content-type']?.split(';').first == 'application/xml') {
client ??= account.client.webdav.csrfClient;
} else {
client ??= account.client;
}

final streamedResponse = await client.send(request);
return http.Response.fromStream(streamedResponse);
},
disableTimeout: disableTimeout,
timeLimit: timeLimit,
);
final streamedResponse = await client.send(request);
final response = await http.Response.fromStream(streamedResponse);

if (subject.isClosed) {
return;
}
Expand All @@ -200,12 +186,8 @@ class RequestManager {
response,
);
break;
} on TimeoutException catch (error, stackTrace) {
_log.info(
'Request timeout',
error,
stackTrace,
);
} on HttpTimeoutException catch (error) {
_log.info('Request timeout', error);

if (subject.isClosed) {
return;
Expand Down
152 changes: 0 additions & 152 deletions packages/neon_framework/test/request_manager_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,76 +161,6 @@ void main() {
await subject.close();
});

test('timeout request', () async {
var subject = BehaviorSubject<Result<String>>();

expect(
subject.stream,
emitsInOrder([
equals(Result<String>.loading()),
equals(Result<String>.error('TimeoutException after 0:00:00.050000: Future not completed')),
emitsDone,
]),
);

expect(
subject.stream,
neverEmits([
equals(Result.success(base64String('Test value'))),
emitsDone,
]),
);

await RequestManager.instance.wrap<String, Uint8List>(
account: account,
subject: subject,
getRequest: () => http.Request('GET', Uri(host: 'delay')),
converter: const BinaryResponseConverter(),
unwrap: (deserialized) => base64.encode(deserialized),
timeLimit: const Duration(milliseconds: 50),
);

await subject.close();

subject = BehaviorSubject<Result<String>>.seeded(Result.success('Seed value'));

expect(
subject.stream,
emitsInOrder([
equals(Result.success('Seed value')),
equals(Result.success('Seed value').asLoading()),
equals(
Result(
'Seed value',
'TimeoutException after 0:00:00.050000: Future not completed',
isLoading: false,
isCached: false,
),
),
emitsDone,
]),
);

expect(
subject.stream,
neverEmits([
equals(Result.success(base64String('Test value'))),
emitsDone,
]),
);

await RequestManager.instance.wrap<String, Uint8List>(
account: account,
subject: subject,
getRequest: () => http.Request('GET', Uri(host: 'delay')),
converter: const BinaryResponseConverter(),
unwrap: (deserialized) => base64.encode(deserialized),
timeLimit: const Duration(milliseconds: 50),
);

await subject.close();
});

test('throwing request', () async {
var subject = BehaviorSubject<Result<String>>();

Expand Down Expand Up @@ -385,88 +315,6 @@ void main() {
).called(1);
});

test('timeout request', () async {
var subject = BehaviorSubject<Result<String>>();

expect(
subject.stream,
emitsInOrder([
equals(Result<String>.loading()),
equals(Result(base64String('Cached value'), null, isLoading: true, isCached: true)),
equals(
Result(
base64String('Cached value'),
'TimeoutException after 0:00:00.050000: Future not completed',
isLoading: false,
isCached: true,
),
),
emitsDone,
]),
);

expect(
subject.stream,
neverEmits([
equals(Result.success(base64String('Test value'))),
emitsDone,
]),
);

await RequestManager.instance.wrap<String, Uint8List>(
account: account,
subject: subject,
getRequest: () => http.Request('GET', Uri(host: 'delay')),
converter: const BinaryResponseConverter(),
unwrap: (deserialized) => base64.encode(deserialized),
timeLimit: const Duration(milliseconds: 50),
);

await subject.close();
verify(() => cache.get(account, any())).called(1);
verifyNever(() => cache.set(any(), any(), any()));

subject = BehaviorSubject<Result<String>>.seeded(Result.success('Seed value'));

expect(
subject.stream,
emitsInOrder([
equals(Result.success('Seed value')),
equals(Result(base64String('Cached value'), null, isLoading: true, isCached: true)),
equals(
Result(
base64String('Cached value'),
'TimeoutException after 0:00:00.050000: Future not completed',
isLoading: false,
isCached: true,
),
),
emitsDone,
]),
);

expect(
subject.stream,
neverEmits([
equals(Result.success(base64String('Test value'))),
emitsDone,
]),
);

await RequestManager.instance.wrap<String, Uint8List>(
account: account,
subject: subject,
getRequest: () => http.Request('GET', Uri(host: 'delay')),
converter: const BinaryResponseConverter(),
unwrap: (deserialized) => base64.encode(deserialized),
timeLimit: const Duration(milliseconds: 50),
);

await subject.close();
verify(() => cache.get(account, any())).called(1);
verifyNever(() => cache.set(any(), any(), any()));
});

test('throwing request', () async {
var subject = BehaviorSubject<Result<String>>();

Expand Down

0 comments on commit bc87322

Please sign in to comment.