Skip to content

Commit

Permalink
Enforce logger module via lint rules
Browse files Browse the repository at this point in the history
This adds lint rules (and fixes various errors) to ensure we use the `logger`
intermediary module, rather than accessing the console directly.
  • Loading branch information
jryans committed Oct 1, 2020
1 parent fa9921e commit 09bd91a
Show file tree
Hide file tree
Showing 16 changed files with 33 additions and 22 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.js
Expand Up @@ -28,6 +28,8 @@ module.exports = {
"indent": "off",
"no-constant-condition": "off",
"no-async-promise-executor": "off",
// We use a `logger` intermediary module
"no-console": "error",
},
overrides: [{
"files": ["src/**/*.ts"],
Expand Down
2 changes: 1 addition & 1 deletion spec/olm-loader.js
Expand Up @@ -31,5 +31,5 @@ try {
const crypto = require('crypto');
utils.setCrypto(crypto);
} catch (err) {
console.log('nodejs was compiled without crypto support: some tests will fail');
logger.log('nodejs was compiled without crypto support: some tests will fail');
}
3 changes: 2 additions & 1 deletion spec/unit/crypto/CrossSigningInfo.spec.js
Expand Up @@ -26,6 +26,7 @@ import {MemoryCryptoStore} from '../../../src/crypto/store/memory-crypto-store';
import 'fake-indexeddb/auto';
import 'jest-localstorage-mock';
import {OlmDevice} from "../../../src/crypto/OlmDevice";
import {logger} from '../../../src/logger';

const userId = "@alice:example.com";

Expand All @@ -51,7 +52,7 @@ const masterKeyPub = "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk";

describe("CrossSigningInfo.getCrossSigningKey", function() {
if (!global.Olm) {
console.warn('Not running megolm backup unit tests: libolm not present');
logger.warn('Not running megolm backup unit tests: libolm not present');
return;
}

Expand Down
3 changes: 2 additions & 1 deletion spec/unit/crypto/cross-signing.spec.js
Expand Up @@ -22,6 +22,7 @@ import {TestClient} from '../../TestClient';
import {HttpResponse, setHttpResponses} from '../../test-utils';
import { resetCrossSigningKeys } from "./crypto-utils";
import { MatrixError } from '../../../src/http-api';
import {logger} from '../../../src/logger';

async function makeTestClient(userInfo, options, keys) {
if (!keys) keys = {};
Expand Down Expand Up @@ -49,7 +50,7 @@ async function makeTestClient(userInfo, options, keys) {

describe("Cross Signing", function() {
if (!global.Olm) {
console.warn('Not running megolm backup unit tests: libolm not present');
logger.warn('Not running megolm backup unit tests: libolm not present');
return;
}

Expand Down
5 changes: 3 additions & 2 deletions spec/unit/crypto/secrets.spec.js
Expand Up @@ -22,14 +22,15 @@ import {TestClient} from '../../TestClient';
import {makeTestClients} from './verification/util';
import {encryptAES} from "../../../src/crypto/aes";
import {resetCrossSigningKeys, createSecretStorageKey} from "./crypto-utils";
import {logger} from '../../../src/logger';

import * as utils from "../../../src/utils";

try {
const crypto = require('crypto');
utils.setCrypto(crypto);
} catch (err) {
console.log('nodejs was compiled without crypto support');
logger.log('nodejs was compiled without crypto support');
}

async function makeTestClient(userInfo, options) {
Expand Down Expand Up @@ -60,7 +61,7 @@ function sign(obj, key, userId) {

describe("Secrets", function() {
if (!global.Olm) {
console.warn('Not running megolm backup unit tests: libolm not present');
logger.warn('Not running megolm backup unit tests: libolm not present');
return;
}

Expand Down
5 changes: 3 additions & 2 deletions spec/unit/crypto/verification/util.js
Expand Up @@ -18,12 +18,13 @@ limitations under the License.
import {TestClient} from '../../../TestClient';
import {MatrixEvent} from "../../../../src/models/event";
import nodeCrypto from "crypto";
import {logger} from '../../../../src/logger';

export async function makeTestClients(userInfos, options) {
const clients = [];
const clientMap = {};
const sendToDevice = function(type, map) {
// console.log(this.getUserId(), "sends", type, map);
// logger.log(this.getUserId(), "sends", type, map);
for (const [userId, devMap] of Object.entries(map)) {
if (userId in clientMap) {
for (const [deviceId, msg] of Object.entries(devMap)) {
Expand Down Expand Up @@ -67,7 +68,7 @@ export async function makeTestClients(userInfos, options) {
setImmediate(() => {
for (const tc of clients) {
if (tc.client === this) { // eslint-disable-line babel/no-invalid-this
console.log("sending remote echo!!");
logger.log("sending remote echo!!");
tc.client.emit("Room.timeline", remoteEcho);
} else {
tc.client.emit("Room.timeline", event);
Expand Down
2 changes: 1 addition & 1 deletion src/client.js
Expand Up @@ -2054,7 +2054,7 @@ MatrixClient.prototype._restoreKeyBackup = function(
// This is async.
this._crypto.storeSessionBackupPrivateKey(privKey)
.catch((e) => {
console.warn("Error caching session backup key:", e);
logger.warn("Error caching session backup key:", e);
}).then(cacheCompleteCallback);

if (progressCallback) {
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/CrossSigning.js
Expand Up @@ -734,7 +734,7 @@ export async function requestKeysDuringVerification(baseApis, userId, deviceId)
if (baseApis.getUserId() !== userId) {
return;
}
console.log("Cross-signing: Self-verification done; requesting keys");
logger.log("Cross-signing: Self-verification done; requesting keys");
// This happens asynchronously, and we're not concerned about waiting for
// it. We return here in order to test.
return new Promise((resolve, reject) => {
Expand All @@ -748,7 +748,7 @@ export async function requestKeysDuringVerification(baseApis, userId, deviceId)
const crossSigning = new CrossSigningInfo(
original.userId,
{ getCrossSigningKey: async (type) => {
console.debug("Cross-signing: requesting secret",
logger.debug("Cross-signing: requesting secret",
type, deviceId);
const { promise } = client.requestSecret(
`m.cross_signing.${type}`, [deviceId],
Expand Down Expand Up @@ -811,6 +811,6 @@ export async function requestKeysDuringVerification(baseApis, userId, deviceId)
timeout,
]).then(resolve, reject);
}).catch((e) => {
console.warn("Cross-signing: failure while requesting keys:", e);
logger.warn("Cross-signing: failure while requesting keys:", e);
});
}
2 changes: 1 addition & 1 deletion src/crypto/OlmDevice.js
Expand Up @@ -144,7 +144,7 @@ OlmDevice.prototype.init = async function(opts = {}) {
try {
if (fromExportedDevice) {
if (pickleKey) {
console.warn(
logger.warn(
'ignoring opts.pickleKey'
+ ' because opts.fromExportedDevice is present.',
);
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/SecretStorage.js
Expand Up @@ -247,7 +247,7 @@ export class SecretStorage extends EventEmitter {
) {
const hasKey = await this.hasKey(keys[0]);
if (hasKey) {
console.log("Fixing up passthrough secret: " + name);
logger.log("Fixing up passthrough secret: " + name);
await this.storePassthrough(name, keys[0]);
const newData = await this._baseApis.getAccountDataFromServer(name);
return newData;
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/index.js
Expand Up @@ -146,7 +146,7 @@ export function Crypto(baseApis, sessionStore, userId, deviceId,
method,
);
} else {
console.warn(`Excluding unknown verification method ${method}`);
logger.warn(`Excluding unknown verification method ${method}`);
}
}
} else {
Expand Down
5 changes: 3 additions & 2 deletions src/crypto/verification/QRCode.js
Expand Up @@ -26,6 +26,7 @@ import {
newUserCancelledError,
} from './Error';
import {encodeUnpaddedBase64, decodeBase64} from "../olmlib";
import {logger} from '../../logger';

export const SHOW_QR_CODE_METHOD = "m.qr_code.show.v1";
export const SCAN_QR_CODE_METHOD = "m.qr_code.scan.v1";
Expand Down Expand Up @@ -94,15 +95,15 @@ export class ReciprocateQRCode extends Base {
if (!targetKey) throw newKeyMismatchError();

if (keyInfo !== targetKey) {
console.error("key ID from key info does not match");
logger.error("key ID from key info does not match");
throw newKeyMismatchError();
}
for (const deviceKeyId in device.keys) {
if (!deviceKeyId.startsWith("ed25519")) continue;
const deviceTargetKey = keys[deviceKeyId];
if (!deviceTargetKey) throw newKeyMismatchError();
if (device.keys[deviceKeyId] !== deviceTargetKey) {
console.error("master key does not match");
logger.error("master key does not match");
throw newKeyMismatchError();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/http-api.js
Expand Up @@ -961,7 +961,7 @@ export async function retryNetworkOperation(maxAttempts, callback) {
try {
if (attempts > 0) {
const timeout = 1000 * Math.pow(2, attempts);
console.log(`network operation failed ${attempts} times,` +
logger.log(`network operation failed ${attempts} times,` +
` retrying in ${timeout}ms...`);
await new Promise(r => setTimeout(r, timeout));
}
Expand Down
2 changes: 2 additions & 0 deletions src/logger.js
Expand Up @@ -40,11 +40,13 @@ log.methodFactory = function(methodName, logLevel, loggerName) {
methodName === "warn" ||
methodName === "trace" ||
methodName === "info";
/* eslint-disable no-console */
if (supportedByConsole) {
return console[methodName](...args);
} else {
return console.log(...args);
}
/* eslint-enable no-console */
};
};

Expand Down
9 changes: 5 additions & 4 deletions src/models/relations.js
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import {EventEmitter} from 'events';
import {EventStatus} from '../models/event';
import {logger} from '../logger';

/**
* A container for relation events that supports easy access to common ways of
Expand Down Expand Up @@ -60,15 +61,15 @@ export class Relations extends EventEmitter {

const relation = event.getRelation();
if (!relation) {
console.error("Event must have relation info");
logger.error("Event must have relation info");
return;
}

const relationType = relation.rel_type;
const eventType = event.getType();

if (this.relationType !== relationType || this.eventType !== eventType) {
console.error("Event relation info doesn't match this container");
logger.error("Event relation info doesn't match this container");
return;
}

Expand Down Expand Up @@ -104,15 +105,15 @@ export class Relations extends EventEmitter {

const relation = event.getRelation();
if (!relation) {
console.error("Event must have relation info");
logger.error("Event must have relation info");
return;
}

const relationType = relation.rel_type;
const eventType = event.getType();

if (this.relationType !== relationType || this.eventType !== eventType) {
console.error("Event relation info doesn't match this container");
logger.error("Event relation info doesn't match this container");
return;
}

Expand Down
3 changes: 2 additions & 1 deletion src/pushprocessor.js
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
*/

import {escapeRegExp, globToRegexp, isNullOrUndefined} from "./utils";
import {logger} from './logger';

/**
* @module pushprocessor
Expand Down Expand Up @@ -444,7 +445,7 @@ PushProcessor.rewriteDefaultRules = function(incomingRules) {
} else {
// Add the rule
const ruleId = override.rule_id;
console.warn(`Adding default global override for ${ruleId}`);
logger.warn(`Adding default global override for ${ruleId}`);
globalOverrides.push(override);
}
}
Expand Down

0 comments on commit 09bd91a

Please sign in to comment.