291129 - Add console logging to disk

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect, P1)

Description

[Benjamin Smedberg]

To aid in debugging xulrunner applications and even Firefox startup when we
don't get far enough to open a browser window, we need to be able to log console
errors to disk. In debug builds this will happen always; in release builds, this
will only happen if the startup code detects an unusual error condition that
might cause a main window to not open properly. An app author can force a log by
exporting XRE_CONSOLE_LOG=/path/to/log in the environment.

The log appends, so it works through our multiple-startup routines. I currently
have it written in the native charset, but I could easily be convinced to use UTF8.

Comment 1

[Benjamin Smedberg]

Attachment: Add console logging to disk, rev. 1

Comment 2

[Darin Fisher]

Hrm... why not just make the console service understand some environment
variable?  Then have it dump messages to the log file if that environment
variable is defined?  The console service has a fixed limit on the number of log
messages it can hold.  If we generate too many messages than the current code
will not log the initial messages.  Hmm... it would seem that you could also
just implement a nsIConsoleListener that writes to the log file when that
environment variable is set.  I don't see the need for all that code that tries
to enable console logging based on certain error conditions.

Comment 3

[Benjamin Smedberg]

Because I also want to log errors messages when there is *not* an envvar set, if
startup goes wrong, even *after* those messages have been logged. For example:
if a nsICommandLineHandler (implemented in JS) throws an exception, it will log
the error, but I won't know that I want to save it to disk until later on when
we check whether there is a window open.

Comment 4

[Darin Fisher]

Another question: it seems like you are trying to enable logging whenever
evaluation of a command line fails.  You appear to do that in the command line
runner code instead of in nsAppRunner.cpp because you want to trap cases where
the command line enters the code via one of the remoting technologies.  Why
bother?  I mean, if the remoting technology is being used, then wouldn't that
imply that an instance of the application is already running?  Why do we need to
invoke this logging in that case?  In other words, is it possible to constrain
all of the changes to nsAppRunner.cpp and not extend nsIXULAppInfo? 
nsIXULAppInfo is an interface we probably want to freeze at some point, and is
this flag really something we want to expose on that object?

Comment 5

[Benjamin Smedberg]

Attachment: Add console logging to disk, rev. 2

This removes the unnecessary code from toolkit/components/commandlines and
moves the method to the nsIXULRuntime interface. The rest of the impl remains
the same.

Comment 6

[Darin Fisher]

Comment on attachment 185270 [details] [diff] [review]
Add console logging to disk, rev. 2

>Index: toolkit/xre/nsAppRunner.cpp

>         PRBool more;
>         windowEnumerator->HasMoreElements(&more);
>+        if (!more) {

nit: since not checking return value of HasMoreElements, might be
a good idea to initialize |more| to some value.


>Index: toolkit/xre/nsConsoleWriter.cpp

>+    rv = gDirServiceProvider->GetUserAppDataDirectory(getter_AddRefs(lfile));
>+    if (NS_FAILED(rv))
>+      return;
>+
>+    lfile->AppendNative(NS_LITERAL_CSTRING("console.log"));
>+  }

maybe the temp directory would be a better place for this?


r=darin

Comment 7

[Benjamin Smedberg]

Comment on attachment 185270 [details] [diff] [review]
Add console logging to disk, rev. 2

I didn't want it in the temp dir, so that QA folk could tell users to "send
me/attach C:\Documents and Settings\...\console.log"

Comment 8

[Benjamin Smedberg]

Fixed on trunk for 1.8b3