Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interceptor onError handling broken #2167

Closed
fischermario opened this issue Mar 27, 2024 · 3 comments · Fixed by #2169
Closed

Interceptor onError handling broken #2167

fischermario opened this issue Mar 27, 2024 · 3 comments · Fixed by #2169
Labels
b: regression Worked before but not now fixed p: dio Targeting `dio` package s: bug Something isn't working 🚒 URGENT Needs to be resolved immediately

Comments

@fischermario
Copy link

fischermario commented Mar 27, 2024

Package

dio

Version

5.4.2

Operating-System

Android

Adapter

Default Dio

Output of flutter doctor -v

[✓] Flutter (Channel stable, 3.19.4, on Ubuntu **************, locale en_US.UTF-8)
    • Flutter version 3.19.4 on channel stable at **************
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 68bfaea224 (**************), 2024-03-20 15:36:31 -0700
    • Engine revision a5c24f538d
    • Dart version 3.3.2
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at **************
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /snap/android-studio/151/jbr/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • clang version 10.0.0-4ubuntu1
    • cmake version 3.16.3
    • ninja version 1.10.0
    • pkg-config version 0.29.1

[✓] Android Studio (version 2023.2)
    • Android Studio at /snap/android-studio/151
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)

[✓] VS Code (version 1.85.0)
    • VS Code at /usr/share/code
    • Flutter extension version 3.84.0

[✓] Connected device (2 available)
    • sdk gphone64 x86 64 (mobile) • emulator-5554 • android-x64    • Android 13 (API 33) (emulator)
    • Chrome (web)                 • chrome        • web-javascript • Google Chrome 122.0.6261.128

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Dart Version

3.3.2

Steps to Reproduce

The onError handler of a new class that extends the "Interceptor" class will not allow subsequent interceptors to successfully catch exceptions.

Please consider the following example which is derived from this example:

class DioCustomInterceptor extends Interceptor {
  @override
  void onError(
    DioException err,
    ErrorInterceptorHandler handler,
  ) async {
    debugPrint('onError');

    super.onError(err, handler);
  }
}

// ...

Dio dio = Dio(BaseOptions(
  receiveTimeout: const Duration(milliseconds: 2000),
  connectTimeout: const Duration(milliseconds: 2000),
  sendTimeout: const Duration(milliseconds: 2000),
  receiveDataWhenStatusError: true,
));
dio.interceptors.addAll([
  DioCustomInterceptor(),
  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());

Expected Result

I/flutter (xx): onError
I/flutter (xx): CAUGHT error: DioException [receive timeout]: The request took longer than 0:00:02.000000 to receive data. It was aborted. To get rid of this exception, try raising the RequestOptions.receiveTimeout above the duration of 0:00:02.000000 or improve the response time of the server.
I/flutter (xx): 500

Actual Result

The debugger halts the execution after the "onError" output. Forcing the debugger to continue yields the following error:

E/flutter (xx): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: Instance of 'InterceptorState<DioException>'
E/flutter (xx): #0      DioMixin.request (package:dio/src/dio_mixin.dart:350:36)
E/flutter (xx): #1      DioMixin.get (package:dio/src/dio_mixin.dart:64:12)
...
E/flutter (xx): 
I/flutter (xx): CAUGHT error: DioException [receive timeout]: The request took longer than 0:00:02.000000 to receive data. It was aborted. To get rid of this exception, try raising the RequestOptions.receiveTimeout above the duration of 0:00:02.000000 or improve the response time of the server.
I/flutter (xx): 500

This means the onError handler of the second interceptor is acutally executed, but it is not able not catch the exception properly.
If we now replace

class DioCustomInterceptor extends Interceptor {

with

class DioCustomInterceptor extends QueuedInterceptor {

everything works as expected.
Using inline InterceptorsWrapper instead of a custom class also prevents this problem from appearing.

I guess that there is a simple explanation for this behavior, but I could not find it in the docs.

For example the dio_cache_interceptor package is implemented by extending the "Interceptor" class which triggers the described behavior that breaks my project.

@kuhnroyal
Copy link
Member

The cause is #2139

In your example you can just remove the async keyword, or actually return an uncompleted Future.
Not sure how/if that works for other interceptors...

@AlexV525
Copy link
Member

Generally we should avoid async on void, as the lint rule: https://dart.dev/tools/linter-rules/avoid_void_async

We can figure out what we can do here for better experiences.

@AlexV525 AlexV525 added p: dio Targeting `dio` package b: regression Worked before but not now and removed h: need triage This issue needs to be categorized labels Mar 28, 2024
@AlexV525 AlexV525 added the 🚒 URGENT Needs to be resolved immediately label Mar 28, 2024
@AlexV525
Copy link
Member

It actually breaks all interceptors written in void async or Future<void> async.

AlexV525 added a commit that referenced this issue Mar 28, 2024
Reverts #2139
Fixes #2167
Reopen #2138

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [ ] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

### Additional context and info (if any)

Added tests were fake, so they remained effective after the revert.
@AlexV525 AlexV525 added the fixed label Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b: regression Worked before but not now fixed p: dio Targeting `dio` package s: bug Something isn't working 🚒 URGENT Needs to be resolved immediately
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants