You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jack Weissburg <jw...@pivotal.io> on 2019/05/24 20:17:51 UTC
[DISCUSS] Remove exception.getMessage() error handling
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
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Kirk Lund <kl...@apache.org>.
I favor providing a detailed log message that includes the Throwable as an
argument.
I have seen code like the following in Geode:
} catch (Exception exception) {
logger.error(exception.getMessage());
}
...but I consider that to be a naive anti-pattern for informing the user
that a problem occurred. For one thing, getMessage() is not guaranteed to
have a non-null value even for Exceptions thrown by the JDK. For example,
NullPointerExceptions thrown by the JDK usually (if not always) have a null
message and the result in the Geode log is:
null
...which is clearly not useful and is very confusing. It actually just
looks like a bug.
I think any error-handling that logs an exception should include a log
message with at least some basic context about what was attempted that
resulted in the exception:
logger.error("Attempt to perform operation foo resulted in {}",
exception);
The above will then use exception.toString() for {} which provides much
better detail to the user about what problem actually occurred.
On Tue, May 28, 2019 at 10:50 AM Bruce Schuchardt <bs...@pivotal.io>
wrote:
> 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 <on...@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 <ds...@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 <ji...@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 <jweissburg@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
> >>>>
>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Bruce Schuchardt <bs...@pivotal.io>.
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 <on...@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 <ds...@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 <ji...@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 <jw...@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
>>>>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Owen Nichols <on...@pivotal.io>.
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 <nn...@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 <on...@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 <ab...@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 <on...@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 <ds...@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 <ji...@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 <jweissburg@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
>>>>>>
>>>>
>>>
>>
>>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Nabarun Nag <nn...@apache.org>.
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 <on...@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 <ab...@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 <on...@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 <ds...@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 <ji...@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 <jweissburg@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
> >>>>
> >>
> >
>
>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Owen Nichols <on...@pivotal.io>.
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 <ab...@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 <on...@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 <ds...@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 <ji...@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 <jw...@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
>>>>
>>
>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Anthony Baker <ab...@pivotal.io>.
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 <on...@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 <ds...@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 <ji...@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 <jw...@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
>>>
>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Owen Nichols <on...@pivotal.io>.
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 <ds...@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 <ji...@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 <jw...@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
>>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Dan Smith <ds...@pivotal.io>.
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 <ji...@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 <jw...@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
>
Re: [DISCUSS] Remove exception.getMessage() error handling
Posted by Jinmei Liao <ji...@pivotal.io>.
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 <jw...@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