Bug 1772726 - Port osfile.jsm usage to IOUtils in services/sync r=markh

Differential Revision: https://phabricator.services.mozilla.com/D148444
This commit is contained in:
Barret Rennie 2022-08-03 17:05:21 +00:00
parent 3f144b8c3d
commit ec47642ebf
9 changed files with 86 additions and 76 deletions

View File

@ -42,14 +42,9 @@ ChromeUtils.defineESModuleGetters(lazy, {
PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs", PlacesUtils: "resource://gre/modules/PlacesUtils.sys.mjs",
}); });
XPCOMUtils.defineLazyModuleGetters(lazy, {
OS: "resource://gre/modules/osfile.jsm",
});
function ensureDirectory(path) { function ensureDirectory(path) {
let basename = lazy.OS.Path.dirname(path); return IOUtils.makeDirectory(PathUtils.parent(path), {
return lazy.OS.File.makeDir(basename, { createAncestors: true,
from: lazy.OS.Constants.Path.profileDir,
}); });
} }

View File

@ -39,7 +39,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
XPCOMUtils.defineLazyModuleGetters(lazy, { XPCOMUtils.defineLazyModuleGetters(lazy, {
Observers: "resource://services-common/observers.js", Observers: "resource://services-common/observers.js",
OS: "resource://gre/modules/osfile.jsm",
Resource: "resource://services-sync/resource.js", Resource: "resource://services-sync/resource.js",
}); });
@ -725,13 +724,13 @@ BookmarksStore.prototype = {
}, },
async _openMirror() { async _openMirror() {
let mirrorPath = lazy.OS.Path.join( let mirrorPath = PathUtils.join(
lazy.OS.Constants.Path.profileDir, PathUtils.profileDir,
"weave", "weave",
"bookmarks.sqlite" "bookmarks.sqlite"
); );
await lazy.OS.File.makeDir(lazy.OS.Path.dirname(mirrorPath), { await IOUtils.makeDirectory(PathUtils.parent(mirrorPath), {
from: lazy.OS.Constants.Path.profileDir, createAncestors: true,
}); });
return lazy.SyncedBookmarksMirror.open({ return lazy.SyncedBookmarksMirror.open({

View File

@ -33,7 +33,6 @@ XPCOMUtils.defineLazyModuleGetters(lazy, {
FxAccounts: "resource://gre/modules/FxAccounts.jsm", FxAccounts: "resource://gre/modules/FxAccounts.jsm",
ObjectUtils: "resource://gre/modules/ObjectUtils.jsm", ObjectUtils: "resource://gre/modules/ObjectUtils.jsm",
Observers: "resource://services-common/observers.js", Observers: "resource://services-common/observers.js",
OS: "resource://gre/modules/osfile.jsm",
Resource: "resource://services-sync/resource.js", Resource: "resource://services-sync/resource.js",
Status: "resource://services-sync/status.js", Status: "resource://services-sync/status.js",
Svc: "resource://services-sync/util.js", Svc: "resource://services-sync/util.js",
@ -198,23 +197,36 @@ class ErrorSanitizer {
"28": this.E_NO_SPACE_ON_DEVICE, // ENOSPC "28": this.E_NO_SPACE_ON_DEVICE, // ENOSPC
}; };
static DOMErrorSubstitutions = {
NotFoundError: this.E_NO_FILE_OR_DIR,
NotAllowedError: this.E_PERMISSION_DENIED,
};
static reWinError = /^(?<head>Win error (?<errno>\d+))(?<detail>.*) \(.*\r?\n?\)$/m; static reWinError = /^(?<head>Win error (?<errno>\d+))(?<detail>.*) \(.*\r?\n?\)$/m;
static reUnixError = /^(?<head>Unix error (?<errno>\d+))(?<detail>.*) \(.*\)$/; static reUnixError = /^(?<head>Unix error (?<errno>\d+))(?<detail>.*) \(.*\)$/;
static #cleanOSErrorMessage(error) { static #cleanOSErrorMessage(message, error = undefined) {
let match = this.reWinError.exec(error); if (DOMException.isInstance(error)) {
const sub = this.DOMErrorSubstitutions[error.name];
message = message.replaceAll("\\", "/");
if (sub) {
return `${sub} ${message}`;
}
}
let match = this.reWinError.exec(message);
if (match) { if (match) {
let head = let head =
this.WindowsErrorSubstitutions[match.groups.errno] || match.groups.head; this.WindowsErrorSubstitutions[match.groups.errno] || match.groups.head;
return head + match.groups.detail.replaceAll("\\", "/"); return head + match.groups.detail.replaceAll("\\", "/");
} }
match = this.reUnixError.exec(error); match = this.reUnixError.exec(message);
if (match) { if (match) {
let head = let head =
this.UnixErrorSubstitutions[match.groups.errno] || match.groups.head; this.UnixErrorSubstitutions[match.groups.errno] || match.groups.head;
return head + match.groups.detail; return head + match.groups.detail;
} }
return error; return message;
} }
// A regex we can use to replace the profile dir in error messages. We use a // A regex we can use to replace the profile dir in error messages. We use a
@ -222,30 +234,36 @@ class ErrorSanitizer {
// This escaping function is from: // This escaping function is from:
// https://developer.mozilla.org/en/docs/Web/JavaScript/Guide/Regular_Expressions // https://developer.mozilla.org/en/docs/Web/JavaScript/Guide/Regular_Expressions
static reProfileDir = new RegExp( static reProfileDir = new RegExp(
lazy.OS.Constants.Path.profileDir.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), PathUtils.profileDir.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"),
"gi" "gi"
); );
// The "public" entry-point. /**
static cleanErrorMessage(error) { * Clean an error message, removing PII and normalizing OS-specific messages.
*
* @param {string} message The error message
* @param {Error?} error The error class instance, if any.
*/
static cleanErrorMessage(message, error = undefined) {
// There's a chance the profiledir is in the error string which is PII we // There's a chance the profiledir is in the error string which is PII we
// want to avoid including in the ping. // want to avoid including in the ping.
error = error.replace(this.reProfileDir, "[profileDir]"); message = message.replace(this.reProfileDir, "[profileDir]");
// MSG_INVALID_URL from /dom/bindings/Errors.msg -- no way to access this // MSG_INVALID_URL from /dom/bindings/Errors.msg -- no way to access this
// directly from JS. // directly from JS.
if (error.endsWith("is not a valid URL.")) { if (message.endsWith("is not a valid URL.")) {
error = "<URL> is not a valid URL."; message = "<URL> is not a valid URL.";
} }
// Try to filter things that look somewhat like a URL (in that they contain a // Try to filter things that look somewhat like a URL (in that they contain a
// colon in the middle of non-whitespace), in case anything else is including // colon in the middle of non-whitespace), in case anything else is including
// these in error messages. Note that JSON.stringified stuff comes through // these in error messages. Note that JSON.stringified stuff comes through
// here, so we explicitly ignore double-quotes as well. // here, so we explicitly ignore double-quotes as well.
error = error.replace(/[^\s"]+:[^\s"]+/g, "<URL>"); message = message.replace(/[^\s"]+:[^\s"]+/g, "<URL>");
// Anywhere that's normalized the guid in errors we can easily filter // Anywhere that's normalized the guid in errors we can easily filter
// to make it easier to aggregate these types of errors // to make it easier to aggregate these types of errors
error = error.replace(/<guid: ([^>]+)>/g, "<GUID>"); message = message.replace(/<guid: ([^>]+)>/g, "<GUID>");
return this.#cleanOSErrorMessage(error);
return this.#cleanOSErrorMessage(message, error);
} }
} }
@ -1161,6 +1179,13 @@ class SyncTelemetryImpl {
return { name: "autherror", from: error.source }; return { name: "autherror", from: error.source };
} }
if (DOMException.isInstance(error)) {
return {
name: "unexpectederror",
error: ErrorSanitizer.cleanErrorMessage(error.message, error),
};
}
let httpCode = let httpCode =
error.status || (error.response && error.response.status) || error.code; error.status || (error.response && error.response.status) || error.code;

View File

@ -28,7 +28,6 @@ const { XPCOMUtils } = ChromeUtils.importESModule(
"resource://gre/modules/XPCOMUtils.sys.mjs" "resource://gre/modules/XPCOMUtils.sys.mjs"
); );
const lazy = {}; const lazy = {};
ChromeUtils.defineModuleGetter(lazy, "OS", "resource://gre/modules/osfile.jsm");
const FxAccountsCommon = ChromeUtils.import( const FxAccountsCommon = ChromeUtils.import(
"resource://gre/modules/FxAccountsCommon.js" "resource://gre/modules/FxAccountsCommon.js"
); );
@ -343,7 +342,7 @@ var Utils = {
let [fileName] = args.splice(-1); let [fileName] = args.splice(-1);
return PathUtils.join( return PathUtils.join(
Services.dirsvc.get("ProfD", Ci.nsIFile).path, PathUtils.profileDir,
"weave", "weave",
...args, ...args,
`${fileName}.json` `${fileName}.json`
@ -375,9 +374,9 @@ var Utils = {
} }
try { try {
return await CommonUtils.readJSON(path); return await IOUtils.readJSON(path);
} catch (e) { } catch (e) {
if (!(e instanceof lazy.OS.File.Error && e.becauseNoSuchFile)) { if (!DOMException.isInstance(e) || e.name !== "NotFoundError") {
if (that._log) { if (that._log) {
that._log.debug("Failed to load json", e); that._log.debug("Failed to load json", e);
} }
@ -402,16 +401,14 @@ var Utils = {
* Promise resolved when the write has been performed. * Promise resolved when the write has been performed.
*/ */
async jsonSave(filePath, that, obj) { async jsonSave(filePath, that, obj) {
let path = lazy.OS.Path.join( let path = PathUtils.join(
lazy.OS.Constants.Path.profileDir, PathUtils.profileDir,
"weave", "weave",
...(filePath + ".json").split("/") ...(filePath + ".json").split("/")
); );
let dir = lazy.OS.Path.dirname(path); let dir = PathUtils.parent(path);
await lazy.OS.File.makeDir(dir, { await IOUtils.makeDirectory(dir, { createAncestors: true });
from: lazy.OS.Constants.Path.profileDir,
});
if (that._log) { if (that._log) {
that._log.trace("Saving json to disk: " + path); that._log.trace("Saving json to disk: " + path);
@ -419,7 +416,7 @@ var Utils = {
let json = typeof obj == "function" ? obj.call(that) : obj; let json = typeof obj == "function" ? obj.call(that) : obj;
return CommonUtils.writeJSON(json, path); return IOUtils.writeJSON(path, json);
}, },
/** /**
@ -474,20 +471,20 @@ var Utils = {
* Object to use for logging * Object to use for logging
*/ */
jsonMove(aFrom, aTo, that) { jsonMove(aFrom, aTo, that) {
let pathFrom = lazy.OS.Path.join( let pathFrom = PathUtils.join(
lazy.OS.Constants.Path.profileDir, PathUtils.profileDir,
"weave", "weave",
...(aFrom + ".json").split("/") ...(aFrom + ".json").split("/")
); );
let pathTo = lazy.OS.Path.join( let pathTo = PathUtils.join(
lazy.OS.Constants.Path.profileDir, PathUtils.profileDir,
"weave", "weave",
...(aTo + ".json").split("/") ...(aTo + ".json").split("/")
); );
if (that._log) { if (that._log) {
that._log.trace("Moving " + pathFrom + " to " + pathTo); that._log.trace("Moving " + pathFrom + " to " + pathTo);
} }
return lazy.OS.File.move(pathFrom, pathTo, { noOverwrite: true }); return IOUtils.move(pathFrom, pathTo, { noOverwrite: true });
}, },
/** /**
@ -502,15 +499,15 @@ var Utils = {
* Object to use for logging * Object to use for logging
*/ */
jsonRemove(filePath, that) { jsonRemove(filePath, that) {
let path = lazy.OS.Path.join( let path = PathUtils.join(
lazy.OS.Constants.Path.profileDir, PathUtils.profileDir,
"weave", "weave",
...(filePath + ".json").split("/") ...(filePath + ".json").split("/")
); );
if (that._log) { if (that._log) {
that._log.trace("Deleting " + path); that._log.trace("Deleting " + path);
} }
return lazy.OS.File.remove(path, { ignoreAbsent: true }); return IOUtils.remove(path, { ignoreAbsent: true });
}, },
/** /**

View File

@ -7,7 +7,6 @@ const { BookmarkHTMLUtils } = ChromeUtils.importESModule(
const { BookmarkJSONUtils } = ChromeUtils.importESModule( const { BookmarkJSONUtils } = ChromeUtils.importESModule(
"resource://gre/modules/BookmarkJSONUtils.sys.mjs" "resource://gre/modules/BookmarkJSONUtils.sys.mjs"
); );
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { const {
Bookmark, Bookmark,
BookmarkFolder, BookmarkFolder,
@ -472,8 +471,8 @@ async function test_restoreOrImport(engine, { replace }) {
}); });
_(`Get Firefox!: ${bmk1.guid}`); _(`Get Firefox!: ${bmk1.guid}`);
let backupFilePath = OS.Path.join( let backupFilePath = PathUtils.join(
OS.Constants.Path.tmpDir, PathUtils.tempDir,
`t_b_e_${Date.now()}.json` `t_b_e_${Date.now()}.json`
); );

View File

@ -1,7 +1,6 @@
/* Any copyright is dedicated to the Public Domain. /* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */ http://creativecommons.org/publicdomain/zero/1.0/ */
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { PromiseUtils } = ChromeUtils.import( const { PromiseUtils } = ChromeUtils.import(
"resource://gre/modules/PromiseUtils.jsm" "resource://gre/modules/PromiseUtils.jsm"
); );
@ -125,11 +124,9 @@ add_task(async function test_invalidChangedIDs() {
let tracker = engine._tracker; let tracker = engine._tracker;
await tracker._beforeSave(); await tracker._beforeSave();
await OS.File.writeAtomic( await IOUtils.writeUTF8(tracker._storage.path, "5", {
tracker._storage.path, tmpPath: tracker._storage.path + ".tmp",
new TextEncoder().encode("5"), });
{ tmpPath: tracker._storage.path + ".tmp" }
);
ok(!tracker._storage.dataReady); ok(!tracker._storage.dataReady);
const changes = await tracker.getChangedIDs(); const changes = await tracker.getChangedIDs();

View File

@ -1,7 +1,6 @@
/* Any copyright is dedicated to the Public Domain. /* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */ http://creativecommons.org/publicdomain/zero/1.0/ */
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { Service } = ChromeUtils.import("resource://services-sync/service.js"); const { Service } = ChromeUtils.import("resource://services-sync/service.js");
async function makeSteamEngine() { async function makeSteamEngine() {
@ -116,7 +115,6 @@ add_task(async function test_lastSync() {
add_task(async function test_toFetch() { add_task(async function test_toFetch() {
_("SyncEngine.toFetch corresponds to file on disk"); _("SyncEngine.toFetch corresponds to file on disk");
await SyncTestingInfrastructure(server); await SyncTestingInfrastructure(server);
const filename = "weave/toFetch/steam.json";
await testSteamEngineStorage({ await testSteamEngineStorage({
toFetch: guidSetOfSize(3), toFetch: guidSetOfSize(3),
@ -153,9 +151,13 @@ add_task(async function test_toFetch() {
await testSteamEngineStorage({ await testSteamEngineStorage({
toFetch: guidSetOfSize(2), toFetch: guidSetOfSize(2),
async beforeCheck() { async beforeCheck() {
let toFetchPath = OS.Path.join(OS.Constants.Path.profileDir, filename); let toFetchPath = PathUtils.join(
let bytes = new TextEncoder().encode(JSON.stringify(this.toFetch)); PathUtils.profileDir,
await OS.File.writeAtomic(toFetchPath, bytes, { "weave",
"toFetch",
"steam.json"
);
await IOUtils.writeJSON(toFetchPath, this.toFetch, {
tmpPath: toFetchPath + ".tmp", tmpPath: toFetchPath + ".tmp",
}); });
}, },
@ -169,7 +171,6 @@ add_task(async function test_toFetch() {
add_task(async function test_previousFailed() { add_task(async function test_previousFailed() {
_("SyncEngine.previousFailed corresponds to file on disk"); _("SyncEngine.previousFailed corresponds to file on disk");
await SyncTestingInfrastructure(server); await SyncTestingInfrastructure(server);
const filename = "weave/failed/steam.json";
await testSteamEngineStorage({ await testSteamEngineStorage({
previousFailed: guidSetOfSize(3), previousFailed: guidSetOfSize(3),
@ -206,12 +207,13 @@ add_task(async function test_previousFailed() {
await testSteamEngineStorage({ await testSteamEngineStorage({
previousFailed: guidSetOfSize(2), previousFailed: guidSetOfSize(2),
async beforeCheck() { async beforeCheck() {
let previousFailedPath = OS.Path.join( let previousFailedPath = PathUtils.join(
OS.Constants.Path.profileDir, PathUtils.profileDir,
filename "weave",
"failed",
"steam.json"
); );
let bytes = new TextEncoder().encode(JSON.stringify(this.previousFailed)); await IOUtils.writeJSON(previousFailedPath, this.previousFailed, {
await OS.File.writeAtomic(previousFailedPath, bytes, {
tmpPath: previousFailedPath + ".tmp", tmpPath: previousFailedPath + ".tmp",
}); });
}, },

View File

@ -7,7 +7,6 @@ const { Resource } = ChromeUtils.import("resource://services-sync/resource.js");
const { RotaryEngine } = ChromeUtils.import( const { RotaryEngine } = ChromeUtils.import(
"resource://testing-common/services/sync/rotaryengine.js" "resource://testing-common/services/sync/rotaryengine.js"
); );
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { getFxAccountsSingleton } = ChromeUtils.import( const { getFxAccountsSingleton } = ChromeUtils.import(
"resource://gre/modules/FxAccounts.jsm" "resource://gre/modules/FxAccounts.jsm"
); );
@ -546,7 +545,7 @@ add_task(async function test_engine_fail_ioerror() {
equal(failureReason.name, "unexpectederror"); equal(failureReason.name, "unexpectederror");
// ensure the profile dir in the exception message has been stripped. // ensure the profile dir in the exception message has been stripped.
ok( ok(
!failureReason.error.includes(OS.Constants.Path.profileDir), !failureReason.error.includes(PathUtils.profileDir),
failureReason.error failureReason.error
); );
ok(failureReason.error.includes("[profileDir]"), failureReason.error); ok(failureReason.error.includes("[profileDir]"), failureReason.error);
@ -671,13 +670,10 @@ add_task(async function test_clean_real_os_error() {
engine.enabled = true; engine.enabled = true;
let server = await serverForFoo(engine); let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server); await SyncTestingInfrastructure(server);
let path = let path = PathUtils.join(PathUtils.profileDir, "no", "such", "path.json");
Services.appinfo.OS == "WINNT"
? "no\\such\\path.json"
: "no/such/path.json";
try { try {
await CommonUtils.writeJSON({}, path); await IOUtils.readJSON(path);
throw new Error("should fail to write the file"); throw new Error("should fail to read the file");
} catch (ex) { } catch (ex) {
engine._errToThrow = ex; engine._errToThrow = ex;
} }
@ -692,7 +688,7 @@ add_task(async function test_clean_real_os_error() {
equal(failureReason.name, "unexpectederror"); equal(failureReason.name, "unexpectederror");
equal( equal(
failureReason.error, failureReason.error,
"OS error [File/Path not found] during operation open on file no/such/path.json" "OS error [File/Path not found] Could not open the file at [profileDir]/no/such/path.json"
); );
}); });
} finally { } finally {

View File

@ -50,7 +50,6 @@ XPCOMUtils.defineLazyModuleGetters(lazy, {
JsonSchema: "resource://gre/modules/JsonSchema.jsm", JsonSchema: "resource://gre/modules/JsonSchema.jsm",
Log: "resource://gre/modules/Log.jsm", Log: "resource://gre/modules/Log.jsm",
Logger: "resource://tps/logger.jsm", Logger: "resource://tps/logger.jsm",
OS: "resource://gre/modules/osfile.jsm",
SessionStore: "resource:///modules/sessionstore/SessionStore.jsm", SessionStore: "resource:///modules/sessionstore/SessionStore.jsm",
Svc: "resource://services-sync/util.js", Svc: "resource://services-sync/util.js",
SyncTelemetry: "resource://services-sync/telemetry.js", SyncTelemetry: "resource://services-sync/telemetry.js",
@ -1009,7 +1008,8 @@ var TPS = {
_getFileRelativeToSourceRoot(testFileURL, relativePath) { _getFileRelativeToSourceRoot(testFileURL, relativePath) {
let file = lazy.fileProtocolHandler.getFileFromURLSpec(testFileURL); let file = lazy.fileProtocolHandler.getFileFromURLSpec(testFileURL);
let root = file.parent.parent.parent.parent.parent; // <root>/services/sync/tests/tps/test_foo.js // <root>/services/sync/tests/tps // <root>/services/sync/tests // <root>/services/sync // <root>/services // <root> let root = file.parent.parent.parent.parent.parent; // <root>/services/sync/tests/tps/test_foo.js // <root>/services/sync/tests/tps // <root>/services/sync/tests // <root>/services/sync // <root>/services // <root>
root.appendRelativePath(lazy.OS.Path.normalize(relativePath)); root.appendRelativePath(relativePath);
root.normalize();
return root; return root;
}, },