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.

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

DeferredFutureTask and global CallbackExceptionHandler #138

Open
aalmiray opened this issue Oct 28, 2017 · 5 comments
Open

DeferredFutureTask and global CallbackExceptionHandler #138

aalmiray opened this issue Oct 28, 2017 · 5 comments
Labels
Milestone

Comments

@aalmiray
Copy link
Contributor

DeferredFutureTask has a pair of TODOs stating the need of re-routing an exception through CallbackExceptionHandler.. Problem is, the exception may occur during resolve or cleanup thus they do not occur under any of the 4 types of callbacks currently handled by CEH.

@aalmiray aalmiray added the bug label Oct 28, 2017
@aalmiray aalmiray added this to the 2.0 milestone Oct 28, 2017
@saturnism
Copy link
Member

Good thing I renamed the global exception handler to CallbackExceptionHandler. We can have a CancellationExceptionHandler?

@aalmiray
Copy link
Contributor Author

@saturnism
Copy link
Member

trying to lookup the code in the new package, but i don't think this is it: https://github.com/jdeferred/jdeferred/blob/2.x/subprojects/jdeferred-core/src/main/java/org/jdeferred2/DeferredFutureTask.java#L180-L183

@saturnism
Copy link
Member

saturnism commented Dec 29, 2017

ah, i think it's referring to https://github.com/jdeferred/jdeferred/blob/2.x/subprojects/jdeferred-core/src/main/java/org/jdeferred2/DeferredFutureTask.java#L188-L191

this might never happen since exceptions are handled by the global exception handler already in each of the calls, right?

If that's the case, would you prefer to have a new CallbackType for CANCEL_CALLBACK, and re-use the CallbackExceptionHandler?

@aalmiray
Copy link
Contributor Author

The thrown Throwablemay occur during deferred.resolve(get());. Given the previous catch blocks for InterruptedException and ExecutionException then the third catch will handle any other plumbing error.

I'd say let's keep it as is for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants