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 >>> >
