diff --git a/services/common/moz.build b/services/common/moz.build index 144ccaff047a..42c5431de82e 100644 --- a/services/common/moz.build +++ b/services/common/moz.build @@ -22,7 +22,6 @@ EXTRA_JS_MODULES["services-common"] += [ "kinto-http-client.sys.mjs", "kinto-offline-client.sys.mjs", "kinto-storage-adapter.sys.mjs", - "logmanager.sys.mjs", "observers.sys.mjs", "rest.sys.mjs", "uptake-telemetry.sys.mjs", diff --git a/services/common/tests/unit/test_load_modules.js b/services/common/tests/unit/test_load_modules.js index d86165266f77..f188acb4f512 100644 --- a/services/common/tests/unit/test_load_modules.js +++ b/services/common/tests/unit/test_load_modules.js @@ -6,12 +6,7 @@ const { AppConstants } = ChromeUtils.importESModule( ); const MODULE_BASE = "resource://services-common/"; -const shared_modules = [ - "async.sys.mjs", - "logmanager.sys.mjs", - "rest.sys.mjs", - "utils.sys.mjs", -]; +const shared_modules = ["async.sys.mjs", "rest.sys.mjs", "utils.sys.mjs"]; const non_android_modules = ["tokenserverclient.sys.mjs"]; @@ -51,6 +46,7 @@ function expectImportsToFail(mm, base = MODULE_BASE) { function run_test() { expectImportsToSucceed(shared_modules); expectImportsToSucceed(shared_test_modules, TEST_BASE); + expectImportsToSucceed(["LogManager.sys.mjs"], "resource://gre/modules/"); if (AppConstants.platform != "android") { expectImportsToSucceed(non_android_modules); diff --git a/services/common/tests/unit/xpcshell.toml b/services/common/tests/unit/xpcshell.toml index e4035f66b273..35c10dfce334 100644 --- a/services/common/tests/unit/xpcshell.toml +++ b/services/common/tests/unit/xpcshell.toml @@ -20,8 +20,6 @@ tags = "blocklist" ["test_load_modules.js"] -["test_logmanager.js"] - ["test_observers.js"] ["test_restrequest.js"] diff --git a/services/fxaccounts/FxAccountsCommon.sys.mjs b/services/fxaccounts/FxAccountsCommon.sys.mjs index 0533b92e1479..18f129af38c1 100644 --- a/services/fxaccounts/FxAccountsCommon.sys.mjs +++ b/services/fxaccounts/FxAccountsCommon.sys.mjs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { Log } from "resource://gre/modules/Log.sys.mjs"; -import { LogManager } from "resource://services-common/logmanager.sys.mjs"; +import { LogManager } from "resource://gre/modules/LogManager.sys.mjs"; // loglevel should be one of "Fatal", "Error", "Warn", "Info", "Config", // "Debug", "Trace" or "All". If none is specified, "Debug" will be used by @@ -29,7 +29,13 @@ let logs = [ ]; // For legacy reasons, the log manager still thinks it's part of sync. -export let logManager = new LogManager("services.sync.", logs, "sync"); +export let logManager = new LogManager({ + prefRoot: "services.sync.", + logNames: logs, + logFilePrefix: "sync", + logFileSubDirectoryEntries: ["weave", "logs"], + testTopicPrefix: "services-tests:common:log-manager:", +}); // A boolean to indicate if personally identifiable information (or anything // else sensitive, such as credentials) should be logged. diff --git a/services/common/logmanager.sys.mjs b/toolkit/modules/LogManager.sys.mjs similarity index 87% rename from services/common/logmanager.sys.mjs rename to toolkit/modules/LogManager.sys.mjs index 724cfde38b51..ee8f3fcee8db 100644 --- a/services/common/logmanager.sys.mjs +++ b/toolkit/modules/LogManager.sys.mjs @@ -1,8 +1,15 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -"use strict;"; +/** + * This module provides a file-based, persistent logging facility for scenarios where + * retaining those logs over time and across browser restarts is important. + * Unless you need this feature specifically, please use console.createInstance. + */ + +// See Bug 1889052 +// eslint-disable-next-line mozilla/use-console-createInstance import { Log } from "resource://gre/modules/Log.sys.mjs"; const lazy = {}; @@ -120,10 +127,13 @@ class StorageStreamAppender extends Log.Appender { } } -// A storage appender that is flushable to a file on disk. Policies for -// when to flush, to what file, log rotation etc are up to the consumer -// (although it does maintain a .sawError property to help the consumer decide -// based on its policies) +/** + * A storage appender that is flushable to a file on disk. + * + * Policies for when to flush, to what file, log rotation etc are up to the consumer + * (although it does maintain a .sawError property to help the consumer decide + * based on its policies) + */ class FlushableStorageAppender extends StorageStreamAppender { constructor(formatter) { super(formatter); @@ -142,8 +152,12 @@ class FlushableStorageAppender extends StorageStreamAppender { this.sawError = false; } - // Flush the current stream to a file. Somewhat counter-intuitively, you - // must pass a log which will be written to with details of the operation. + /** + * Flush the current stream to a file. + * + * Somewhat counter-intuitively, you must pass a log which will be written to + * with details of the operation. + */ async flushToFile(subdirArray, filename, log) { let inStream = this.getInputStream(); this.reset(); @@ -194,22 +208,39 @@ class FlushableStorageAppender extends StorageStreamAppender { } } -// The public LogManager object. -export function LogManager(prefRoot, logNames, logFilePrefix) { - this._prefObservers = []; - this.init(prefRoot, logNames, logFilePrefix); -} +/** + * Each LogManager monitors preferences, resolves log levels and verbosity, + * and manages the creation, rotation and clean up of log files in a profile subdirectory. + */ +export class LogManager { + constructor(options = {}) { + this._prefObservers = []; + this.#init(options); + } -LogManager.StorageStreamAppender = StorageStreamAppender; + static StorageStreamAppender = StorageStreamAppender; -LogManager.prototype = { - _cleaningUpFileLogs: false, + _cleaningUpFileLogs = false; - init(prefRoot, logNames, logFilePrefix) { + #init({ + prefRoot, + logNames, + logFilePrefix, + logFileSubDirectoryEntries, + testTopicPrefix, + } = {}) { this._prefs = Services.prefs.getBranch(prefRoot); this._prefsBranch = prefRoot; this.logFilePrefix = logFilePrefix; + this._testTopicPrefix = testTopicPrefix; + + // 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. + this.logFileSubDirectoryEntries = Object.freeze(logFileSubDirectoryEntries); + if (!formatter) { // Create a formatter and various appenders to attach to the logs. formatter = new Log.BasicFormatter(); @@ -284,7 +315,7 @@ LogManager.prototype = { } // and use the first specified log as a "root" for our log. this._log = Log.repository.getLogger(logNames[0] + ".LogManager"); - }, + } /** * Cleanup this instance @@ -298,23 +329,15 @@ LogManager.prototype = { allBranches.delete(this._prefsBranch); } catch (e) {} this._prefs = null; - }, - - 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"]; - }, + } get sawError() { return this._fileAppender.sawError; - }, + } // Result values for resetFileLog. - SUCCESS_LOG_WRITTEN: "success-log-written", - ERROR_LOG_WRITTEN: "error-log-written", + SUCCESS_LOG_WRITTEN = "success-log-written"; + ERROR_LOG_WRITTEN = "error-log-written"; /** * Possibly generate a log file for all accumulated log messages and refresh @@ -357,7 +380,7 @@ LogManager.prototype = { let filename = reasonPrefix + "-" + this.logFilePrefix + "-" + Date.now() + ".txt"; await this._fileAppender.flushToFile( - this._logFileSubDirectoryEntries, + this.logFileSubDirectoryEntries, filename, this._log ); @@ -379,7 +402,7 @@ LogManager.prototype = { this._log.error("Failed to resetFileLog", ex); return null; } - }, + } /** * Finds all logs older than maxErrorAge and deletes them using async I/O. @@ -396,22 +419,24 @@ LogManager.prototype = { return fileInfo.lastModified < threshold; }; return this._deleteLogFiles(shouldDelete); - }, + } /** * Finds all logs and removes them. */ removeAllLogs() { return this._deleteLogFiles(() => true); - }, + } - // Delete some log files. A callback is invoked for each found log file to - // determine if that file should be removed. + /** + * Delete some log files. A callback is invoked for each found log file to + * determine if that file should be removed. + */ async _deleteLogFiles(cbShouldDelete) { this._cleaningUpFileLogs = true; let logDir = lazy.FileUtils.getDir( "ProfD", - this._logFileSubDirectoryEntries + this.logFileSubDirectoryEntries ); for (const path of await IOUtils.getChildren(logDir.path)) { const name = PathUtils.filename(path); @@ -439,9 +464,12 @@ LogManager.prototype = { this._cleaningUpFileLogs = false; this._log.debug("Done deleting files."); // This notification is used only for tests. - Services.obs.notifyObservers( - null, - "services-tests:common:log-manager:cleanup-logs" - ); - }, -}; + if (this._testTopicPrefix) { + Services.obs.notifyObservers( + null, + `${this._testTopicPrefix}cleanup-logs` + ); + ("cleanup-logs"); + } + } +} diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index ab7dcb284bd8..66730ce815cb 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -186,6 +186,7 @@ EXTRA_JS_MODULES += [ "KeywordUtils.sys.mjs", "LayoutUtils.sys.mjs", "Log.sys.mjs", + "LogManager.sys.mjs", "NewTabUtils.sys.mjs", "NLP.sys.mjs", "ObjectUtils.sys.mjs", diff --git a/services/common/tests/unit/test_logmanager.js b/toolkit/modules/tests/xpcshell/test_LogManager.js similarity index 86% rename from services/common/tests/unit/test_logmanager.js rename to toolkit/modules/tests/xpcshell/test_LogManager.js index 89ac274e61fc..fa5e5abc2e79 100644 --- a/services/common/tests/unit/test_logmanager.js +++ b/toolkit/modules/tests/xpcshell/test_LogManager.js @@ -5,11 +5,18 @@ // other aspects of this. const { LogManager } = ChromeUtils.importESModule( - "resource://services-common/logmanager.sys.mjs" + "resource://gre/modules/LogManager.sys.mjs" +); +const { Log } = ChromeUtils.importESModule( + "resource://gre/modules/Log.sys.mjs" ); const { FileUtils } = ChromeUtils.importESModule( "resource://gre/modules/FileUtils.sys.mjs" ); +const logManagerDefaultOptions = { + logFileSubDirectoryEntries: ["weave", "logs"], + testTopicPrefix: "services-tests:common:log-manager:", +}; // Returns an array of [consoleAppender, dumpAppender, [fileAppenders]] for // the specified log. Note that fileAppenders will usually have length=1 @@ -27,7 +34,12 @@ function getAppenders(log) { // Test that the correct thing happens when no prefs exist for the log manager. add_task(async function test_noPrefs() { // tell the log manager to init with a pref branch that doesn't exist. - let lm = new LogManager("no-such-branch.", ["TestLog"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "no-such-branch.", + logNames: ["TestLog"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog"); let [capp, dapp, fapps] = getAppenders(log); @@ -51,7 +63,12 @@ add_task(async function test_PrefChanges() { "log-manager.test.log.appender.file.level", "Trace" ); - let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager.test.", + logNames: ["TestLog2"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog2"); let [capp, dapp, [fapp]] = getAppenders(log); @@ -96,7 +113,12 @@ add_task(async function test_SharedLogs() { "log-manager-1.test.log.appender.file.level", "Trace" ); - let lm1 = new LogManager("log-manager-1.test.", ["TestLog3"], "test"); + let lm1 = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager-1.test.", + logNames: ["TestLog3"], + logFilePrefix: "test", + }); // and the second. Services.prefs.setStringPref( @@ -108,7 +130,12 @@ add_task(async function test_SharedLogs() { "log-manager-2.test.log.appender.file.level", "Debug" ); - let lm2 = new LogManager("log-manager-2.test.", ["TestLog3"], "test"); + let lm2 = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager-2.test.", + logNames: ["TestLog3"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog3"); let [capp, dapp] = getAppenders(log); @@ -158,7 +185,12 @@ function checkLogFile(prefix) { // Test that we correctly write error logs by default add_task(async function test_logFileErrorDefault() { - let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager.test.", + logNames: ["TestLog2"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog2"); log.error("an error message"); @@ -180,7 +212,12 @@ add_task(async function test_logFileSuccess() { false ); - let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager.test.", + logNames: ["TestLog2"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog2"); log.info("an info message"); @@ -230,7 +267,12 @@ add_task(async function test_logFileError() { false ); - let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager.test.", + logNames: ["TestLog2"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog2"); log.info("an info message"); @@ -307,7 +349,12 @@ add_task(async function test_logFileError() { true ); - let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + let lm = new LogManager({ + ...logManagerDefaultOptions, + prefRoot: "log-manager.test.", + logNames: ["TestLog2"], + logFilePrefix: "test", + }); let log = Log.repository.getLogger("TestLog2"); log.info("an info message"); diff --git a/toolkit/modules/tests/xpcshell/xpcshell.toml b/toolkit/modules/tests/xpcshell/xpcshell.toml index 52328fc24ed6..365646c907de 100644 --- a/toolkit/modules/tests/xpcshell/xpcshell.toml +++ b/toolkit/modules/tests/xpcshell/xpcshell.toml @@ -63,6 +63,8 @@ tags = "remote-settings" ["test_Log.js"] +["test_LogManager.js"] + ["test_Log_double_ext.js"] ["test_Log_nsIStackFrame.js"]