[RFC][IcedTea-Web]: Add Logging of applet exceptions.
Omair Majid
omajid at redhat.com
Tue Jan 11 13:30:20 PST 2011
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() ?
>
> 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.
> + * 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!
> //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"?
> + 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?
> + 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.
> + }
> +}
> 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.
> + // Check whether logging and tracing is enabled.
> + enableLogging = Boolean.parseBoolean(config.getProperty(DeploymentConfiguration.KEY_ENABLE_LOGGING));
> + enableTracing = Boolean.parseBoolean(config.getProperty(DeploymentConfiguration.KEY_ENABLE_TRACING));
> +
> + // Get log directory, create it if it doesn't exist. If unable to create and doesn't exist, don't log.
> + icedteaLogDir = config.getProperty(DeploymentConfiguration.KEY_USER_LOG_DIR);
> + if (icedteaLogDir != null) {
> + File f = new File(icedteaLogDir);
> + if (f.isDirectory() || f.mkdirs())
> + icedteaLogDir += File.separator;
> + else
> + enableLogging = false;
> + }
> + }
> +
> +}
Cheers,
Omair
More information about the distro-pkg-dev
mailing list