ARTEMIS-4171/ARTEMIS-4233 Large Message Send #4430
Conversation
clebertsuconic
force-pushed
the
ARTEMIS-4233
branch
2 times, most recently
from
de772ac
to
b29932e
Compare
| logger.warn("Throwable on currentLargeMessage {}", message.getMessageID(), e); | ||
| logger.warn("********************************************************************************"); | ||
|
|
||
| logger.warn(e.getMessage(), e); |
There was a problem hiding this comment.
Putting this here and removing the 'catch(Exception e)' means it will begin printing warnings with stacktraces for every exception thrown in send now, vs none before, even though its still throwing the exception onward, to someplace that may also print it if not handling it specifically. Its not clear to me thats desirable now if it was specifically avoided before, I expect it to generate a bunch of stacktraces people will then report as 'new errors' as they arent used to it.
Previously it only logged warning for the 'unhandled' Error's it then swallowed. You've changed it to rethrow those too. Perhaps just make a debug log for the Exceptions if wanting to add potential visibility, and retain the warning level for the Throwables?
Using 't' is more common for throwables than 'e'.
There was a problem hiding this comment.
I thought the system was not really supposed to throw an exception at this stage, and logging it would be only beneficial to finding what's going in a production system.
I'm dealing with other cases where it's being hard to identify issues, and I was trying to avoid that pattern here.
One thing is sending an expected exception to the client... (expected as it's something related to the client's configuration...etc..) the other is a server's exception not logging anything and only the client being informed about it.
There was a problem hiding this comment.
The code this method is calling looks to throw exceptions in fairly 'typical' situations, e.g security checks, disk usage checks. I think starting to log a full stacktrace for every one of them, when it specifically didnt before (presumably as the code calling this method should either handle, or itself print, them), is likely asking for a lot of new questions. Along the lines of the more typical 'I suddenly got this new error <provides non-error info log message and/or stacktrace>'.
clebertsuconic
force-pushed
the
ARTEMIS-4233
branch
from
b29932e
to
9b37a28
Compare
This is not really supposed to happen but I'm dealing with it just in case. In case it happened I would like to see the message removed and an exception thrown to the client