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

retryableExceptions configuration and implementation #87

Open
spjoe opened this issue Dec 7, 2017 · 3 comments
Open

retryableExceptions configuration and implementation #87

spjoe opened this issue Dec 7, 2017 · 3 comments
Labels
Milestone

Comments

@spjoe
Copy link

spjoe commented Dec 7, 2017

Most likely

if (retryableExceptions.contains(e) || e.getCause() instanceof EOFException)

should be
if (retryableExceptions.contains(e.getClass()) || e.getCause() instanceof EOFException)
, because retryableExceptions is a set of Classes not a set of exceptions. But actually retryableExceptions.contains(e) can be removed all together, because it is implicit covered by line 44

A more general suggestion of my would be to make recoverable exceptions more configurable. Let users provide predicates if exceptions are recoverable or not, because more often than not you have a whole exception cause chain. And only checking type of the outermost exception does not suffice.

@acogoluegnes acogoluegnes added this to the 0.5.6 milestone Dec 7, 2017
@jhalterman
Copy link
Owner

The API could certainly support a Predicate to determine if a failure should be retryable, and the default predicate could match the retry exceptions list, which the user could still modify. If the user provides there own predicate, it would bypass the default one.

@acogoluegnes
Copy link
Collaborator

Predicate is a JDK 8 API and Lyra 0.5.x minimum JDK version is 1.6, so this would need to go into a major release, e.g. 1.x.

@spjoe
Copy link
Author

spjoe commented Dec 11, 2017

@acogoluegnes With 'Predicate' I meant a concept not the concrete jdk impl. So you can invent your own predicate if you like to be java 6 compatible.

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

3 participants