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

Carry first suppressed exception #2233

Merged
merged 4 commits into from
Dec 8, 2020
Merged

Conversation

sazzad16
Copy link
Collaborator

@sazzad16 sazzad16 commented Aug 25, 2020

within JedisNoReachableClusterNodeException

Resolves #2227
Closes #2228
Closes #2307

@gkorland
Copy link
Contributor

@sazzad16 why only the first exception and not all the exceptions like in #2228?
If we go with only the first exception why not set the exception as the cause?

edisNoReachableClusterNodeException noReachableNode = new JedisNoReachableClusterNodeException("No reachable node in cluster", superssed);

@sazzad16
Copy link
Collaborator Author

@gkorland This is to start a discussion about the approach. My disliking about #2228 is creating an exception (JedisNoReachableClusterNodeException) even when it is not necessary.

So, WDYT should be our approach? Suppressing all exception / first exception as cause / anything else?

@sazzad16
Copy link
Collaborator Author

@gkorland Please check #2228 regarding same issue.

@gkorland
Copy link
Contributor

gkorland commented Dec 7, 2020

@sazzad16 what about collecting all the suppressed exceptions in a list and only in case we need to throw an exception use this list?

@sazzad16
Copy link
Collaborator Author

sazzad16 commented Dec 7, 2020

@gkorland WDYT about #2228 ?

@gkorland
Copy link
Contributor

gkorland commented Dec 7, 2020

Yes I thought of some compromise, avoiding creating the exception in advance and still collect all the suppressed

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