Async Cache is caching exceptions

[akmalviya03]

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');
  });
}

[lrhn]

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]

Creating a new class only for this functionality can be redundant, because AsyncCache and AsyncMemoizer are showing behaviour when error occurred. We can introduce a flag. What's your opinion on this?

[lrhn]

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]

Sure, adding a new flag with the name canRetry. Or should we name this flag something else?

[akmalviya03]

Added new variable _canCacheException it's making more sense over canRetry