karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cristiano Costantini <cristiano.costant...@gmail.com>
Subject Re: Problems due to the endorsed java.lang.Exception
Date Mon, 21 Mar 2016 08:13:00 GMT
Hi all,

Guillaume, could it be that pax logging works also if using "protected" in
the getClassContext method?

In fact I'm testing and comparing the two options, but I see that also with
the solution of making the endorsed class method like this:
    protected Class[] getClassContext() {
        return classContext;
    }

I am still getting the same logs
with [bundleId:bundleSymbolicName:bundleVersion] information





Il giorno dom 20 mar 2016 alle ore 09:24 Jean-Baptiste Onofré <
jb@nanthrax.net> ha scritto:

> +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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message