llfbandit / dio_cache_interceptor

Dio HTTP cache interceptor with multiple stores respecting HTTP directives (ETag, Last-Modified, Cache-Control) with options.
https://pub.dev/packages/dio_cache_interceptor
119 stars 70 forks source link

DioCacheInterceptor breaks error handling #148

Closed fischermario closed 5 months ago

fischermario commented 6 months ago

The onError handler of DioCacheInterceptor is not able to hand over error handling to the next interceptor in the pipeline. After the onError handler is finished an exception occurs that is not cought by the subsequent interceptor(s).

Please consider the following example:

late CacheOptions cacheOptions;
late CacheStore cacheStore;
Dio dio = Dio(BaseOptions(
  receiveTimeout: const Duration(milliseconds: 2000),
  connectTimeout: const Duration(milliseconds: 2000),
  sendTimeout: const Duration(milliseconds: 2000),
  receiveDataWhenStatusError: true,
));
getTemporaryDirectory().then((dir) async {
  cacheStore = FileCacheStore(dir.path);
  cacheOptions = CacheOptions(
    store: cacheStore,
    // We will use cache functionality on demand
    policy: CachePolicy.noCache,
  );
  dio.interceptors.addAll([
    DioCacheInterceptor(
      options: cacheOptions,
    ),
    InterceptorsWrapper(
      onError: (err, handler) {
        debugPrint('CAUGHT error: $err');
        handler.resolve(
          Response(
            requestOptions: err.requestOptions,
            statusCode: 500,
          ),
        );
      },
    ),
  ]);
  Response resp = await dio.get('https://httpbin.org/delay/10');
  debugPrint(resp.statusCode.toString());
}

Changing this line to

class DioCacheInterceptor extends QueuedInterceptor {

fixes this problem (I can create a PR if interested).

I have also informed the Dio project about this behavior here.

Is there a particular reason why you chose to extend "Interceptor" instead of "QueuedInterceptor"?

llfbandit commented 6 months ago

I don't use QueuedInterceptor because of concurrency concern. There's no need to stream each request because of this package. That said, dio should follow interceptor chain setup...

kuhnroyal commented 6 months ago

Edit: Meant to post in the dio issue.