-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(dio): prevent request hang when async interceptor throws #2499
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -386,6 +386,25 @@ abstract class DioMixin implements Dio { | |
| } | ||
| } | ||
|
|
||
| // Create an error zone that catches uncaught async errors from | ||
| // interceptor callbacks. This prevents requests from hanging when | ||
| // an async interceptor throws without calling the handler. | ||
| // Synchronous exceptions are not caught here and propagate normally. | ||
| Zone createInterceptorZone( | ||
| _BaseHandler handler, | ||
| void Function(Object error) onUncaughtError, | ||
| ) { | ||
| return Zone.current.fork( | ||
| specification: ZoneSpecification( | ||
| handleUncaughtError: (self, parent, zone, error, stackTrace) { | ||
| if (!handler.isCompleted) { | ||
| onUncaughtError(error); | ||
| } | ||
|
Comment on lines
+399
to
+402
|
||
| }, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| // Convert the request interceptor to a functional callback in which | ||
| // we can handle the return value of interceptor callback. | ||
| FutureOr Function(dynamic) requestInterceptorWrapper( | ||
|
|
@@ -398,7 +417,13 @@ abstract class DioMixin implements Dio { | |
| requestOptions.cancelToken, | ||
| Future(() async { | ||
| final handler = RequestInterceptorHandler(); | ||
| cb(state.data as RequestOptions, handler); | ||
| createInterceptorZone( | ||
| handler, | ||
| (error) => handler.reject( | ||
| assureDioException(error, requestOptions), | ||
| true, | ||
| ), | ||
| ).run(() => cb(state.data as RequestOptions, handler)); | ||
| return handler.future; | ||
| }), | ||
| ); | ||
|
|
@@ -420,7 +445,13 @@ abstract class DioMixin implements Dio { | |
| requestOptions.cancelToken, | ||
| Future(() async { | ||
| final handler = ResponseInterceptorHandler(); | ||
| cb(state.data as Response, handler); | ||
| createInterceptorZone( | ||
| handler, | ||
| (error) => handler.reject( | ||
| assureDioException(error, requestOptions), | ||
| true, | ||
| ), | ||
| ).run(() => cb(state.data as Response, handler)); | ||
| return handler.future; | ||
| }), | ||
| ); | ||
|
|
@@ -440,7 +471,10 @@ abstract class DioMixin implements Dio { | |
| : InterceptorState(assureDioException(error, requestOptions)); | ||
| Future<InterceptorState> handleError() async { | ||
| final handler = ErrorInterceptorHandler(); | ||
| cb(state.data, handler); | ||
| createInterceptorZone( | ||
| handler, | ||
| (error) => handler.next(assureDioException(error, requestOptions)), | ||
| ).run(() => cb(state.data, handler)); | ||
| return handler.future; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -426,6 +426,87 @@ void main() { | |
| ); | ||
| }); | ||
|
|
||
| test('Async interceptor error does not hang the request', () async { | ||
| final dio = Dio() | ||
| ..options.baseUrl = MockAdapter.mockBase | ||
| ..httpClientAdapter = MockAdapter(); | ||
| const errorMsg = 'async interceptor error'; | ||
| dio.interceptors.add( | ||
| InterceptorsWrapper( | ||
| // ignore: void_checks | ||
| onRequest: (options, handler) async { | ||
| await Future<void>.delayed(const Duration(milliseconds: 10)); | ||
| throw UnsupportedError(errorMsg); | ||
| }, | ||
|
Comment on lines
+437
to
+440
|
||
| ), | ||
| ); | ||
| await expectLater( | ||
| dio.get('/test'), | ||
| throwsA( | ||
| isA<DioException>().having( | ||
| (e) => e.error, | ||
| 'error', | ||
| isA<UnsupportedError>() | ||
| .having((e) => e.message, 'message', errorMsg), | ||
| ), | ||
| ), | ||
| ); | ||
| }); | ||
|
|
||
| test('Async onResponse error does not hang the request', () async { | ||
| final dio = Dio() | ||
| ..options.baseUrl = MockAdapter.mockBase | ||
| ..httpClientAdapter = MockAdapter(); | ||
| const errorMsg = 'async response error'; | ||
| dio.interceptors.add( | ||
| InterceptorsWrapper( | ||
| // ignore: void_checks | ||
| onResponse: (response, handler) async { | ||
| await Future<void>.delayed(const Duration(milliseconds: 10)); | ||
| throw UnsupportedError(errorMsg); | ||
| }, | ||
|
Comment on lines
+464
to
+467
|
||
| ), | ||
| ); | ||
| await expectLater( | ||
| dio.get('/test'), | ||
| throwsA( | ||
| isA<DioException>().having( | ||
| (e) => e.error, | ||
| 'error', | ||
| isA<UnsupportedError>() | ||
| .having((e) => e.message, 'message', errorMsg), | ||
| ), | ||
| ), | ||
| ); | ||
| }); | ||
|
|
||
| test('Async onError interceptor error does not hang', () async { | ||
| final dio = Dio() | ||
| ..options.baseUrl = MockAdapter.mockBase | ||
| ..httpClientAdapter = MockAdapter(); | ||
| const errorMsg = 'async error interceptor error'; | ||
| dio.interceptors.add( | ||
| InterceptorsWrapper( | ||
| // ignore: void_checks | ||
| onError: (err, handler) async { | ||
| await Future<void>.delayed(const Duration(milliseconds: 10)); | ||
| throw UnsupportedError(errorMsg); | ||
| }, | ||
|
Comment on lines
+491
to
+494
|
||
| ), | ||
| ); | ||
| await expectLater( | ||
| dio.get('/test-not-found'), | ||
| throwsA( | ||
| isA<DioException>().having( | ||
| (e) => e.error, | ||
| 'error', | ||
| isA<UnsupportedError>() | ||
| .having((e) => e.message, 'message', errorMsg), | ||
| ), | ||
| ), | ||
| ); | ||
| }); | ||
|
|
||
| group(ImplyContentTypeInterceptor, () { | ||
| Dio createDio() { | ||
| final dio = Dio(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The custom
handleUncaughtErrorswallows async errors whenhandler.isCompletedis already true (it neither forwards toparent.handleUncaughtErrornor rethrows). That can hide genuinely unhandled errors from interceptor code that happen after calling the handler (e.g., fire-and-forget futures). Consider delegating toparent.handleUncaughtError(...)when the handler is already completed so those errors are still reported, while only converting tohandler.reject/nextwhen it isn’t completed yet.