You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by David Jencks <da...@yahoo.com> on 2007/06/26 23:07:12 UTC
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
I put the stack trace in there so you could find out what is
supplying the non-named xa resource without a lot of guessing. Why
do you think it is not useful?
thanks
david jencks
On Jun 25, 2007, at 6:41 PM, dwoods@apache.org wrote:
> Author: dwoods
> Date: Mon Jun 25 18:41:48 2007
> New Revision: 550656
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=550656
> Log:
> GERONIMO-3259 Unuseful exception stack trace in TransactionImpl.java
>
> Modified:
> geronimo/server/trunk/modules/geronimo-transaction/src/main/
> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>
> Modified: geronimo/server/trunk/modules/geronimo-transaction/src/
> main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> geronimo-transaction/src/main/java/org/apache/geronimo/transaction/
> manager/TransactionImpl.java?view=diff&rev=550656&r1=550655&r2=550656
> ======================================================================
> ========
> --- geronimo/server/trunk/modules/geronimo-transaction/src/main/
> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
> (original)
> +++ geronimo/server/trunk/modules/geronimo-transaction/src/main/
> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
> Mon Jun 25 18:41:48 2007
> @@ -708,7 +708,8 @@
> } else {
> // if it isn't a named resource should we really
> stop all processing here!
> // Maybe this would be better to handle else where
> and do we really want to prevent all processing of transactions?
> - new IllegalStateException("Cannot log transactions
> unles XAResources are named! " + committer).printStackTrace();
> + // new IllegalStateException("Cannot log
> transactions unles XAResources are named! " +
> committer).printStackTrace();
> + log.warn("Cannot log transactions as " + committer
> + " is not a NamedXAResource.");
> return committer.toString();
> }
> }
>
>
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by Vamsavardhana Reddy <c1...@gmail.com>.
On 6/28/07, Kevan Miller <ke...@gmail.com> wrote:
>
>
> On Jun 28, 2007, at 12:11 AM, David Jencks wrote:
>
> >
> > On Jun 26, 2007, at 7:08 PM, Donald Woods wrote:
> >
> >> Was just going on Kevan's response to YunFeng, that we shouldn't
> >> be using printStackTrace() in the code -
> >> http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-
> >> codes-tf3975719s134.html
>
> Heh. Sure... Blame it all on me... ;-)
;-)
>
> > I don't have a problem with logging the stack trace rather than
> > printing it to the console, but even printStackTrace IMO is not a
> > really big deal since the exception will only occur when someone
> > has written a broken integration of something that needs xa. For
> > instance I think openejb mdbs are currently broken this way.
>
> Right. I have no objections to logging stack traces, where we think
> they would be useful. My main point was that we should be thinking in
> terms of "logging" information, not "printing". The geronimo log
> should contain the information needed to identify/diagnose a problem
> (not a random mixture of logging and direct printing to STDOUT/STDERR).
I agree.
--kevan
>
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by Donald Woods <dw...@apache.org>.
Updated to log the stack trace as an error in revision 551929.
Donald Woods wrote:
> Agree. I'll update the log statement to include the stack trace.
>
> Thanks for reviewing and commenting on the change.
>
> -Donald
>
> Kevan Miller wrote:
>>
>> On Jun 28, 2007, at 12:11 AM, David Jencks wrote:
>>
>>>
>>> On Jun 26, 2007, at 7:08 PM, Donald Woods wrote:
>>>
>>>> Was just going on Kevan's response to YunFeng, that we shouldn't be
>>>> using printStackTrace() in the code -
>>>>
>>>> http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-codes-tf3975719s134.html
>>>>
>>
>> Heh. Sure... Blame it all on me... ;-)
>>
>>>
>>> I don't have a problem with logging the stack trace rather than
>>> printing it to the console, but even printStackTrace IMO is not a
>>> really big deal since the exception will only occur when someone has
>>> written a broken integration of something that needs xa. For
>>> instance I think openejb mdbs are currently broken this way.
>>
>> Right. I have no objections to logging stack traces, where we think
>> they would be useful. My main point was that we should be thinking in
>> terms of "logging" information, not "printing". The geronimo log
>> should contain the information needed to identify/diagnose a problem
>> (not a random mixture of logging and direct printing to STDOUT/STDERR).
>>
>> --kevan
>>
>>
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by Donald Woods <dw...@apache.org>.
Agree. I'll update the log statement to include the stack trace.
Thanks for reviewing and commenting on the change.
-Donald
Kevan Miller wrote:
>
> On Jun 28, 2007, at 12:11 AM, David Jencks wrote:
>
>>
>> On Jun 26, 2007, at 7:08 PM, Donald Woods wrote:
>>
>>> Was just going on Kevan's response to YunFeng, that we shouldn't be
>>> using printStackTrace() in the code -
>>> http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-codes-tf3975719s134.html
>>>
>
> Heh. Sure... Blame it all on me... ;-)
>
>>
>> I don't have a problem with logging the stack trace rather than
>> printing it to the console, but even printStackTrace IMO is not a
>> really big deal since the exception will only occur when someone has
>> written a broken integration of something that needs xa. For instance
>> I think openejb mdbs are currently broken this way.
>
> Right. I have no objections to logging stack traces, where we think they
> would be useful. My main point was that we should be thinking in terms
> of "logging" information, not "printing". The geronimo log should
> contain the information needed to identify/diagnose a problem (not a
> random mixture of logging and direct printing to STDOUT/STDERR).
>
> --kevan
>
>
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by Kevan Miller <ke...@gmail.com>.
On Jun 28, 2007, at 12:11 AM, David Jencks wrote:
>
> On Jun 26, 2007, at 7:08 PM, Donald Woods wrote:
>
>> Was just going on Kevan's response to YunFeng, that we shouldn't
>> be using printStackTrace() in the code -
>> http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-
>> codes-tf3975719s134.html
Heh. Sure... Blame it all on me... ;-)
>
> I don't have a problem with logging the stack trace rather than
> printing it to the console, but even printStackTrace IMO is not a
> really big deal since the exception will only occur when someone
> has written a broken integration of something that needs xa. For
> instance I think openejb mdbs are currently broken this way.
Right. I have no objections to logging stack traces, where we think
they would be useful. My main point was that we should be thinking in
terms of "logging" information, not "printing". The geronimo log
should contain the information needed to identify/diagnose a problem
(not a random mixture of logging and direct printing to STDOUT/STDERR).
--kevan
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by David Jencks <da...@yahoo.com>.
On Jun 26, 2007, at 7:08 PM, Donald Woods wrote:
> Was just going on Kevan's response to YunFeng, that we shouldn't be
> using printStackTrace() in the code -
> http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-
> codes-tf3975719s134.html
I don't have a problem with logging the stack trace rather than
printing it to the console, but even printStackTrace IMO is not a
really big deal since the exception will only occur when someone has
written a broken integration of something that needs xa. For
instance I think openejb mdbs are currently broken this way.
thanks
david jencks
>
> -Donald
>
> David Jencks wrote:
>> I put the stack trace in there so you could find out what is
>> supplying the non-named xa resource without a lot of guessing.
>> Why do you think it is not useful?
>> thanks
>> david jencks
>> On Jun 25, 2007, at 6:41 PM, dwoods@apache.org wrote:
>>> Author: dwoods
>>> Date: Mon Jun 25 18:41:48 2007
>>> New Revision: 550656
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=550656
>>> Log:
>>> GERONIMO-3259 Unuseful exception stack trace in TransactionImpl.java
>>>
>>> Modified:
>>> geronimo/server/trunk/modules/geronimo-transaction/src/main/
>>> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>>>
>>> Modified: geronimo/server/trunk/modules/geronimo-transaction/src/
>>> main/java/org/apache/geronimo/transaction/manager/
>>> TransactionImpl.java
>>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
>>> geronimo-transaction/src/main/java/org/apache/geronimo/
>>> transaction/manager/TransactionImpl.java?
>>> view=diff&rev=550656&r1=550655&r2=550656
>>> ====================================================================
>>> ==========
>>> --- geronimo/server/trunk/modules/geronimo-transaction/src/main/
>>> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>>> (original)
>>> +++ geronimo/server/trunk/modules/geronimo-transaction/src/main/
>>> java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>>> Mon Jun 25 18:41:48 2007
>>> @@ -708,7 +708,8 @@
>>> } else {
>>> // if it isn't a named resource should we really
>>> stop all processing here!
>>> // Maybe this would be better to handle else
>>> where and do we really want to prevent all processing of
>>> transactions?
>>> - new IllegalStateException("Cannot log
>>> transactions unles XAResources are named! " +
>>> committer).printStackTrace();
>>> + // new IllegalStateException("Cannot log
>>> transactions unles XAResources are named! " +
>>> committer).printStackTrace();
>>> + log.warn("Cannot log transactions as " +
>>> committer + " is not a NamedXAResource.");
>>> return committer.toString();
>>> }
>>> }
>>>
>>>
Re: svn commit: r550656 - /geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
Posted by Donald Woods <dw...@apache.org>.
Was just going on Kevan's response to YunFeng, that we shouldn't be using
printStackTrace() in the code -
http://www.nabble.com/Why-printStackTrace%28%29-in-the-source-codes-tf3975719s134.html
-Donald
David Jencks wrote:
> I put the stack trace in there so you could find out what is supplying
> the non-named xa resource without a lot of guessing. Why do you think
> it is not useful?
>
> thanks
> david jencks
>
> On Jun 25, 2007, at 6:41 PM, dwoods@apache.org wrote:
>
>> Author: dwoods
>> Date: Mon Jun 25 18:41:48 2007
>> New Revision: 550656
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=550656
>> Log:
>> GERONIMO-3259 Unuseful exception stack trace in TransactionImpl.java
>>
>> Modified:
>>
>> geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>>
>>
>> Modified:
>> geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>>
>> URL:
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java?view=diff&rev=550656&r1=550655&r2=550656
>>
>> ==============================================================================
>>
>> ---
>> geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>> (original)
>> +++
>> geronimo/server/trunk/modules/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
>> Mon Jun 25 18:41:48 2007
>> @@ -708,7 +708,8 @@
>> } else {
>> // if it isn't a named resource should we really stop
>> all processing here!
>> // Maybe this would be better to handle else where
>> and do we really want to prevent all processing of transactions?
>> - new IllegalStateException("Cannot log transactions
>> unles XAResources are named! " + committer).printStackTrace();
>> + // new IllegalStateException("Cannot log transactions
>> unles XAResources are named! " + committer).printStackTrace();
>> + log.warn("Cannot log transactions as " + committer +
>> " is not a NamedXAResource.");
>> return committer.toString();
>> }
>> }
>>
>>
>
>
>