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.

Sign up for GitHub

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

Jump to bottom

[Netty 5] DefaultPromise may throw checked exceptions that are not advertised #8521

Closed
normanmaurer opened this issue Nov 13, 2018 · 1 comment
Closed

[Netty 5] DefaultPromise may throw checked exceptions that are not advertised #8521

normanmaurer opened this issue Nov 13, 2018 · 1 comment
Assignees
@normanmaurer
Labels
cleanup defect
Projects
Netty 5
Milestone
[Netty 5] Future...

Comments

@normanmaurer
Copy link
Member

At the moment all sync* methods that are implemented by DefaultPromise may throw checked exceptions which are not adviced. This is bad for multiple reasons. We should just wrap the checked exceptions with a RuntimeException.
@normanmaurer normanmaurer added this to To do in Netty 5 via automation Nov 13, 2018
@normanmaurer normanmaurer self-assigned this Apr 1, 2019
@normanmaurer normanmaurer moved this from To do to In progress in Netty 5 Apr 1, 2019
@normanmaurer
Copy link
Member Author

Let me work on this and throw CompletionException as CompletableFuture does.

@normanmaurer normanmaurer mentioned this issue Apr 1, 2019
Merged
normanmaurer added a commit that referenced this issue Apr 1, 2019
@normanmaurer
DefaultPromise may throw checked exceptions that are not advertised
fea799c 
Motivation:

We should not throw check exceptions when the user calls sync*() but should better wrap it in a CompletionException to make it easier for people to reason about what happens.

Modifications:

- Change sync*() to throw CompletionException
- Adjust tests
- Add some more tests

Result:

Fixes #8521.
normanmaurer added a commit that referenced this issue Apr 10, 2019
@normanmaurer
DefaultPromise may throw checked exceptions that are not advertised (#…
7c35781 
…8995)

Motivation:

We should not throw check exceptions when the user calls sync*() but should better wrap it in a CompletionException to make it easier for people to reason about what happens.

Modifications:

- Change sync*() to throw CompletionException
- Adjust tests
- Add some more tests

Result:

Fixes #8521.
Netty 5 automation moved this from In progress to Done Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@normanmaurer normanmaurer

Labels
cleanup defect
Projects
Netty 5
  
Done
Milestone
[Netty 5] Futures / Promises enhance...
Development

No branches or pull requests
1 participant
@normanmaurer