+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 <[email protected]> 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>:
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 <[email protected]>
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é <[email protected]>:
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é
[email protected]
http://blog.nanthrax.net
Talend - http://www.talend.com
--
------------------------
Guillaume Nodet
------------------------
Red Hat, Open Source Integration
Email: [email protected]
Web: http://fusesource.com
Blog: http://gnodet.blogspot.com/
--
Matt Sicker <[email protected]>
--
------------------------
Guillaume Nodet
------------------------
Red Hat, Open Source Integration
Email: [email protected]
Web: http://fusesource.com
Blog: http://gnodet.blogspot.com/
--
Matt Sicker <[email protected]>
--
Jean-Baptiste Onofré
[email protected]
http://blog.nanthrax.net
Talend - http://www.talend.com
