karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jean-Baptiste Onofré ...@nanthrax.net>
Subject Re: Problems due to the endorsed java.lang.Exception
Date Sun, 20 Mar 2016 08:24:36 GMT
+1

I think @XmlTransient is sufficient and limit the impacts.

Regards
JB

On 03/20/2016 09:17 AM, Cristiano Costantini wrote:
> Hello all,
>
> I like the idea of having "bundle id, symbolic name and version" in the
> exception logs, I've used it a lot and I think we should find a solution
> that is as clean as possible and works well by keep this enhanced logging
> capability alive.
>
> The solution of marking the method with @XmlTransient is good and would
> fully solve the CXF problem (I'll try this one tomorrow morning and give
> you a confirmation about this approach).
>
> Also, JaxB sees it as a property because it is a getter, and try to
> marshall it because JaxB default behavior is to marshall properties.
> Then I propose also, for cleanliness to @Deprecate the getClassContext
> method and create a new one called in a non property way, for example
> simply classContext(), without the get.
>
> If Guillaume in pax-logging is using reflection and may be able to access
> the method even if it is protected, then it should be "protected", again
> for cleanliness.
>
>
> I was thinking at what other problems could be caused by this endorsement
> of the class:
> the underlying classContext field is correctly marked as transient, so I
> expect no problem due to serialization, even with third party libraries
> (for example Kryo) which respect the transient modifier.
> Except for JaxB, I don't know any other use case where exceptions may be
> accessed with reflection in search for methods.
>
> So to summarize:
>
> @XmlTransient @Deprecated public getClassContext() { ... }
> +
> protected classContext() { ... }
>
> seems to me the best option.
>
>
> What do you think?
>
> Cristiano
>
>
>
>
>
> Il giorno dom 20 mar 2016 alle ore 04:04 Matt Sicker <boards@gmail.com> ha
> scritto:
>
>> If there's a way to fix this in log4j, that would be better.
>>
>> On 19 March 2016 at 06:02, James Carman <james@carmanconsulting.com>
>> wrote:
>>
>>> That's kind of a nasty trick just to get pretty stack traces.  Is this
>>> really necessary?
>>>
>>> On Fri, Mar 18, 2016 at 3:05 PM Guillaume Nodet <gnodet@apache.org>
>> wrote:
>>>
>>>> I wrote that years ago and nobody complained about it, until today....
>>>> Adding the @XmlTransient should be a good and quick fix.
>>>>
>>>> The benefit of the custom Exception is that this information is stored
>>>> along with the exception. This information is always lost when you need
>>> it
>>>> to render the exception, as the stack trace may be completely different
>>> and
>>>> even in a from different thread, which mean you loose the information.
>> So
>>>> ReflectionUtil.getCurrentStackTrace() works the same, but the
>> information
>>>> is the one of the calling code, not the one of the exception, and
>> that's
>>>> useless.
>>>>
>>>> If you look at a stack trace in Karaf, it looks like the following:
>>>>
>>>> org.apache.karaf.shell.support.MultiException: Error installing
>> bundles:
>>>>
>>>> Unable to install bundle foo
>>>>
>>>> at
>>>>
>>>>
>>>
>> org.apache.karaf.shell.support.MultiException.throwIf(MultiException.java:61)
>>>> ~[28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.karaf.bundle.command.Install.execute(Install.java:113)
>>>> [12:org.apache.karaf.bundle.core:4.1.0.SNAPSHOT]
>>>>
>>>> at
>>>>
>>>>
>>>
>> org.apache.karaf.shell.impl.action.command.ActionCommand.execute(ActionCommand.java:84)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at
>>>>
>>>>
>>>
>> org.apache.karaf.shell.impl.console.osgi.secured.SecuredCommand.execute(SecuredCommand.java:67)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at
>>>>
>>>>
>>>
>> org.apache.karaf.shell.impl.console.osgi.secured.SecuredCommand.execute(SecuredCommand.java:87)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.felix.gogo.runtime.Closure.executeCmd(Closure.java:548)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at
>>> org.apache.felix.gogo.runtime.Closure.executeStatement(Closure.java:474)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.felix.gogo.runtime.Closure.execute(Closure.java:363)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.felix.gogo.runtime.Pipe.doCall(Pipe.java:417)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.felix.gogo.runtime.Pipe.call(Pipe.java:227)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at org.apache.felix.gogo.runtime.Pipe.call(Pipe.java:59)
>>>> [28:org.apache.karaf.shell.core:4.1.0.SNAPSHOT]
>>>>
>>>> at java.util.concurrent.FutureTask.run(FutureTask.java:266) [?:?]
>>>>
>>>> at
>>>>
>>>>
>>>
>> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
>>>> [?:?]
>>>>
>>>> at
>>>>
>>>>
>>>
>> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
>>>> [?:?]
>>>>
>>>> at java.lang.Thread.run(Thread.java:745) [?:?]
>>>>
>>>> The information in the brackets, at the end of each line, is actually
>>>> accurate (bundle id, symbolic name, version).  Without the information
>>>> stored in the Exception, I'm not aware of any way to do that
>>> unfortunately.
>>>>
>>>> 2016-03-18 18:41 GMT+01:00 Matt Sicker <boards@gmail.com>:
>>>>
>>>>> What's wrong with the class context from
>>>>> ReflectionUtil.getCurrentStackTrace() that requires a custom
>>>> implementation
>>>>> of Exception?
>>>>>
>>>>> On 18 March 2016 at 12:11, Guillaume Nodet <gnodet@apache.org>
>> wrote:
>>>>>
>>>>>> The getClassContext() method is used in pax-logging to render the
>>>>> exception
>>>>>> in a nicer way.
>>>>>> I don't have any problem moving the qualifier to protected or
>>> whatever,
>>>>> but
>>>>>> we'd need to change the code in pax-logging
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>> https://github.com/ops4j/org.ops4j.pax.logging/blob/master/pax-logging-log4j2/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java#L136-L144
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>> https://github.com/ops4j/org.ops4j.pax.logging/blob/master/pax-logging-service/src/main/java/org/apache/log4j/OsgiThrowableRenderer.java#L62-L67
>>>>>>
>>>>>> It should not be a big deal, but the current code expect the method
>>> to
>>>> be
>>>>>> public unfortunately.
>>>>>>
>>>>>> Guillaume
>>>>>>
>>>>>>
>>>>>> 2016-03-18 17:29 GMT+01:00 Jean-Baptiste Onofré <jb@nanthrax.net>:
>>>>>>
>>>>>>> Hi Cristiano,
>>>>>>>
>>>>>>> I think you are right, it's probably something existing for a
>>> while.
>>>>> I'm
>>>>>>> just surprised that you encountered now.
>>>>>>>
>>>>>>> Let me check your detailed use case that you sent previously.
>>>>>>>
>>>>>>> Regards
>>>>>>> JB
>>>>>>>
>>>>>>>
>>>>>>> On 03/18/2016 05:14 PM, Cristiano Costantini wrote:
>>>>>>>
>>>>>>>> Hello all,
>>>>>>>>
>>>>>>>> it is some days I was having troubles using CXF services
on
>>>> ServiceMix
>>>>>>>> (and
>>>>>>>> it is some days I spam on the mailing list of ServiceMix,
Camel
>>> and
>>>>> CXF
>>>>>>>> :-D
>>>>>>>> ) and I've finally come to the source of my issues which
are due
>>> to
>>>>> the
>>>>>>>> "endorsed" java.lang.Exception which is used by Karaf:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>> https://github.com/apache/karaf/blob/master/exception/src/main/java/java/lang/Exception.java
>>>>>>>>
>>>>>>>> My problem is due to the fact that this class has a public
>>> property,
>>>>>>>> public Class[] getClassContext() {
>>>>>>>>       return classContext;
>>>>>>>> }
>>>>>>>> which is seen by JaxB at runtime inside Karaf which cause
it to
>>>>> marshall
>>>>>>>> the Exception in SOAP services with a wrong XML, an XML which
>>>> include
>>>>>>>> <classContext> tags.
>>>>>>>>
>>>>>>>> The class in Karaf has not changed since committed by Guillaume
>>>> Nodet
>>>>> in
>>>>>>>> 2010, so this issue with CXF has probably always been there.
>>>>>>>>
>>>>>>>> In my opinion, this getter should be private or protected
as I
>>> guess
>>>>> it
>>>>>> is
>>>>>>>> only used on the nested class method's getThrowableContext
(it
>> is
>>> a
>>>>>>>> replacement for the JRE's class, I don't think there are
>> external
>>>>>> project
>>>>>>>> using this modification of the Exception class).
>>>>>>>>
>>>>>>>>
>>>>>>>> To confirm my thesis, I've hacked the
>>>>>> org.apache.karaf.exception-2.4.0.jar
>>>>>>>> inside the lib/endorsed folder with a class compiled by myself
>>> where
>>>>>> I've
>>>>>>>> changed the method to
>>>>>>>> private Class[] getClassContext() {
>>>>>>>>       return classContext;
>>>>>>>> }
>>>>>>>> and with this hack, Karaf has continued to work (and my CXF
>>> service
>>>>> now
>>>>>>>> works properly).
>>>>>>>>
>>>>>>>> Do you think there are potential side effects in this fix?
>>>>>>>> Do you need me to open an issue on Jira to handle the problem?
>>>>>>>> If it is confirmed that the fix of making this method private,
>> do
>>>> you
>>>>>>>> think
>>>>>>>> it could be included soon on the next Karaf Release?
>>>>>>>>
>>>>>>>> Thank you very much to all,
>>>>>>>> Cristiano
>>>>>>>>
>>>>>>>>
>>>>>>> --
>>>>>>> Jean-Baptiste Onofré
>>>>>>> jbonofre@apache.org
>>>>>>> http://blog.nanthrax.net
>>>>>>> Talend - http://www.talend.com
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> ------------------------
>>>>>> Guillaume Nodet
>>>>>> ------------------------
>>>>>> Red Hat, Open Source Integration
>>>>>>
>>>>>> Email: gnodet@redhat.com
>>>>>> Web: http://fusesource.com
>>>>>> Blog: http://gnodet.blogspot.com/
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <boards@gmail.com>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> ------------------------
>>>> Guillaume Nodet
>>>> ------------------------
>>>> Red Hat, Open Source Integration
>>>>
>>>> Email: gnodet@redhat.com
>>>> Web: http://fusesource.com
>>>> Blog: http://gnodet.blogspot.com/
>>>>
>>>
>>
>>
>>
>> --
>> Matt Sicker <boards@gmail.com>
>>
>

-- 
Jean-Baptiste Onofré
jbonofre@apache.org
http://blog.nanthrax.net
Talend - http://www.talend.com

Mime
View raw message