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

Unexpected Behavior with ErrorInterceptor Not Catching DioException on Retried Requests #2120

Closed
ykaito21 opened this issue Feb 23, 2024 · 19 comments
Labels
e: good for newcomers Good for newcomers fixed p: dio Targeting `dio` package s: best practise It's the best way to do something so far

Comments

@ykaito21
Copy link

Package

dio

Version

5.4.1

Operating-System

iOS

Output of flutter doctor -v

[✓] Flutter (Channel beta, 3.19.0-0.4.pre, on macOS 13.4 22F66 darwin-x64, locale ja-JP)
[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
[✓] Chrome - develop for the web
[✓] Android Studio (version 4.2)
[✓] VS Code (version 1.86.2)
[✓] Connected device (3 available)
[✓] Network resources

Dart Version

3.3.0

Steps to Reproduce

Description:
I am encountering an issue where my custom ErrorInterceptor retries a request using dio.fetch within its onError method when the initial request fails. Although I am explicitly catching exceptions within the onError method, subsequent failures of the retried request do not seem to be caught by the try-catch block. Instead, the onRequest interceptor is triggered again, but any subsequent DioException thrown by the retried request does not seem to be caught within the same onError method.

Steps to Reproduce:

  1. Define custom interceptors: RequestInterceptor, ResponseInterceptor, and ErrorInterceptor.
  2. Add these interceptors to Dio's interceptor chain.
  3. Make a GET request to a non-existent endpoint (e.g., 'https://example.com/data') to intentionally trigger an error.
  4. Within ErrorInterceptor.onError, retry the request using await dio.fetch(err.requestOptions); and wrap this call in a try-catch block to catch any exceptions.
  5. Observe the behavior, particularly the console output and whether the catch block within onError catches the exception from the retried request.

Code Snippet:

import 'package:dio/dio.dart';

class ErrorInterceptor extends QueuedInterceptor {
  final Dio dio;

  ErrorInterceptor(this.dio);

  @override
  Future<void> onError(
      DioException err, ErrorInterceptorHandler handler) async {
    print('onError is called');
    try {
      // This should throw an error
      await dio.fetch(err.requestOptions);
    } catch (e) {
      // Why cannot I catch the error here?
      print('onError is called again');
    }
    handler.next(err);
  }
}

class RequestInterceptor extends Interceptor {
  const RequestInterceptor();

  @override
  Future<void> onRequest(
      RequestOptions options, RequestInterceptorHandler handler) async {
    print('onRequest is called');
    return handler.next(options);
  }
}

class ResponseIntercepter extends Interceptor {
  const ResponseIntercepter();

  @override
  Future<void> onResponse(
      Response response, ResponseInterceptorHandler handler) async {
    print('onResponse is called');
    handler.next(response);
  }
}

void main() async {
  var dio = Dio();

  // Add the custom interceptor
  dio.interceptors.addAll([
    const RequestInterceptor(),
    const ResponseIntercepter(),
    ErrorInterceptor(dio),
  ]);

  // Making a GET request
  try {
    print('Making a GET request...');
    Response response =
        await dio.get('https://example.com/this-does-not-exist');
    print(response.data);
  } catch (e) {
    print("Final error: $e");
  }
}

Additional Context:
I've tried various approaches to ensure that asynchronous operations are correctly awaited and that the try-catch block is properly structured to catch asynchronous exceptions, but the issue persists. This behavior suggests that there might be a problem with how DioException is propagated or caught within onError when retrying requests.

I am looking for guidance on whether this is expected behavior with the current Dio version or if there might be a bug affecting error handling in custom interceptors when retrying requests. Any insights or suggestions on how to ensure that exceptions from retried requests can be caught and handled within onError would be greatly appreciated.

Expected Result

After the request fails and triggers onError, the retried request within onError should also throw a DioException if it fails, which should then be caught by the try-catch block within the same onError method. This should allow for custom error handling or logging of the retried request failure.

Expected Output:

Making a GET request...
onRequest is called
onError is called
onError is called again
Final error: DioException [bad response]: This exception was thrown because the response has a status code of 404 and RequestOptions.validateStatus was configured to throw for this status code.
The status code of 404 has the following meaning: "Client error - the request contains bad syntax or cannot be fulfilled"
Read more about status codes at https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
In order to resolve this exception you typically have either to verify and fix your request code or you have to fix the server code.

Actual Result

The retried request within onError triggers the onRequest interceptor again as expected, but if the retried request fails, the try-catch block within onError does not catch the subsequent DioException. This behavior prevents custom error handling for the retried request failure from being executed as intended.

** Actual Output:**

Making a GET request...
onRequest is called
onError is called
onRequest is called
@ykaito21 ykaito21 added h: need triage This issue needs to be categorized s: bug Something isn't working labels Feb 23, 2024
@LtotheWT
Copy link

+1 any update one this ?

@AlexV525
Copy link
Member

AlexV525 commented Mar 3, 2024

You are reusing the same Dio instance in callbacks, consider adding another instance for retries which could avoid deadlocks in the most of cases.

@ykaito21
Copy link
Author

ykaito21 commented Mar 3, 2024

You are reusing the same Dio instance in callbacks, consider adding another instance for retries which could avoid deadlocks in the most of cases.

You mean I have to use 3 different Dio instances on refresh token and retry? But the queued_interceptor_crsftoken example uses the same instance on retry.

@AlexV525
Copy link
Member

AlexV525 commented Mar 3, 2024

You mean I have to use 3 different Dio instances on refresh token and retry? But the queued_interceptor_crsftoken example uses the same instance on retry.

The example does not contain retries, so 2 instances were used for the flow. More instances do not mean more resource costs because it just acts as a manager. Better start to use packages like dio_smart_retry.

@ykaito21
Copy link
Author

ykaito21 commented Mar 3, 2024

The example does not contain retries, so 2 instances were used for the flow. More instances do not mean more resource costs because it just acts as a manager. Better start to use packages like dio_smart_retry.

Isn't that retry? final originResult = await dio.fetch(options..path += '&pass=true');.
I don't understand the difference from my case.

@AlexV525
Copy link
Member

AlexV525 commented Mar 3, 2024

cc @seunghwanly to provide a detailed explanation.

@seunghwanly
Copy link
Contributor

cc @seunghwanly to provide a detailed explanation.

In my example, I've used 2 different instances for Dio

  • dio: handles main request and response
  • tokenDio: manages token, when token has expired or request failed with 401 status code

/// assume receiving the token has no errors
/// to check `null-safety` and error handling
/// please check inside the [onRequest] closure
final tokenResult = await tokenDio.get('/token');
/// update [csrfToken]
/// assume `token` is in response body
final body = jsonDecode(tokenResult.data) as Map<String, dynamic>?;
options.headers['csrfToken'] = csrfToken = body!['data']['token'];

on onError callback, tokenDio is used for token refresh and adds new 'token' to original dio(dio: L7) as you can see at the last line (L60)

then when its token(csrfToken) has been updated, which I pretended as not null(!= null). I tried a new request with original dio(dio: L7) (new token added).

And for the result, I just added handler when it has succeeded (not for error) like below.

if (originResult.statusCode != null &&
originResult.statusCode! ~/ 100 == 2) {
return handler.resolve(originResult);
}


@ykaito21's example at the top, using same instance might cause infinite loop and keep calling this cycle.

onRequest is called
onError is called

check on debug mode (look for hashCode of dio) and see if same instance is requested all the time.

@seunghwanly
Copy link
Contributor

@AlexV525 Umm .. should I provide more clearer example than the current one? for clear understanding

@AlexV525
Copy link
Member

AlexV525 commented Mar 4, 2024

@AlexV525 Umm .. should I provide more clearer example than the current one? for clear understanding

Yeah feel free to request changes and then we can discuss

@ykaito21
Copy link
Author

ykaito21 commented Mar 4, 2024

@seunghwanly Thank you for your help. For me, your example is clear enough, and I understand the flow, but my point is what happen if final originResult = await dio.fetch(options..path += '&pass=true'); fails, and how to handle it.

  • Should I use different dio instance for retry the original request onError?, but using different instances for the same request requires the same configuration for both instances?
  • Should I call retried original request with in try-catch block or not?

final originResult = await dio.fetch(options..path += '&pass=true');
if (originResult.statusCode != null &&
originResult.statusCode! ~/ 100 == 2) {
return handler.resolve(originResult);
}

@ykaito21's example at the top, using same instance might cause infinite loop and keep calling this cycle.

onRequest is called
onError is called

check on debug mode (look for hashCode of dio) and see if same instance is requested all the time.

indeed, it is the same instance

@seunghwanly
Copy link
Contributor

seunghwanly commented Mar 4, 2024

Should I use different dio instance for retry the original request onError?, but using different instances for the same request requires the same configuration for both instances?

If we need updated data from the server, I'd rather use another instance for the request, and for the same configuration I made an example like the one below.

tokenDio.options = dio.options;

Or else I might not use a different instance to request a retrial. In that case things like changing the host from cached to its origin.

const originHost = 'origin.com';
const cachedHost = 'cached.origin.com';

final originDio = Dio();

... // [originDio]'s interceptor

  onError: (error, handler) async {
    /// Pretend we have an extension called [isTimeout] 
    /// returns `true` when the request past 2 min.
    if (error.isTimeout) {
      /// Set new host
      dio.options.baseUrl = 'https://$cachedHost';
      final result = await dio.fetch(dio.options);
      
      /// handle result
      // TODO
    }
  }
  

Should I call retried original request with in try-catch block or not?

This depends on your code style. I sometimes use the try-catch block to handle specific status codes like 401, 403, 500, and so on. Or handling on 'Presentation Layer' like using Provider, BLoC, or RiverPod.

I hope my answer was helpful enough. Please let me know if you need any further assistance. :)

If we use the same instance in the onError callback and try to catch an error within the try-catch block, we cannot reach the catch block at all.. because the dio will keep rolling between onRequest and onError's try block

P.S. I will soon request changes for the csrfToken example.

@kuhnroyal
Copy link
Member

I have a token refresh interceptor and now that I read this, I think I am actually seeing a similar behavior.
Need to look into this when I find some time.

@ykaito21
Copy link
Author

ykaito21 commented Mar 5, 2024

If we use the same instance in the onError callback and try to catch an error within the try-catch block, we cannot reach the catch block at all.. because the dio will keep rolling between onRequest and onError's try block

You mean if I use the same instance in the onError callback, it doesn't matter to use try-catch or not because it doesn't reach to that catch block if it fails. Isn't that the problem?
And if I use the same instance in the onError callback, that dio should not or cannot throw an Exception because if it does, it will keep rolling between onRequest and onError's try block?

Should I call retried original request with in try-catch block or not?

This depends on your code style. I sometimes use the try-catch block to handle specific status codes like 401, 403, 500, and so on. Or handling on 'Presentation Layer' like using Provider, BLoC, or RiverPod.

but if I use dio(different instances) in the onError, and it throws an Exception without try-catch block or catchError, it will cause Unhandled exception. Is this expected?

class ErrorInterceptor extends QueuedInterceptor {
  final Dio dio;
  final Dio dio2;

  ErrorInterceptor(this.dio, this.dio2);

  @override
  Future<void> onError(
      DioException err, ErrorInterceptorHandler handler) async {
    print('onError is called');
    try {
      // Without try-catch, it will throw unhandled exception
      await dio2.fetch(err.requestOptions);
    } catch (e) {
      print('onError is called again');
    }
    handler.next(err);
  }
}

void main() async {
  var dio = Dio();
  var dio2 = Dio();

  // Add the custom interceptor
  dio.interceptors.addAll([
    ErrorInterceptor(dio, dio2),
  ]);

  // Making a GET request
  try {
    print('Making a GET request...');
    Response response =
        await dio.get('https://example.com/this-does-not-exist');
    print(response.data);
  } catch (e) {
    print("Final error: $e");
  }
}

Really appreciate your assistance!

@seunghwanly
Copy link
Contributor

seunghwanly commented Mar 6, 2024

@ykaito21

You mean if I use the same instance in the onError callback, it doesn't matter to use try-catch or not because it doesn't reach to that catch block if it fails. Isn't that the problem?
And if I use the same instance in the onError callback, that dio should not or cannot throw an Exception because if it does, it will keep rolling between onRequest and onError's try block?

Yes, it create a cycle like this.

dio (onRequest) → Requested → Error → dio (onError) → dio (onRequest) → Requested → Error → dio (onError) → ...

Isn't that the problem?

the actual outputs is just like as what you added as interceptors.addAll()

but if I use dio(different instances) in the onError, and it throws an Exception without try-catch block or catchError, it will cause Unhandled exception. Is this expected?

Yes, 'onError is called again' will be printed as you expected 👍

Control with handler's method

  • next
  • resolve
  • reject
    that might be helpful

@seunghwanly
Copy link
Contributor

seunghwanly commented Mar 6, 2024

@AlexV525

I updated example/lib/queued_interceptor_crsftoken.dart here. It seems to be more clear than before like handling errors and queued interceptors.

Outputs
Outputs

@ykaito21
Copy link
Author

ykaito21 commented Mar 7, 2024

@seunghwanly, thanks for the detailed explanation and updated example.

I have a question about your example, I don't see a case in onError for an automatic retry with a new token, if I wanted to add that, could I just create a new dio in onError and use that like tokenDio?

  /// Add `onError` interceptor to request new CSRF token
  dio.interceptors.add(
    QueuedInterceptorsWrapper(
      /// Request new CSRF token
      /// if the response status code is `401`
      onError: (error, handler) async {
        log('Error Detected: ${error.message}');


        if (error.response == null) return handler.next(error);


        if (error.response?.statusCode == 401) {
          try {
            final tokenDio = Dio(
              BaseOptions(baseUrl: error.requestOptions.baseUrl),
            );


            /// Generate CSRF token
            ///
            /// This is a MOCK REQUEST to generate a CSRF token.
            /// In a real-world scenario, this should be generated by the server.
            final result = await tokenDio.post(
              '/response-headers',
              queryParameters: {
                _headerKey: '94d6d1ca-fa06-468f-a25c-2f769d04c26c',
              },
            );


            if (result.statusCode == null || result.statusCode! ~/ 100 != 2) {
              throw DioException(requestOptions: result.requestOptions);
            }


            final updatedToken = result.headers.value(_headerKey);
            if (updatedToken == null) throw ArgumentError.notNull(_headerKey);


            cachedCSRFToken = updatedToken;
           
             // Can I do like this? or is there better way?
            final retryDio = Dio(
              BaseOptions(baseUrl: error.requestOptions.baseUrl),
            );
            final requestOptions = err.requestOptions;
            requestOptions.headers['token'] = '$updatedToken';
            final res = retryDio.fetch(requestOptions);

            return handler.resolve(res);
          } on DioException catch (e) {
            return handler.reject(e);
          }
        }
      },
    ),
  );

@seunghwanly
Copy link
Contributor

@seunghwanly, thanks for the detailed explanation and updated example.

I have a question about your example, I don't see a case in onError for an automatic retry with a new token, if I wanted to add that, could I just create a new dio in onError and use that like tokenDio?

  /// Add `onError` interceptor to request new CSRF token
  dio.interceptors.add(
    QueuedInterceptorsWrapper(
      /// Request new CSRF token
      /// if the response status code is `401`
      onError: (error, handler) async {
        log('Error Detected: ${error.message}');


        if (error.response == null) return handler.next(error);


        if (error.response?.statusCode == 401) {
          try {
            final tokenDio = Dio(
              BaseOptions(baseUrl: error.requestOptions.baseUrl),
            );


            /// Generate CSRF token
            ///
            /// This is a MOCK REQUEST to generate a CSRF token.
            /// In a real-world scenario, this should be generated by the server.
            final result = await tokenDio.post(
              '/response-headers',
              queryParameters: {
                _headerKey: '94d6d1ca-fa06-468f-a25c-2f769d04c26c',
              },
            );


            if (result.statusCode == null || result.statusCode! ~/ 100 != 2) {
              throw DioException(requestOptions: result.requestOptions);
            }


            final updatedToken = result.headers.value(_headerKey);
            if (updatedToken == null) throw ArgumentError.notNull(_headerKey);


            cachedCSRFToken = updatedToken;
           
             // Can I do like this? or is there better way?
            final retryDio = Dio(
              BaseOptions(baseUrl: error.requestOptions.baseUrl),
            );
            final requestOptions = err.requestOptions;
            requestOptions.headers['token'] = '$updatedToken';
            final res = retryDio.fetch(requestOptions);

            return handler.resolve(res);
          } on DioException catch (e) {
            return handler.reject(e);
          }
        }
      },
    ),
  );

Yes, I wrote the example for QueuedInterceptorsWrapper to see callbacks handled in a row. And as it said before

The example does not contain retries, so 2 instances were used for the flow. More instances do not mean more resource costs because it just acts as a manager. Better start to use packages like dio_smart_retry.

use another dio to manage retry or other requests to handle exception cases. Here is an example for a retry interceptor.

@ykaito21 ykaito21 closed this as completed Mar 7, 2024
@AlexV525
Copy link
Member

AlexV525 commented Mar 7, 2024

@AlexV525

I updated example/lib/queued_interceptor_crsftoken.dart here. It seems to be more clear than before like handling errors and queued interceptors.

Outputs 스크린샷 2024-03-07 오전 2 02 05

@seunghwanly Hi sorry for missing the thread. Could you submit the pull request?

@seunghwanly
Copy link
Contributor

@AlexV525
I updated example/lib/queued_interceptor_crsftoken.dart here. It seems to be more clear than before like handling errors and queued interceptors.
Outputs 스크린샷 2024-03-07 오전 2 02 05

@seunghwanly Hi sorry for missing the thread. Could you submit the pull request?

Thanks, I opened #2128

@AlexV525 AlexV525 added e: good for newcomers Good for newcomers p: dio Targeting `dio` package s: best practise It's the best way to do something so far fixed and removed h: need triage This issue needs to be categorized s: bug Something isn't working labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e: good for newcomers Good for newcomers fixed p: dio Targeting `dio` package s: best practise It's the best way to do something so far
Projects
None yet
Development

No branches or pull requests

5 participants