8000 Propagate exceptions for submitted/scheduled tasks by tbruyelle · Pull Request #727 · androidannotations/androidannotations · GitHub 8000
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Conversation

@tbruyelle
8000 Copy link
Contributor

Fix improvement for #646

For remind, the goal of #646 was to stop catching exception in @Backgound annotated methods, in order to let the user deals with it.

First shot was to simply remove the try/catch blocks, but as discussed here, there is a problem because ExecutorService.submit/schedule doesn't propagate exception during executions. This is by design. As a result, exceptions thrown are not logged and not visible.

To handle that, I suggest a solution :
Call futur.get() (in another thread pool) to check if an exception occurred during execution. If yes then throw it so it could be caught by the system.

I admit it looks like a nasty trick, but I didn't find a better solution. If someone has, I will be happy to read it.

@DayS
Copy link
Contributor
DayS commented Sep 20, 2013

This should work yes, but I still don't like this solution, even if I haven't a better idea right now :/

Note: you should update your pull-request. There are some conflicts right now

@tbruyelle
Copy link
Contributor Author

Ok I didn't know you didn't like the solution, so I finally decided to create the PR in order to up the thing.

The issue about invisible exceptions is a real problem for me because I want to keep track of all unexpected exceptions (like every java developer I presume :)). That says, I also don't appreciate a lot the solution, I hope someone could find a better one.

@DayS
Copy link
Contributor
DayS commented Sep 20, 2013

We should brainstorm on this point. In the meantime, I think we should keep this PR opened in case someone needs it.

@DayS
Copy link
Contributor
DayS commented Oct 18, 2013

We merged @JoanZapata 's PR. Thanks anyway :)

@DayS DayS closed this Oct 18, 2013
@tbruyelle
Copy link
Contributor Author

Np this is a better solution

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0