Repository navigation
Async Cache is caching exceptions #366
Description
Steps to reproduce
To reproduce the same issue you can refer the code sample given below.
Expected results
It should only cache successful result.
Actual results
It should not cache exception.
This callback is running
We have successfully cancelled future
Error First Exception: Future Cancelled
Error Second Exception: Future Cancelled
Code sample
Code sample
import 'package:async/async.dart';
late AsyncCache<String> temporaryCache;
late CancelableOperation<String> cancelableOperation;
Future<void> main(List<String> arguments) async {
temporaryCache = AsyncCache(Duration(seconds: 15));
cancelableOperation =
CancelableOperation.fromFuture(_futureToRun(), onCancel: () {
print('We have successfully cancelled future');
});
getData().then((value) => print('First')).onError((error, stackTrace) => print('Error First $error'));
cancelableOperation.cancel();
getData().then((value) => print('Second')).onError((error, stackTrace) => print('Error Second $error'));
}
Future<String> _futureToRun() async {
await Future.delayed(const Duration(seconds: 5));
return Future.value("Temporary Cache");
}
Future<String> getData() async {
return temporaryCache.fetch(() async {
print('This callback is running');
String? value = await cancelableOperation.valueOrCancellation();
if (value != null) {
return value;
}
throw Exception('Future Cancelled');
});
}
Activity
lrhn commentedon Dec 8, 2023
It caches the actual result, which is reasonable.
Only caching successful futures is more of a "retry" behavior than a cache.
Meaning that a cache which doesn't cache errors is a reasonable thing, but a different feature than this async cache.
We could have both, or the current cache could have a flag.
akmalviya03 commentedon Dec 8, 2023
Creating a new class only for this functionality can be redundant, because
AsyncCacheandAsyncMemoizerare showing behaviour when error occurred. We can introduce a flag. What's your opinion on this?lrhn commentedon Dec 8, 2023
A flag is probably the simplest approach.
Extra work which only happens if an error occurs should not be a problem.
Still means that the default will still be the current behavior.
akmalviya03 commentedon Dec 8, 2023
Sure, adding a new flag with the name
canRetry. Or should we name this flag something else?akmalviya03 commentedon Dec 8, 2023
Added new variable
_canCacheExceptionit's making more sense overcanRetry