Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: Catch sync/async* exceptions in interceptor's handlers #2139

Merged
merged 10 commits into from
Mar 15, 2024

Conversation

CaiJingLong
Copy link
Contributor

@CaiJingLong CaiJingLong commented Mar 14, 2024

Resolves #2138

New Pull Request Checklist

  • I have read the Documentation
  • I have searched for a similar pull request in the project and found none
  • 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
  • I have updated the documentation (if necessary)
  • I have run the tests without failures
  • I have updated the CHANGELOG.md in the corresponding package

Signed-off-by: CaiJingLong <cjl_spy@163.com>
@CaiJingLong CaiJingLong requested a review from a team as a code owner March 14, 2024 03:18
Signed-off-by: CaiJingLong <cjl_spy@163.com>
Signed-off-by: CaiJingLong <cjl_spy@163.com>
Signed-off-by: CaiJingLong <cjl_spy@163.com>
Signed-off-by: CaiJingLong <cjl_spy@163.com>
@kuhnroyal
Copy link
Member

Should we be more explicit about this and also change the return type of the Interceptor functions to FutureOr<void>?
That might break a couple interceptor implementations though :/

@AlexV525
Copy link
Member

Should we be more explicit about this and also change the return type of the Interceptor functions to FutureOr<void>? That might break a couple interceptor implementations though :/

We can keep those as long as Dart can recognize them, IMO. :D

@AlexV525 AlexV525 changed the title fix: Uncatch error in interceptor fix: Catch sync/async* exceptions in interceptor's handlers Mar 15, 2024
@AlexV525 AlexV525 force-pushed the fix-uncatch-error-in-interceptor branch from ea87715 to e1399ce Compare March 15, 2024 02:56
Copy link
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.

Nice fix, I created a follow up #2141

@AlexV525 AlexV525 added this pull request to the merge queue Mar 15, 2024
Merged via the queue into main with commit a3e7dc2 Mar 15, 2024
3 checks passed
@AlexV525 AlexV525 deleted the fix-uncatch-error-in-interceptor branch March 15, 2024 15:07
@kuhnroyal
Copy link
Member

Looks like the await callback breaks some stuff.

@kuhnroyal
Copy link
Member

Should we be more explicit about this and also change the return type of the Interceptor functions to FutureOr<void>? That might break a couple interceptor implementations though :/

I think this is related to the change of the return type to FutureOr...

AlexV525 added a commit that referenced this pull request 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.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A request with an interceptor, which produces an error, makes the code stuck.
3 participants