[RFC][IcedTea-Web]: Add Logging of applet exceptions.
Andrew Su
asu at redhat.com
Tue Jan 11 18:46:00 PST 2011
On 01/11/2011 04:30 PM, Omair Majid wrote:
> On 01/11/2011 03:39 PM, Andrew Su wrote:
>>
>> ----- Original Message -----
>>>> From: "Andrew Su"<asu at redhat.com> To:
>>>> "OpenJDK"<distro-pkg-dev at openjdk.java.net> Sent: Tuesday, January
>>>> 11, 2011 2:19:57 PM Subject: [RFC][IcedTea-Web]: Add Logging of
>>>> applet exceptions. Hello,
>>>>
>>>> I have attached a patch which allows exceptions be logged to file
>>>> if the option "Enable logging" is set in deployments.property
>>>> file.
>>>>
>>>> Comments? Questions? Concerns?
>> Hi,
>>
>> I have updated the patch (removed unused import).
>
> Am I correct in understanding that this only log exceptions from
> init()/start()/stop()/destroy() ?
Yes.
>
>>
>> diff -r dc02a605f905 ChangeLog
>> --- a/ChangeLog Fri Jan 07 08:00:08 2011 -0500
>> +++ b/ChangeLog Tue Jan 11 15:37:29 2011 -0500
>> @@ -1,3 +1,13 @@
>> +2011-01-11 Andrew Su<asu at redhat.com>
>> +
>> + * makefile.am: Added net.sourceforge.jnlp.config to NETX_PKGS
>> + variable.
>
> Looks like a typo here (should be net.sourceforge.jnlp.debug). On that
> note, net.sourceforge.jnlp.config is not in NETX_PKGS either. That
> must have been my mistake.
Oops, I was going to put that in another patch, but accidentally typed
it there..
>
>> + * netx/net/sourceforge/jnlp/NetxPanel.java:
>> + (showAppletException): Override, adds logging to file. Then proceed
>> + with showAppletException in sun.applet.AppletPanel
>> + * netx/net/sourceforge/jnlp/debug/AppletLogger.java: New class.
>> + * netx/net/sourceforge/jnlp/debug/Log.java: New class.
>> +
>> 2011-01-04 Omair Majid<omajid at redhat.com>
>>
>> * netx/net/sourceforge/jnlp/security/KeyStores.java
>> diff -r dc02a605f905 Makefile.am
>> --- a/Makefile.am Fri Jan 07 08:00:08 2011 -0500
>> +++ b/Makefile.am Tue Jan 11 15:37:29 2011 -0500
>> @@ -29,7 +29,8 @@
>> net.sourceforge.jnlp.cache net.sourceforge.jnlp.event \
>> net.sourceforge.jnlp.security
>> net.sourceforge.jnlp.security.viewer \
>> net.sourceforge.jnlp.services net.sourceforge.jnlp.tools \
>> - net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel
>> + net.sourceforge.jnlp.util net.sourceforge.jnlp.controlpanel \
>> + net.sourceforge.jnlp.debug
>>
>> # Conditional defintions
>> if ENABLE_PLUGIN
>> diff -r dc02a605f905 netx/net/sourceforge/jnlp/NetxPanel.java
>> --- a/netx/net/sourceforge/jnlp/NetxPanel.java Fri Jan 07 08:00:08
>> 2011 -0500
>> +++ b/netx/net/sourceforge/jnlp/NetxPanel.java Tue Jan 11 15:37:29
>> 2011 -0500
>> @@ -22,6 +22,7 @@
>>
>> package net.sourceforge.jnlp;
>>
>> +import net.sourceforge.jnlp.debug.AppletLogger;
>> import net.sourceforge.jnlp.runtime.AppThreadGroup;
>> import net.sourceforge.jnlp.runtime.AppletInstance;
>> import net.sourceforge.jnlp.runtime.JNLPRuntime;
>> @@ -68,6 +69,12 @@
>> super.run();
>> }
>>
>> + @Override
>> + protected void showAppletException(Throwable t) {
>> + AppletLogger.log(t); // log the exception
>> + super.showAppletException(t);
>> + }
>> +
>
> Nicely done!
Thanks!
>
>> //Overriding to use Netx classloader. You might need to relax
>> visibility
>> //in sun.applet.AppletPanel for runLoader().
>> protected void runLoader() {
>> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/AppletLogger.java
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/netx/net/sourceforge/jnlp/debug/AppletLogger.java Tue Jan 11
>> 15:37:29 2011 -0500
>> @@ -0,0 +1,42 @@
>> +package net.sourceforge.jnlp.debug;
>> +
>> +import java.io.IOException;
>> +import java.util.logging.FileHandler;
>> +import java.util.logging.Level;
>> +import java.util.logging.Logger;
>> +import java.util.logging.XMLFormatter;
>> +
>> +/**
>> + * This class writes log information to file.
>> + * @author Andrew Su (asu at redhat.com,andrew.su at utoronto.ca)
>> + *
>> + */
>> +public class AppletLogger extends Log {
>
> This may be just nitpicking, but I dont like the names. "AppletLogger
> extends Log" sounds a little strange. Maybe use "AppletLog extends
> Log" or "AppletLogger extends Logger"?
Ah, you're not going to like the follow up patch for adding tracing,
"AppletTracer" guess I'll make that "AppletTrace". ;)
>
>> + public static Logger logger;
>> +
>> + static {
>> + try {
>> + // If logging is enabled, we create logger.
>> + if (enableLogging) {
>> + String fn = icedteaLogDir + "Plugin" +
>> java.lang.System.currentTimeMillis() + ".log";
>> + boolean append = false;
>> + FileHandler fh = new FileHandler(fn, append);
>> + fh.setFormatter(new XMLFormatter());
>
> Hmm.. is this line saying that logs should be written in XML?
Yes, this is to follow the format of how the proprietary plugin does
logging. This will keep any tools being used with those logs to continue
with our plugin's logs.
>
>> + logger =
>> Logger.getLogger(AppletLogger.class.getPackage().getName() + "." +
>> AppletLogger.class.getName());
>> + logger.setLevel(Level.ALL);
>> + logger.addHandler(fh);
>> + }
>> + } catch (IOException e) {
>> + e.printStackTrace();
>> + }
>> + }
>> +
>> + public synchronized static void log(Throwable e) {
>> + if (enableLogging) {
>> + String ex = e.toString();
>> + for (StackTraceElement ste : e.getStackTrace())
>> + ex = ex + "\n\tat " + ste;
>> + logger.log(Level.FINE, ex);
>> + }
>
> Perhaps using ex.printStackTrace(PrintStream) might be better? I dont
> know how well the simple for loop handles exceptions with causes.
Ah, yes, changed.
>
>> + }
>> +}
>> diff -r dc02a605f905 netx/net/sourceforge/jnlp/debug/Log.java
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/netx/net/sourceforge/jnlp/debug/Log.java Tue Jan 11 15:37:29
>> 2011 -0500
>> @@ -0,0 +1,46 @@
>> +package net.sourceforge.jnlp.debug;
>> +
>> +import java.io.File;
>> +
>> +import javax.naming.ConfigurationException;
>> +
>> +import net.sourceforge.jnlp.config.DeploymentConfiguration;
>> +
>> +/**
>> + * This file provides the information required to do logging.
>> + * @author Andrew Su (asu at redhat.com,andrew.su at utoronto.ca)
>> + *
>> + */
>> +public class Log {
>> +
>> + // Directory where the logs are stored.
>> + public static String icedteaLogDir;
>> +
>> + public static boolean enableLogging = false;
>> + public static boolean enableTracing = false;
>> +
>> + // Prepare for logging.
>> + static {
>> + DeploymentConfiguration config = new DeploymentConfiguration();
>> + try {
>> + config.load();
>> + } catch (ConfigurationException e) {
>> + e.printStackTrace();
>> + }
>> +
>
> This is done by JNLPRuntime.initialize() early in the plugin
> initialization phase. I dont think this is needed here at all.
Changed to calling JNLPRuntime.getConfiguration().
Thanks for looking it over, here is the updated patch with the said changes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20110111_add_logging_v4.patch
Type: text/x-patch
Size: 5447 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110111/1d2a6b55/20110111_add_logging_v4.patch
More information about the distro-pkg-dev
mailing list