Skip to content

feat(dio): propagate custom exception types from interceptors (#1950)#2514

Open
realmeylisdev wants to merge 4 commits into
cfug:mainfrom
realmeylisdev:feat/1950-custom-exception-unwrapping
Open

feat(dio): propagate custom exception types from interceptors (#1950)#2514
realmeylisdev wants to merge 4 commits into
cfug:mainfrom
realmeylisdev:feat/1950-custom-exception-unwrapping

Conversation

@realmeylisdev
Copy link
Copy Markdown

Summary

Adds an opt-in mechanism for developers to propagate custom exception types from interceptors back to the caller of dio.get/post/..., so that try { … } on MyException catch (e) matches at the await boundary instead of always seeing DioException.

Resolves #1950.

Continuation of prior work in #2484 (closed because the contributor's fork was deleted, not for design reasons). API shape follows @kuhnroyal's suggestion in #1950 (comment). Original issue filed by @WutangNailao.

Reproducing the original problem (without this PR)

import 'package:dio/dio.dart';

class MyApiException implements Exception {
  MyApiException(this.code);
  final String code;
}

Future<void> main() async {
  final dio = Dio()
    ..interceptors.add(InterceptorsWrapper(
      onResponse: (response, handler) {
        // The user wants to throw their own exception type here.
        throw MyApiException('unauthorized');
      },
    ));

  try {
    await dio.get('https://example.com/');
  } on MyApiException catch (e) {
    print('caught MyApiException: \${e.code}');     // <-- never reached
  } on DioException catch (e) {
    print('caught DioException, inner: \${e.error}'); // <-- always reached
  }
}

Before this PR: on MyApiException never matches. The exception is always wrapped as DioException; the original is accessible only via e.error.

After this PR (opt-in):

dio.interceptors.add(InterceptorsWrapper(
  onResponse: (response, handler) {
    handler.rejectCustomError(
      MyApiException('unauthorized'),
      response.requestOptions,
    );
  },
));

try {
  await dio.get('https://example.com/');
} on MyApiException catch (e) {
  print('caught MyApiException: \${e.code}');     // <-- now matches
}

The equivalent throw-based form also works:

throw DioException.custom(
  MyApiException('unauthorized'),
  requestOptions: response.requestOptions,
);

Changes

  • DioException.custom(...) factory and instance flag propagateInnerError (default false).
  • rejectCustomError(...) helper on RequestInterceptorHandler, ResponseInterceptorHandler, and ErrorInterceptorHandler.
  • Single unwrap point in DioMixin.fetch()'s final catch using Error.throwWithStackTrace to preserve the throw-site stack.
  • copyWith carries the flag (so downstream onError interceptors can enrich without losing the marker).
  • dio/README.md: new "Throwing custom exceptions from interceptors" subsection + propagateInnerError field listing.
  • dio/CHANGELOG.md: Unreleased entry.

Backwards compatibility

Default behavior unchanged. Every existing on DioException catch (e) keeps working — the new behavior is strictly opt-in via the new factory or helper. A regression sentinel test (unmarked DioException(error: customEx) still wraps) verifies this.

Compatibility Policy unaffected (no SDK bump). Error.throwWithStackTrace is in Dart 2.16; dio's floor is 2.18.

Why this design

  • One funnel. All interceptor errors converge at DioMixin.fetch()'s final catch; unwrap there is sufficient and safe (transport errors created by _dispatchRequest never set propagateInnerError).
  • Marker survives by virtue of assureDioException's short-circuit (if (error is DioException) return error;). No need to teach errorInterceptorWrapper, QueuedInterceptor._handleRequest/Response/Error, or _dispatchRequest about the marker — they already pass through DioException unchanged.
  • No interaction with fix(dio): prevent request hang when async interceptor throws #2499 (the in-flight async-zone fix). Disjoint code regions; either order of merge is fine.

Test plan

Added 9 tests in dio/test/interceptor_test.dart under group Custom exception unwrapping (#1950):

  1. handler.rejectCustomError from onResponse propagates inner type
  2. handler.rejectCustomError from onRequest propagates inner type
  3. handler.rejectCustomError from onError propagates inner type
  4. throw DioException.custom(...) directly from interceptor propagates inner type
  5. Regression sentinel: unmarked DioException(error: customEx) still wraps as DioException
  6. copyWith preserves propagateInnerError through an onError chain (with callFollowingErrorInterceptor: true)
  7. rejectCustomError works inside QueuedInterceptor
  8. DioException.custom factory sets propagateInnerError: true
  9. DioException default constructor leaves propagateInnerError: false

Verified locally:

  • melos run format (CI-strict): clean
  • dart analyze --fatal-infos on dio package: no issues
  • Full dio VM test suite: all 38 interceptor tests + all 4 queued-interceptor tests pass; broader suite green except for test/test_suite_test.dart: download delete on cancel which is a pre-existing flake against external httpbun.com (passes on isolated re-run; unrelated to this change).

Files changed

  • dio/lib/src/dio_exception.dart — flag, factory, copyWith
  • dio/lib/src/interceptor.dart — three rejectCustomError helpers
  • dio/lib/src/dio_mixin.dart — funnel unwrap
  • dio/test/interceptor_test.dart — 9 new tests + helper class
  • dio/CHANGELOG.md — Unreleased entry
  • dio/README.md — docs subsection + propagateInnerError field

)

Add `DioException.custom(...)` factory and `rejectCustomError(...)`
helpers on the three interceptor handlers, allowing developers to
propagate the original exception type from inside an `Interceptor` to
the caller of the request method. The marker survives traversal
through subsequent `onError` interceptors, through `QueuedInterceptor`,
and through `copyWith`. Unwrap happens at the single funnel in
`DioMixin.fetch()` via `Error.throwWithStackTrace`.

Default behavior is unchanged: existing `DioException` constructors and
factories leave `propagateInnerError: false`, so callers that
`on DioException catch (e)` continue to work without modification.

Continuation of prior work in cfug#2484 (closed because the contributor's
fork was deleted, not for design reasons). API shape follows
@kuhnroyal's suggestion in
cfug#1950 (comment).
Original issue filed by @WutangNailao.

Resolves cfug#1950.
@kuhnroyal
Copy link
Copy Markdown
Member

Thanks! Wondered what happened to the last PR. I only have a phone for the next 10 days, will try to review afterwards.

@realmeylisdev
Copy link
Copy Markdown
Author

Old fork got deleted on my end, which auto-closed it — no design issues, just sat with only a Copilot review. This PR is the same DioException.custom(...) shape you suggested in #1950, with a few refinements.

Copy link
Copy Markdown
Member

@kuhnroyal kuhnroyal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just 2 ideas for some consistency in naming but I would be fine either way.

/// See [DioException.custom] for details.
///
/// Resolves https://github.com/cfug/dio/issues/1950.
void rejectCustomError(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably go with just rejectCustom everywhere

/// behavior is unchanged.
///
/// Resolves https://github.com/cfug/dio/issues/1950.
final bool propagateInnerError;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isCustom?

@AlexV525
Copy link
Copy Markdown
Member

(To resolve the current CI failure, update the branch to the latest for recent fixes)

Signed-off-by: Meylis <meylis@divine.video>
After merging main, the cfug#2499 async-interceptor error zone routes
uncaught async throws through assureDioException, whose DioException
short-circuit preserves the propagateInnerError marker. Lock in that an
async `throw DioException.custom(...)` (which hung before the merge)
still unwraps to the inner type at the funnel.

Signed-off-by: Meylis <meylis@divine.video>
@realmeylisdev
Copy link
Copy Markdown
Author

Thanks @AlexV525 — merged latest main into the branch (now at the http2_adapter v2.7.1 tip, via #2522 / #2499). That pulls in the HTTP/2 authority-header fix behind the Verify packages abilities failure (HTTP/2 error: Connection is being forcefully terminated).

The merge is disjoint from the #1950 custom-exception funnel — assureDioException's is DioException short-circuit preserves the propagateInnerError marker, so it survives #2499's new async error-zone cleanly. As a bonus that interaction now also propagates the inner type for an async throw DioException.custom(...); I added a regression test (74b0cc2) locking that in.

Locally green: dart analyze --fatal-infos, dart format, and the full dio VM suite (including all #1950 tests).

The new workflow runs are showing action_required — could you approve them when you get a chance? 🙏

@github-actions
Copy link
Copy Markdown
Contributor

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
dio/lib/src/dio_exception.dart 🟢 89.39% 🟢 91.3% 🟢 1.91%
dio/lib/src/dio_mixin.dart 🟢 94.12% 🟢 94.23% 🟢 0.11%
dio/lib/src/interceptor.dart 🟢 99.3% 🟢 99.34% 🟢 0.04%
Overall Coverage 🟢 88.88% 🟢 89.02% 🟢 0.14%

Minimum allowed coverage is 0%, this run produced 89.02%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Developer should throw custom exception in interceptor

3 participants