It sounds like the consensus is: don’t change [the current questionable uses of getMessage] unless willing go a step further and provide a detailed, informative message, which may include the exception’s toString.
Thanks. -Owen > On May 28, 2019, at 3:32 PM, Nabarun Nag <n...@apache.org> wrote: > > Looking at the ticket, it looks like it is an improvement request for some > additional information for the end user, can we just do what Kirk and Bruce > suggested. Add some logs to explain what happened. In my opinion, an end > user will be more happy to get some detail information rather than an > exception class name. > > Regards > Nabarun Nag > > On Tue, May 28, 2019 at 3:08 PM Owen Nichols <onich...@pivotal.io> wrote: > >> This example came from https://issues.apache.org/jira/browse/GEODE-6796 >> in which the submitter assumed that Geode was deliberately emitting a >> poorly-worded and confusing error message. >> >> @abaker It sounds like your recommendation for this particular ticket >> would be to resolve as ’Not A Bug’, is that correct? >> >>> On May 28, 2019, at 10:03 AM, Anthony Baker <aba...@pivotal.io> 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 >>>>>> >>>> >>> >> >>
