-
Notifications
You must be signed in to change notification settings - Fork 157
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
Catch errors in Dispatchers and log #1200
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} catch (Error err) { | ||
// Make sure we log, so we don't silently exit our loop. | ||
// https://github.com/nats-io/nats.java/issues/1198 | ||
err.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about
connection.processException(new Exception(err));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning more toward logging and failing, than still processing the error and continuing on.
I'm thinking an Error
indicates something that's not recoverable, versus an Exception
being something that's just unexpected or can be retried.
In the case that the Error
is not recoverable, there is probably not much we can do to keep running anyway. Might as well log and quit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to indeed process as exception and continue.
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
85a9dd8
to
0714961
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves #1198
We could handle the
Error
in theErrorListener
just like theException
is handled.But, an
Error
is likely to not be recoverable so probably stopping the dispatcher is the way to go? Just need to log that it happened.