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

RetryException is not serializable bc Attempt is not #40

Open
eschultink opened this issue May 31, 2019 · 1 comment
Open

RetryException is not serializable bc Attempt is not #40

eschultink opened this issue May 31, 2019 · 1 comment

Comments

@eschultink
Copy link

RetryException implements Serialzable, but does not fulfill the contract, because it has a field Attempt<>:

public final class RetryException extends Exception {
private final Attempt<?> lastFailedAttempt;

which does not implement Serializable:

public class Attempt<T> {
private final T result;
private final Throwable throwable;
private final int attemptNumber;
private final long delaySinceFirstAttempt;

as well as being generic, which makes it non-obvious how to solve this. A few options:

  1. just make Attempt<> implement Serializable, ignoring potential problem with T not also being Serializable. Then it's up to users - if they're using Retryer with serializable return types, things will work; if not, no worse off than now.

  2. make result transient. should work, with caveat that if the instance has undergone serialization and deserialization, result will be lost, so in some sense hasResult()/getResult() won't respect contracts, as will be false/throw IllegalStateException, even though original call did have a result. But again, this only happens after Attempt has undergone a serialization+deserialization cycle, which currently throws an error, so I would argue that everyone's still better off.

  3. require T to implement Serializable. Probably not great solution, as would constrain users too much and breaks Retryer's interface.

I'd personally vote for (1) as the simplest, lowest impact change; but quick version of (2) might be just as good, if later followed by bigger change to make hasResult()/getResult() behave sensibly.

@jgiardin
Copy link

jgiardin commented Apr 5, 2021

@eschultink I am experiencing this serialization issue as well - Did you ever get resolution or a workaround? Or perhaps recommend a different retry library?

Thanks!

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

No branches or pull requests

2 participants