diff --git a/services/common/logmanager.js b/services/common/logmanager.js index b6e026ea5d58..95805b67adac 100644 --- a/services/common/logmanager.js +++ b/services/common/logmanager.js @@ -142,20 +142,23 @@ LogManager.prototype = { this._prefs = null; }, - get _logFileDirectory() { - // At this point we don't allow a custom directory for the logs so - // about:sync-log can be used. We could fix this later if necessary. - return FileUtils.getDir("ProfD", ["weave", "logs"]); + get _logFileSubDirectoryEntries() { + // At this point we don't allow a custom directory for the logs, nor allow + // it to be outside the profile directory. + // This returns an array of the the relative directory entries below the + // profile dir, and is the directory about:sync-log uses. + return ["weave", "logs"]; }, /** * Copy an input stream to the named file, doing everything off the main * thread. - * outputFile is an nsIFile, but is used only for the name. - * Returns a promise that is resolved with the file modification date on - * completion or rejected if there is an error. + * outputFileName is a string with the tail of the filename - the file will + * be created in the log directory. + * Returns a promise that is resolved on completion or rejected if + * there is an error. */ - _copyStreamToFile: Task.async(function* (inputStream, outputFile) { + _copyStreamToFile: Task.async(function* (inputStream, outputFileName) { // The log data could be large, so we don't want to pass it all in a single // message, so use BUFFER_SIZE chunks. const BUFFER_SIZE = 8192; @@ -163,7 +166,11 @@ LogManager.prototype = { // get a binary stream let binaryStream = Cc["@mozilla.org/binaryinputstream;1"].createInstance(Ci.nsIBinaryInputStream); binaryStream.setInputStream(inputStream); - yield OS.File.makeDir(outputFile.parent.path, { ignoreExisting: true }); + // We assume the profile directory exists, but not that the dirs under it do. + let profd = FileUtils.getDir("ProfD", []); + let outputFile = FileUtils.getDir("ProfD", this._logFileSubDirectoryEntries); + yield OS.File.makeDir(outputFile.path, { ignoreExisting: true, from: profd.path }); + outputFile.append(outputFileName); let output = yield OS.File.open(outputFile.path, { write: true} ); try { while (true) { @@ -221,12 +228,10 @@ LogManager.prototype = { // We have reasonPrefix at the start of the filename so all "error" // logs are grouped in about:sync-log. let filename = reasonPrefix + "-" + this.logFilePrefix + "-" + Date.now() + ".txt"; - let file = this._logFileDirectory; - file.append(filename); - this._log.trace("Beginning stream copy to " + file.leafName + ": " + + this._log.trace("Beginning stream copy to " + filename + ": " + Date.now()); try { - yield this._copyStreamToFile(inStream, file); + yield this._copyStreamToFile(inStream, filename); this._log.trace("onCopyComplete", Date.now()); } catch (ex) { this._log.error("Failed to copy log stream to file", ex); @@ -256,7 +261,8 @@ LogManager.prototype = { */ cleanupLogs: Task.async(function* () { this._cleaningUpFileLogs = true; - let iterator = new OS.File.DirectoryIterator(this._logFileDirectory.path); + let logDir = FileUtils.getDir("ProfD", this._logFileSubDirectoryEntries); + let iterator = new OS.File.DirectoryIterator(logDir.path); let maxAge = this._prefs.get("log.appender.file.maxErrorAge", DEFAULT_MAX_ERROR_AGE); let threshold = Date.now() - 1000 * maxAge; diff --git a/services/common/tests/unit/test_logmanager.js b/services/common/tests/unit/test_logmanager.js index 125190850e9b..837618033eb0 100644 --- a/services/common/tests/unit/test_logmanager.js +++ b/services/common/tests/unit/test_logmanager.js @@ -6,6 +6,7 @@ Cu.import("resource://services-common/logmanager.js"); Cu.import("resource://gre/modules/Log.jsm"); +Cu.import("resource://gre/modules/FileUtils.jsm"); function run_test() { run_next_test(); @@ -101,3 +102,34 @@ add_task(function* test_SharedLogs() { lm1.finalize(); lm2.finalize(); }); + +// A little helper to test what log files exist. We expect exactly zero (if +// prefix is null) or exactly one with the specified prefix. +function checkLogFile(prefix) { + let logsdir = FileUtils.getDir("ProfD", ["weave", "logs"], true); + let entries = logsdir.directoryEntries; + if (!prefix) { + // expecting no files. + ok(!entries.hasMoreElements()); + } else { + // expecting 1 file. + ok(entries.hasMoreElements()); + let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); + equal(logfile.leafName.slice(-4), ".txt"); + ok(logfile.leafName.startsWith(prefix + "-test-"), logfile.leafName); + // and remove it ready for the next check. + logfile.remove(false); + } +} + +// Test that we correctly write error logs by default +add_task(function* test_logFileErrorDefault() { + let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + + let log = Log.repository.getLogger("TestLog2"); + log.error("an error message"); + yield lm.resetFileLog(lm.REASON_ERROR); + // One error log file exists. + checkLogFile("error"); + lm.finalize(); +});