In some ways I agree with Owen - it's always better to have more
information about a problem. I can recall seeing blank error messages
or "null" on many occasions.
In other ways I agree with Dan and Anthony that this needs to not be a
global search&replace, but I think that's mostly because our alert
system is intertwined with our logging system. An operator wouldn't
know what do make of "UnknownHostException" in an alert but someone
doing forensic analysis might want to see that exception name in a log file.
On 5/28/19 10:03 AM, Anthony Baker wrote:
In the example you provided, I don’t agree that adding the exception class name
creates a better user experience.
Anthony
On May 25, 2019, at 6:39 PM, Owen Nichols <onich...@pivotal.io> wrote:
Here’s an example of a message that was logged before Jack’s change:
l192.168.99.1: nodename nor servname provided, or not known
Here’s what it will look like now with .toString() instead of .getMessage():
java.net.UnknownHostException: l192.168.99.1: nodename nor servname provided,
or not known
I cannot think of a single good reason to ever print an exception’s message stripped
of all context. As Jack noted, many exceptions don’t even have a message; the key
information is the class of the exception itself. Even if you aren’t catching
Exception but rather some very specific subclass, code should never make assumptions
about the text of an exception (see PR#3616
<https://github.com/apache/geode/pull/3616> for a recent example).
@jinmei There is no built-in method in java to capture the entire stacktrace as
a string. Exception::toString() just concatenates the exception class name and
message (i.e. the first line only of a full stacktrace)
On May 24, 2019, at 3:37 PM, Dan Smith <dsm...@pivotal.io> wrote:
I think the right thing to do in those 100+ cases may be different in each
case, a blanket search and replace might not be the best idea.
-Dan
On Fri, May 24, 2019 at 3:05 PM Jinmei Liao <jil...@pivotal.io> wrote:
does exception.toString() print out the entire stack trace? If so, I don't
think we should use that to replace exception.getMessage().
On Fri, May 24, 2019 at 1:18 PM Jack Weissburg <jweissb...@pivotal.io>
wrote:
Hi All,
Owen and I investigated a strange error message caused because Geode
previously handled an error exception with exception.getMessage() instead
of
exception.toString().
The issue is that the exception message from exception.getMessage() could
be unhelpful or empty. Instead, we should return the whole exception via
exception.toString().
exception.getMessage()is used widely throughout the Geode code base (100+
times) and could be prone to cause more issues similar to
https://issues.apache.org/jira/browse/GEODE-6796
I would like to change the remaining instances of
exception.getMessage()to
avoid future issues of this variety.
Thoughts? Comments?
Best,
Jack Weissburg
--
Cheers
Jinmei
