From 03fad2639d3f435ea7464c8bfb7e3e1c9ef7b5fb Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 15:41:39 +0200 Subject: [PATCH 01/16] test/utils/MegolmExportEncryption-test.js -> test/utils/MegolmExportEncryption-test.ts Signed-off-by: Kerry Archibald --- ...olmExportEncryption-test.js => MegolmExportEncryption-test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/utils/{MegolmExportEncryption-test.js => MegolmExportEncryption-test.ts} (100%) diff --git a/test/utils/MegolmExportEncryption-test.js b/test/utils/MegolmExportEncryption-test.ts similarity index 100% rename from test/utils/MegolmExportEncryption-test.js rename to test/utils/MegolmExportEncryption-test.ts From a9745a3a768797757213198d0caabc73e0d4e81c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 15:42:51 +0200 Subject: [PATCH 02/16] test/utils/ShieldUtils-test.js - test/utils/ShieldUtils-test.ts Signed-off-by: Kerry Archibald --- test/utils/{ShieldUtils-test.js => ShieldUtils-test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/utils/{ShieldUtils-test.js => ShieldUtils-test.ts} (100%) diff --git a/test/utils/ShieldUtils-test.js b/test/utils/ShieldUtils-test.ts similarity index 100% rename from test/utils/ShieldUtils-test.js rename to test/utils/ShieldUtils-test.ts From 69bd04d35041c9261a14c8c81f15218a5d9cb17f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 15:55:58 +0200 Subject: [PATCH 03/16] type fixes for ShieldUtils-test Signed-off-by: Kerry Archibald --- test/utils/ShieldUtils-test.ts | 54 ++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/test/utils/ShieldUtils-test.ts b/test/utils/ShieldUtils-test.ts index 85f9de31508..1ad184526c5 100644 --- a/test/utils/ShieldUtils-test.ts +++ b/test/utils/ShieldUtils-test.ts @@ -1,7 +1,27 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { + MatrixClient, + Room } from 'matrix-js-sdk/src/matrix'; + import { shieldStatusForRoom } from '../../src/utils/ShieldUtils'; import DMRoomMap from '../../src/utils/DMRoomMap'; -function mkClient(selfTrust) { +function mkClient(selfTrust = false) { return { getUserId: () => "@self:localhost", checkUserTrust: (userId) => ({ @@ -12,7 +32,7 @@ function mkClient(selfTrust) { isVerified: () => userId === "@self:localhost" ? selfTrust : userId[2] == "T", }), getStoredDevicesForUser: (userId) => ["DEVICE"], - }; + } as unknown as MatrixClient; } describe("mkClient self-test", function() { @@ -42,9 +62,10 @@ describe("mkClient self-test", function() { describe("shieldStatusForMembership self-trust behaviour", function() { beforeAll(() => { - DMRoomMap.sharedInstance = { + const mockInstance = { getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null, - }; + } as unknown as DMRoomMap; + jest.spyOn(DMRoomMap, 'shared').mockReturnValue(mockInstance); }); it.each( @@ -55,7 +76,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF1:h", "@FF2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual("normal"); }); @@ -68,7 +89,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@TT2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -81,7 +102,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@FF2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -94,7 +115,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -107,7 +128,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -120,7 +141,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -128,9 +149,10 @@ describe("shieldStatusForMembership self-trust behaviour", function() { describe("shieldStatusForMembership other-trust behaviour", function() { beforeAll(() => { - DMRoomMap.sharedInstance = { + const mockInstance = { getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null, - }; + } as unknown as DMRoomMap; + jest.spyOn(DMRoomMap, 'shared').mockReturnValue(mockInstance); }); it.each( @@ -140,7 +162,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -152,7 +174,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h", "@TT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -164,7 +186,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h", "@FT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -176,7 +198,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@WF:h", "@FT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); From 1670025bd2af9a355c2761998202f602d61f242e Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 15:56:20 +0200 Subject: [PATCH 04/16] test/DecryptionFailureTracker-test.js -> test/DecryptionFailureTracker-test.ts Signed-off-by: Kerry Archibald --- ...ionFailureTracker-test.js => DecryptionFailureTracker-test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{DecryptionFailureTracker-test.js => DecryptionFailureTracker-test.ts} (100%) diff --git a/test/DecryptionFailureTracker-test.js b/test/DecryptionFailureTracker-test.ts similarity index 100% rename from test/DecryptionFailureTracker-test.js rename to test/DecryptionFailureTracker-test.ts From 7bd93d075c4d8d45befcbfd59c889782c9a44b48 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 16:06:35 +0200 Subject: [PATCH 05/16] remove unsupported assertion failure messages Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 39 +++++++++++++++------------ 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index b0494f97aa6..6033a14ba56 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -19,10 +19,8 @@ import { MatrixEvent } from 'matrix-js-sdk/src/matrix'; import { DecryptionFailureTracker } from '../src/DecryptionFailureTracker'; class MockDecryptionError extends Error { - constructor(code) { + constructor(public readonly errcode = 'MOCK_DECRYPTION_ERROR') { super(); - - this.errcode = code || 'MOCK_DECRYPTION_ERROR'; } } @@ -52,7 +50,8 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - expect(count).not.toBe(0, 'should track a failure for an event that failed decryption'); + // should track a failure for an event that failed decryption + expect(count).not.toBe(0); done(); }); @@ -74,7 +73,8 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - expect(count).not.toBe(0, 'should track a failure for an event that failed decryption'); + // should track a failure for an event that failed decryption + expect(count).not.toBe(0); done(); }); @@ -94,7 +94,8 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - expect(count).toBe(0, 'should not track a failure for an event that never became visible'); + // should not track a failure for an event that never became visible + expect(count).toBe(0); done(); }); @@ -102,7 +103,8 @@ describe('DecryptionFailureTracker', function() { it('does not track a failed decryption where the event is subsequently successfully decrypted', (done) => { const decryptedEvent = createFailedDecryptionEvent(); const tracker = new DecryptionFailureTracker((total) => { - expect(true).toBe(false, 'should not track an event that has since been decrypted correctly'); + // should not track an event that has since been decrypted correctly + expect(true).toBe(false); }, () => "UnknownError"); tracker.addVisibleEvent(decryptedEvent); @@ -176,7 +178,8 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); tracker.trackFailures(); - expect(count).toBe(2, count + ' failures tracked, should only track a single failure per event'); + // should only track a single failure per event + expect(count).toBe(2); done(); }); @@ -203,7 +206,8 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - expect(count).toBe(1, 'should only track a single failure per event'); + // should only track a single failure per event + expect(count).toBe(1); done(); }); @@ -240,7 +244,8 @@ describe('DecryptionFailureTracker', function() { secondTracker.checkFailures(Infinity); secondTracker.trackFailures(); - expect(count).toBe(1, count + ' failures tracked, should only track a single failure per event'); + // should only track a single failure per event + expect(count).toBe(1); done(); }); @@ -275,15 +280,13 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); //expect(counts['UnknownError']).toBe(1, 'should track one UnknownError'); - expect(counts['OlmKeysNotSentError']).toBe(2, 'should track two OlmKeysNotSentError'); + // should track two OlmKeysNotSentError + expect(counts['OlmKeysNotSentError']).toBe(2); }); it('should aggregate error codes correctly', () => { const counts = {}; - const tracker = new DecryptionFailureTracker( - (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, - (errorCode) => 'OlmUnspecifiedError', - ); + const tracker = DecryptionFailureTracker.instance; const decryptedEvent1 = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); @@ -306,8 +309,9 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); + // should track three OlmUnspecifiedError expect(counts['OlmUnspecifiedError']) - .toBe(3, 'should track three OlmUnspecifiedError, got ' + counts['OlmUnspecifiedError']); + .toBe(3); }); it('should remap error codes correctly', () => { @@ -330,7 +334,8 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); + // should track remapped error code expect(counts['1_EDOC_RORRE']) - .toBe(1, 'should track remapped error code'); + .toBe(1); }); }); From 1ae748cc51088d60722320dbefae04a62310e2e1 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 16:34:40 +0200 Subject: [PATCH 06/16] fix ts issues in DecryptionFailureTracker Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 178 ++++++++++++-------------- 1 file changed, 84 insertions(+), 94 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 6033a14ba56..8e92c6fd042 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -16,10 +16,14 @@ limitations under the License. import { MatrixEvent } from 'matrix-js-sdk/src/matrix'; +import Analytics from '../src/Analytics'; import { DecryptionFailureTracker } from '../src/DecryptionFailureTracker'; class MockDecryptionError extends Error { - constructor(public readonly errcode = 'MOCK_DECRYPTION_ERROR') { + constructor( + public readonly errcode, + public readonly data = {}, + ) { super(); } } @@ -28,20 +32,26 @@ function createFailedDecryptionEvent() { const event = new MatrixEvent({ event_id: "event-id-" + Math.random().toString(16).slice(2), }); + // @ts-ignore private properties event.setClearData(event.badEncryptedMessage(":(")); return event; } describe('DecryptionFailureTracker', function() { - it('tracks a failed decryption for a visible event', function(done) { + const trackEventSpy = jest.spyOn(Analytics, 'trackEvent'); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('tracks a failed decryption for a visible event', function() { const failedDecryptionEvent = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; tracker.addVisibleEvent(failedDecryptionEvent); - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(failedDecryptionEvent, err); // Pretend "now" is Infinity @@ -51,18 +61,15 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should track a failure for an event that failed decryption - expect(count).not.toBe(0); - - done(); + expect(trackEventSpy).toHaveBeenCalledTimes(1); }); it('tracks a failed decryption for an event that becomes visible later', function(done) { const failedDecryptionEvent = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(failedDecryptionEvent, err); tracker.addVisibleEvent(failedDecryptionEvent); @@ -74,18 +81,17 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should track a failure for an event that failed decryption - expect(count).not.toBe(0); + expect(trackEventSpy).toHaveBeenCalledTimes(1); done(); }); - it('does not track a failed decryption for an event that never becomes visible', function(done) { + it('does not track a failed decryption for an event that never becomes visible', function() { const failedDecryptionEvent = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(failedDecryptionEvent, err); // Pretend "now" is Infinity @@ -95,24 +101,20 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should not track a failure for an event that never became visible - expect(count).toBe(0); - - done(); + expect(trackEventSpy).not.toHaveBeenCalled(); }); - it('does not track a failed decryption where the event is subsequently successfully decrypted', (done) => { + it('does not track a failed decryption where the event is subsequently successfully decrypted', () => { const decryptedEvent = createFailedDecryptionEvent(); - const tracker = new DecryptionFailureTracker((total) => { - // should not track an event that has since been decrypted correctly - expect(true).toBe(false); - }, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; tracker.addVisibleEvent(decryptedEvent); - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(decryptedEvent, err); // Indicate successful decryption: clear data can be anything where the msgtype is not m.bad.encrypted + // @ts-ignore private prop decryptedEvent.setClearData({}); tracker.eventDecrypted(decryptedEvent, null); @@ -121,20 +123,21 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - done(); + + // no tracking occurred + expect(trackEventSpy).not.toHaveBeenCalled(); }); it('does not track a failed decryption where the event is subsequently successfully decrypted ' + - 'and later becomes visible', (done) => { + 'and later becomes visible', () => { const decryptedEvent = createFailedDecryptionEvent(); - const tracker = new DecryptionFailureTracker((total) => { - expect(true).toBe(false, 'should not track an event that has since been decrypted correctly'); - }, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(decryptedEvent, err); // Indicate successful decryption: clear data can be anything where the msgtype is not m.bad.encrypted + // @ts-ignore private property decryptedEvent.setClearData({}); tracker.eventDecrypted(decryptedEvent, null); @@ -145,20 +148,19 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - done(); + expect(trackEventSpy).not.toHaveBeenCalled(); }); - it('only tracks a single failure per event, despite multiple failed decryptions for multiple events', (done) => { + it('only tracks a single failure per error code, despite multiple failed decryptions for multiple events', () => { const decryptedEvent = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; tracker.addVisibleEvent(decryptedEvent); // Arbitrary number of failed decryptions for both events - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(decryptedEvent, err); tracker.eventDecrypted(decryptedEvent, err); tracker.eventDecrypted(decryptedEvent, err); @@ -178,22 +180,18 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); tracker.trackFailures(); - // should only track a single failure per event - expect(count).toBe(2); - - done(); + expect(trackEventSpy).toHaveBeenCalledTimes(1); }); it('should not track a failure for an event that was tracked previously', (done) => { const decryptedEvent = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + const tracker = DecryptionFailureTracker.instance; tracker.addVisibleEvent(decryptedEvent); // Indicate decryption - const err = new MockDecryptionError(); + const err = new MockDecryptionError(''); tracker.eventDecrypted(decryptedEvent, err); // Pretend "now" is Infinity @@ -207,62 +205,60 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should only track a single failure per event - expect(count).toBe(1); + expect(trackEventSpy).toHaveBeenCalledTimes(1); done(); }); - xit('should not track a failure for an event that was tracked in a previous session', (done) => { - // This test uses localStorage, clear it beforehand - localStorage.clear(); + // this functionality is commented out in DecryptionFailureTracker + // commenting as well as xit here to avoid fixing ts issues for dead code + // xit('should not track a failure for an event that was tracked in a previous session', (done) => { + // // This test uses localStorage, clear it beforehand + // localStorage.clear(); - const decryptedEvent = createFailedDecryptionEvent(); + // const decryptedEvent = createFailedDecryptionEvent(); - let count = 0; - const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + // let count = 0; + // const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - tracker.addVisibleEvent(decryptedEvent); + // tracker.addVisibleEvent(decryptedEvent); - // Indicate decryption - const err = new MockDecryptionError(); - tracker.eventDecrypted(decryptedEvent, err); + // // Indicate decryption + // const err = new MockDecryptionError(''); + // tracker.eventDecrypted(decryptedEvent, err); - // Pretend "now" is Infinity - // NB: This saves to localStorage specific to DFT - tracker.checkFailures(Infinity); + // // Pretend "now" is Infinity + // // NB: This saves to localStorage specific to DFT + // tracker.checkFailures(Infinity); - tracker.trackFailures(); + // tracker.trackFailures(); - // Simulate the browser refreshing by destroying tracker and creating a new tracker - const secondTracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + // // Simulate the browser refreshing by destroying tracker and creating a new tracker + // const secondTracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - secondTracker.addVisibleEvent(decryptedEvent); + // secondTracker.addVisibleEvent(decryptedEvent); - //secondTracker.loadTrackedEvents(); + // //secondTracker.loadTrackedEvents(); - secondTracker.eventDecrypted(decryptedEvent, err); - secondTracker.checkFailures(Infinity); - secondTracker.trackFailures(); + // secondTracker.eventDecrypted(decryptedEvent, err); + // secondTracker.checkFailures(Infinity); + // secondTracker.trackFailures(); - // should only track a single failure per event - expect(count).toBe(1); + // // should only track a single failure per event + // expect(count).toBe(1); - done(); - }); + // done(); + // }); it('should count different error codes separately for multiple failures with different error codes', () => { - const counts = {}; - const tracker = new DecryptionFailureTracker( - (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, - (error) => error === "UnknownError" ? "UnknownError" : "OlmKeysNotSentError", - ); + const tracker = DecryptionFailureTracker.instance; const decryptedEvent1 = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); const decryptedEvent3 = createFailedDecryptionEvent(); const error1 = new MockDecryptionError('UnknownError'); - const error2 = new MockDecryptionError('OlmKeysNotSentError'); + const error2 = new MockDecryptionError('MEGOLM_UNKNOWN_INBOUND_SESSION_ID'); tracker.addVisibleEvent(decryptedEvent1); tracker.addVisibleEvent(decryptedEvent2); @@ -279,22 +275,22 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - //expect(counts['UnknownError']).toBe(1, 'should track one UnknownError'); - // should track two OlmKeysNotSentError - expect(counts['OlmKeysNotSentError']).toBe(2); + // called once for each error type + expect(trackEventSpy).toHaveBeenCalledTimes(2); + expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'UnknownError', '1'); + expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'OlmKeysNotSentError', '2'); }); it('should aggregate error codes correctly', () => { - const counts = {}; const tracker = DecryptionFailureTracker.instance; const decryptedEvent1 = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); const decryptedEvent3 = createFailedDecryptionEvent(); - const error1 = new MockDecryptionError('ERROR_CODE_1'); - const error2 = new MockDecryptionError('ERROR_CODE_2'); - const error3 = new MockDecryptionError('ERROR_CODE_3'); + const error1 = new MockDecryptionError(undefined); + const error2 = new MockDecryptionError(undefined); + const error3 = new MockDecryptionError(undefined); tracker.addVisibleEvent(decryptedEvent1); tracker.addVisibleEvent(decryptedEvent2); @@ -309,21 +305,16 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // should track three OlmUnspecifiedError - expect(counts['OlmUnspecifiedError']) - .toBe(3); + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'UnknownError', '3'); }); it('should remap error codes correctly', () => { - const counts = {}; - const tracker = new DecryptionFailureTracker( - (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, - (errorCode) => Array.from(errorCode).reverse().join(''), - ); + const tracker = DecryptionFailureTracker.instance; const decryptedEvent = createFailedDecryptionEvent(); - const error = new MockDecryptionError('ERROR_CODE_1'); + const error = new MockDecryptionError('MEGOLM_UNKNOWN_INBOUND_SESSION_ID'); tracker.addVisibleEvent(decryptedEvent); @@ -334,8 +325,7 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // should track remapped error code - expect(counts['1_EDOC_RORRE']) - .toBe(1); + // MEGOLM_UNKNOWN_INBOUND_SESSION_ID maps to OlmKeysNotSentError + expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'OlmKeysNotSentError', '1'); }); }); From 113d69c089dd57ece5a1ba621c643e8980512618 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 14 Apr 2022 16:40:14 +0200 Subject: [PATCH 07/16] add mock restores Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 4 ++++ test/utils/ShieldUtils-test.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 8e92c6fd042..50cba135d14 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -44,6 +44,10 @@ describe('DecryptionFailureTracker', function() { jest.clearAllMocks(); }); + afterAll(() => { + jest.spyOn(Analytics, 'trackEvent').mockRestore(); + }); + it('tracks a failed decryption for a visible event', function() { const failedDecryptionEvent = createFailedDecryptionEvent(); diff --git a/test/utils/ShieldUtils-test.ts b/test/utils/ShieldUtils-test.ts index 1ad184526c5..50372783e56 100644 --- a/test/utils/ShieldUtils-test.ts +++ b/test/utils/ShieldUtils-test.ts @@ -68,6 +68,10 @@ describe("shieldStatusForMembership self-trust behaviour", function() { jest.spyOn(DMRoomMap, 'shared').mockReturnValue(mockInstance); }); + afterAll(() => { + jest.spyOn(DMRoomMap, 'shared').mockRestore(); + }); + it.each( [[true, true], [true, false], [false, true], [false, false]], From 753a9982e49a97110a4996fcd9aa96ba2f06aafc Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 19 Apr 2022 09:16:06 +0200 Subject: [PATCH 08/16] newline Signed-off-by: Kerry Archibald --- test/utils/ShieldUtils-test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/utils/ShieldUtils-test.ts b/test/utils/ShieldUtils-test.ts index 50372783e56..10ccb80f966 100644 --- a/test/utils/ShieldUtils-test.ts +++ b/test/utils/ShieldUtils-test.ts @@ -16,7 +16,8 @@ limitations under the License. import { MatrixClient, - Room } from 'matrix-js-sdk/src/matrix'; + Room, +} from 'matrix-js-sdk/src/matrix'; import { shieldStatusForRoom } from '../../src/utils/ShieldUtils'; import DMRoomMap from '../../src/utils/DMRoomMap'; From 6c47747e92ffcf298ea484e1cde26149b4d12b12 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 20 Apr 2022 11:21:45 +0200 Subject: [PATCH 09/16] remove commented decriptionfailuretracker code and test Signed-off-by: Kerry Archibald --- src/DecryptionFailureTracker.ts | 8 ------ test/DecryptionFailureTracker-test.ts | 40 --------------------------- 2 files changed, 48 deletions(-) diff --git a/src/DecryptionFailureTracker.ts b/src/DecryptionFailureTracker.ts index 2bb522e7fe9..e950ff4a46f 100644 --- a/src/DecryptionFailureTracker.ts +++ b/src/DecryptionFailureTracker.ts @@ -122,14 +122,6 @@ export class DecryptionFailureTracker { return DecryptionFailureTracker.internalInstance; } - // loadTrackedEvents() { - // this.trackedEvents = new Set(JSON.parse(localStorage.getItem('mx-decryption-failure-event-ids')) || []); - // } - - // saveTrackedEvents() { - // localStorage.setItem('mx-decryption-failure-event-ids', JSON.stringify([...this.trackedEvents])); - // } - public eventDecrypted(e: MatrixEvent, err: MatrixError): void { if (err) { this.addDecryptionFailure(new DecryptionFailure(e.getId(), err.errcode)); diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 50cba135d14..7d82ef670fc 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -214,46 +214,6 @@ describe('DecryptionFailureTracker', function() { done(); }); - // this functionality is commented out in DecryptionFailureTracker - // commenting as well as xit here to avoid fixing ts issues for dead code - // xit('should not track a failure for an event that was tracked in a previous session', (done) => { - // // This test uses localStorage, clear it beforehand - // localStorage.clear(); - - // const decryptedEvent = createFailedDecryptionEvent(); - - // let count = 0; - // const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - - // tracker.addVisibleEvent(decryptedEvent); - - // // Indicate decryption - // const err = new MockDecryptionError(''); - // tracker.eventDecrypted(decryptedEvent, err); - - // // Pretend "now" is Infinity - // // NB: This saves to localStorage specific to DFT - // tracker.checkFailures(Infinity); - - // tracker.trackFailures(); - - // // Simulate the browser refreshing by destroying tracker and creating a new tracker - // const secondTracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - - // secondTracker.addVisibleEvent(decryptedEvent); - - // //secondTracker.loadTrackedEvents(); - - // secondTracker.eventDecrypted(decryptedEvent, err); - // secondTracker.checkFailures(Infinity); - // secondTracker.trackFailures(); - - // // should only track a single failure per event - // expect(count).toBe(1); - - // done(); - // }); - it('should count different error codes separately for multiple failures with different error codes', () => { const tracker = DecryptionFailureTracker.instance; From 641867f1f42dd7ea69a5ff4ffe668e1fce4d4904 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 20 Apr 2022 11:29:19 +0200 Subject: [PATCH 10/16] make should aggregate error codes correctly pass Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 7d82ef670fc..4ea2e56af2a 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -21,8 +21,8 @@ import { DecryptionFailureTracker } from '../src/DecryptionFailureTracker'; class MockDecryptionError extends Error { constructor( - public readonly errcode, - public readonly data = {}, + public readonly errcode: string, + public readonly data: Record = {}, ) { super(); } @@ -252,10 +252,10 @@ describe('DecryptionFailureTracker', function() { const decryptedEvent2 = createFailedDecryptionEvent(); const decryptedEvent3 = createFailedDecryptionEvent(); - const error1 = new MockDecryptionError(undefined); - const error2 = new MockDecryptionError(undefined); - const error3 = new MockDecryptionError(undefined); - + const error1 = new MockDecryptionError('ERROR_CODE_1'); + const error2 = new MockDecryptionError('ERROR_CODE_2'); + const error3 = new MockDecryptionError('ERROR_CODE_3'); + tracker.addVisibleEvent(decryptedEvent1); tracker.addVisibleEvent(decryptedEvent2); tracker.addVisibleEvent(decryptedEvent3); @@ -269,8 +269,12 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - expect(trackEventSpy).toHaveBeenCalledTimes(1); - expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'UnknownError', '3'); + expect(trackEventSpy).toHaveBeenCalledTimes(3); + expect(trackEventSpy.mock.calls).toEqual( + [['E2E', 'Decryption failure', 'UnknownError', '1'], + ['E2E', 'Decryption failure', 'UnknownError', '1'], + ['E2E', 'Decryption failure', 'UnknownError', '1']] + ); }); it('should remap error codes correctly', () => { From 876c37e892bd9feec1e703084897a169d6529f9b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 20 Apr 2022 11:49:10 +0200 Subject: [PATCH 11/16] cheaters types in MegolmExportEncryption Signed-off-by: Kerry Archibald --- test/utils/MegolmExportEncryption-test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/utils/MegolmExportEncryption-test.ts b/test/utils/MegolmExportEncryption-test.ts index 8b16085efcc..4fb92a8f8bb 100644 --- a/test/utils/MegolmExportEncryption-test.ts +++ b/test/utils/MegolmExportEncryption-test.ts @@ -20,7 +20,8 @@ import { Crypto } from "@peculiar/webcrypto"; const webCrypto = new Crypto(); -function getRandomValues(buf) { +function getRandomValues(buf: T): T { + // @ts-ignore fussy generics return nodeCrypto.randomFillSync(buf); } @@ -64,7 +65,7 @@ const TEST_VECTORS=[ ], ]; -function stringToArray(s) { +function stringToArray(s: string): ArrayBufferLike { return new TextEncoder().encode(s).buffer; } @@ -72,7 +73,10 @@ describe('MegolmExportEncryption', function() { let MegolmExportEncryption; beforeAll(() => { - window.crypto = { subtle: webCrypto.subtle, getRandomValues }; + window.crypto = { + subtle: webCrypto.subtle, + getRandomValues, + }; MegolmExportEncryption = require("../../src/utils/MegolmExportEncryption"); }); From 67c7cd63cd98b9bf21ed1faa7d887b55c91688b1 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 20 Apr 2022 11:49:15 +0200 Subject: [PATCH 12/16] lint Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 4ea2e56af2a..08fd5f9e38e 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -255,7 +255,7 @@ describe('DecryptionFailureTracker', function() { const error1 = new MockDecryptionError('ERROR_CODE_1'); const error2 = new MockDecryptionError('ERROR_CODE_2'); const error3 = new MockDecryptionError('ERROR_CODE_3'); - + tracker.addVisibleEvent(decryptedEvent1); tracker.addVisibleEvent(decryptedEvent2); tracker.addVisibleEvent(decryptedEvent3); @@ -272,8 +272,8 @@ describe('DecryptionFailureTracker', function() { expect(trackEventSpy).toHaveBeenCalledTimes(3); expect(trackEventSpy.mock.calls).toEqual( [['E2E', 'Decryption failure', 'UnknownError', '1'], - ['E2E', 'Decryption failure', 'UnknownError', '1'], - ['E2E', 'Decryption failure', 'UnknownError', '1']] + ['E2E', 'Decryption failure', 'UnknownError', '1'], + ['E2E', 'Decryption failure', 'UnknownError', '1']], ); }); From 241175a7e34036b788e29117e3bfb3a206f31674 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 21 Apr 2022 09:00:21 +0200 Subject: [PATCH 13/16] Revert "fix ts issues in DecryptionFailureTracker" This reverts commit 1ae748cc51088d60722320dbefae04a62310e2e1. Signed-off-by: Kerry Archibald --- test/DecryptionFailureTracker-test.ts | 166 ++++++++++++++++---------- 1 file changed, 104 insertions(+), 62 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 08fd5f9e38e..6033a14ba56 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -16,14 +16,10 @@ limitations under the License. import { MatrixEvent } from 'matrix-js-sdk/src/matrix'; -import Analytics from '../src/Analytics'; import { DecryptionFailureTracker } from '../src/DecryptionFailureTracker'; class MockDecryptionError extends Error { - constructor( - public readonly errcode: string, - public readonly data: Record = {}, - ) { + constructor(public readonly errcode = 'MOCK_DECRYPTION_ERROR') { super(); } } @@ -32,30 +28,20 @@ function createFailedDecryptionEvent() { const event = new MatrixEvent({ event_id: "event-id-" + Math.random().toString(16).slice(2), }); - // @ts-ignore private properties event.setClearData(event.badEncryptedMessage(":(")); return event; } describe('DecryptionFailureTracker', function() { - const trackEventSpy = jest.spyOn(Analytics, 'trackEvent'); - - beforeEach(() => { - jest.clearAllMocks(); - }); - - afterAll(() => { - jest.spyOn(Analytics, 'trackEvent').mockRestore(); - }); - - it('tracks a failed decryption for a visible event', function() { + it('tracks a failed decryption for a visible event', function(done) { const failedDecryptionEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); tracker.addVisibleEvent(failedDecryptionEvent); - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(failedDecryptionEvent, err); // Pretend "now" is Infinity @@ -65,15 +51,18 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should track a failure for an event that failed decryption - expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(count).not.toBe(0); + + done(); }); it('tracks a failed decryption for an event that becomes visible later', function(done) { const failedDecryptionEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(failedDecryptionEvent, err); tracker.addVisibleEvent(failedDecryptionEvent); @@ -85,17 +74,18 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should track a failure for an event that failed decryption - expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(count).not.toBe(0); done(); }); - it('does not track a failed decryption for an event that never becomes visible', function() { + it('does not track a failed decryption for an event that never becomes visible', function(done) { const failedDecryptionEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(failedDecryptionEvent, err); // Pretend "now" is Infinity @@ -105,20 +95,24 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should not track a failure for an event that never became visible - expect(trackEventSpy).not.toHaveBeenCalled(); + expect(count).toBe(0); + + done(); }); - it('does not track a failed decryption where the event is subsequently successfully decrypted', () => { + it('does not track a failed decryption where the event is subsequently successfully decrypted', (done) => { const decryptedEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + const tracker = new DecryptionFailureTracker((total) => { + // should not track an event that has since been decrypted correctly + expect(true).toBe(false); + }, () => "UnknownError"); tracker.addVisibleEvent(decryptedEvent); - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(decryptedEvent, err); // Indicate successful decryption: clear data can be anything where the msgtype is not m.bad.encrypted - // @ts-ignore private prop decryptedEvent.setClearData({}); tracker.eventDecrypted(decryptedEvent, null); @@ -127,21 +121,20 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - - // no tracking occurred - expect(trackEventSpy).not.toHaveBeenCalled(); + done(); }); it('does not track a failed decryption where the event is subsequently successfully decrypted ' + - 'and later becomes visible', () => { + 'and later becomes visible', (done) => { const decryptedEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + const tracker = new DecryptionFailureTracker((total) => { + expect(true).toBe(false, 'should not track an event that has since been decrypted correctly'); + }, () => "UnknownError"); - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(decryptedEvent, err); // Indicate successful decryption: clear data can be anything where the msgtype is not m.bad.encrypted - // @ts-ignore private property decryptedEvent.setClearData({}); tracker.eventDecrypted(decryptedEvent, null); @@ -152,19 +145,20 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - expect(trackEventSpy).not.toHaveBeenCalled(); + done(); }); - it('only tracks a single failure per error code, despite multiple failed decryptions for multiple events', () => { + it('only tracks a single failure per event, despite multiple failed decryptions for multiple events', (done) => { const decryptedEvent = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); tracker.addVisibleEvent(decryptedEvent); // Arbitrary number of failed decryptions for both events - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(decryptedEvent, err); tracker.eventDecrypted(decryptedEvent, err); tracker.eventDecrypted(decryptedEvent, err); @@ -184,18 +178,22 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); tracker.trackFailures(); - expect(trackEventSpy).toHaveBeenCalledTimes(1); + // should only track a single failure per event + expect(count).toBe(2); + + done(); }); it('should not track a failure for an event that was tracked previously', (done) => { const decryptedEvent = createFailedDecryptionEvent(); - const tracker = DecryptionFailureTracker.instance; + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); tracker.addVisibleEvent(decryptedEvent); // Indicate decryption - const err = new MockDecryptionError(''); + const err = new MockDecryptionError(); tracker.eventDecrypted(decryptedEvent, err); // Pretend "now" is Infinity @@ -209,20 +207,62 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); // should only track a single failure per event - expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(count).toBe(1); + + done(); + }); + + xit('should not track a failure for an event that was tracked in a previous session', (done) => { + // This test uses localStorage, clear it beforehand + localStorage.clear(); + + const decryptedEvent = createFailedDecryptionEvent(); + + let count = 0; + const tracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + + tracker.addVisibleEvent(decryptedEvent); + + // Indicate decryption + const err = new MockDecryptionError(); + tracker.eventDecrypted(decryptedEvent, err); + + // Pretend "now" is Infinity + // NB: This saves to localStorage specific to DFT + tracker.checkFailures(Infinity); + + tracker.trackFailures(); + + // Simulate the browser refreshing by destroying tracker and creating a new tracker + const secondTracker = new DecryptionFailureTracker((total) => count += total, () => "UnknownError"); + + secondTracker.addVisibleEvent(decryptedEvent); + + //secondTracker.loadTrackedEvents(); + + secondTracker.eventDecrypted(decryptedEvent, err); + secondTracker.checkFailures(Infinity); + secondTracker.trackFailures(); + + // should only track a single failure per event + expect(count).toBe(1); done(); }); it('should count different error codes separately for multiple failures with different error codes', () => { - const tracker = DecryptionFailureTracker.instance; + const counts = {}; + const tracker = new DecryptionFailureTracker( + (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, + (error) => error === "UnknownError" ? "UnknownError" : "OlmKeysNotSentError", + ); const decryptedEvent1 = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); const decryptedEvent3 = createFailedDecryptionEvent(); const error1 = new MockDecryptionError('UnknownError'); - const error2 = new MockDecryptionError('MEGOLM_UNKNOWN_INBOUND_SESSION_ID'); + const error2 = new MockDecryptionError('OlmKeysNotSentError'); tracker.addVisibleEvent(decryptedEvent1); tracker.addVisibleEvent(decryptedEvent2); @@ -239,13 +279,13 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // called once for each error type - expect(trackEventSpy).toHaveBeenCalledTimes(2); - expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'UnknownError', '1'); - expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'OlmKeysNotSentError', '2'); + //expect(counts['UnknownError']).toBe(1, 'should track one UnknownError'); + // should track two OlmKeysNotSentError + expect(counts['OlmKeysNotSentError']).toBe(2); }); it('should aggregate error codes correctly', () => { + const counts = {}; const tracker = DecryptionFailureTracker.instance; const decryptedEvent1 = createFailedDecryptionEvent(); @@ -269,20 +309,21 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - expect(trackEventSpy).toHaveBeenCalledTimes(3); - expect(trackEventSpy.mock.calls).toEqual( - [['E2E', 'Decryption failure', 'UnknownError', '1'], - ['E2E', 'Decryption failure', 'UnknownError', '1'], - ['E2E', 'Decryption failure', 'UnknownError', '1']], - ); + // should track three OlmUnspecifiedError + expect(counts['OlmUnspecifiedError']) + .toBe(3); }); it('should remap error codes correctly', () => { - const tracker = DecryptionFailureTracker.instance; + const counts = {}; + const tracker = new DecryptionFailureTracker( + (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, + (errorCode) => Array.from(errorCode).reverse().join(''), + ); const decryptedEvent = createFailedDecryptionEvent(); - const error = new MockDecryptionError('MEGOLM_UNKNOWN_INBOUND_SESSION_ID'); + const error = new MockDecryptionError('ERROR_CODE_1'); tracker.addVisibleEvent(decryptedEvent); @@ -293,7 +334,8 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // MEGOLM_UNKNOWN_INBOUND_SESSION_ID maps to OlmKeysNotSentError - expect(trackEventSpy).toHaveBeenCalledWith('E2E', 'Decryption failure', 'OlmKeysNotSentError', '1'); + // should track remapped error code + expect(counts['1_EDOC_RORRE']) + .toBe(1); }); }); From eed8e97594e7330f0872131b1ec350a2662f2ae7 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 21 Apr 2022 09:00:46 +0200 Subject: [PATCH 14/16] Revert "remove unsupported assertion failure messages" This reverts commit 7bd93d075c4d8d45befcbfd59c889782c9a44b48. --- test/DecryptionFailureTracker-test.ts | 39 ++++++++++++--------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 6033a14ba56..b0494f97aa6 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -19,8 +19,10 @@ import { MatrixEvent } from 'matrix-js-sdk/src/matrix'; import { DecryptionFailureTracker } from '../src/DecryptionFailureTracker'; class MockDecryptionError extends Error { - constructor(public readonly errcode = 'MOCK_DECRYPTION_ERROR') { + constructor(code) { super(); + + this.errcode = code || 'MOCK_DECRYPTION_ERROR'; } } @@ -50,8 +52,7 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - // should track a failure for an event that failed decryption - expect(count).not.toBe(0); + expect(count).not.toBe(0, 'should track a failure for an event that failed decryption'); done(); }); @@ -73,8 +74,7 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - // should track a failure for an event that failed decryption - expect(count).not.toBe(0); + expect(count).not.toBe(0, 'should track a failure for an event that failed decryption'); done(); }); @@ -94,8 +94,7 @@ describe('DecryptionFailureTracker', function() { // Immediately track the newest failures tracker.trackFailures(); - // should not track a failure for an event that never became visible - expect(count).toBe(0); + expect(count).toBe(0, 'should not track a failure for an event that never became visible'); done(); }); @@ -103,8 +102,7 @@ describe('DecryptionFailureTracker', function() { it('does not track a failed decryption where the event is subsequently successfully decrypted', (done) => { const decryptedEvent = createFailedDecryptionEvent(); const tracker = new DecryptionFailureTracker((total) => { - // should not track an event that has since been decrypted correctly - expect(true).toBe(false); + expect(true).toBe(false, 'should not track an event that has since been decrypted correctly'); }, () => "UnknownError"); tracker.addVisibleEvent(decryptedEvent); @@ -178,8 +176,7 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); tracker.trackFailures(); - // should only track a single failure per event - expect(count).toBe(2); + expect(count).toBe(2, count + ' failures tracked, should only track a single failure per event'); done(); }); @@ -206,8 +203,7 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // should only track a single failure per event - expect(count).toBe(1); + expect(count).toBe(1, 'should only track a single failure per event'); done(); }); @@ -244,8 +240,7 @@ describe('DecryptionFailureTracker', function() { secondTracker.checkFailures(Infinity); secondTracker.trackFailures(); - // should only track a single failure per event - expect(count).toBe(1); + expect(count).toBe(1, count + ' failures tracked, should only track a single failure per event'); done(); }); @@ -280,13 +275,15 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); //expect(counts['UnknownError']).toBe(1, 'should track one UnknownError'); - // should track two OlmKeysNotSentError - expect(counts['OlmKeysNotSentError']).toBe(2); + expect(counts['OlmKeysNotSentError']).toBe(2, 'should track two OlmKeysNotSentError'); }); it('should aggregate error codes correctly', () => { const counts = {}; - const tracker = DecryptionFailureTracker.instance; + const tracker = new DecryptionFailureTracker( + (total, errorCode) => counts[errorCode] = (counts[errorCode] || 0) + total, + (errorCode) => 'OlmUnspecifiedError', + ); const decryptedEvent1 = createFailedDecryptionEvent(); const decryptedEvent2 = createFailedDecryptionEvent(); @@ -309,9 +306,8 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // should track three OlmUnspecifiedError expect(counts['OlmUnspecifiedError']) - .toBe(3); + .toBe(3, 'should track three OlmUnspecifiedError, got ' + counts['OlmUnspecifiedError']); }); it('should remap error codes correctly', () => { @@ -334,8 +330,7 @@ describe('DecryptionFailureTracker', function() { tracker.trackFailures(); - // should track remapped error code expect(counts['1_EDOC_RORRE']) - .toBe(1); + .toBe(1, 'should track remapped error code'); }); }); From 259b4e5a8daa1bd28fdee083e7f7c889102a173d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 21 Apr 2022 09:01:03 +0200 Subject: [PATCH 15/16] Revert "test/DecryptionFailureTracker-test.js -> test/DecryptionFailureTracker-test.ts" This reverts commit 1670025bd2af9a355c2761998202f602d61f242e. --- ...ionFailureTracker-test.ts => DecryptionFailureTracker-test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{DecryptionFailureTracker-test.ts => DecryptionFailureTracker-test.js} (100%) diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.js similarity index 100% rename from test/DecryptionFailureTracker-test.ts rename to test/DecryptionFailureTracker-test.js From b5f353877958fea3824598c6e73863ece0495711 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 21 Apr 2022 09:02:45 +0200 Subject: [PATCH 16/16] revert change to DecryptionFailureTracker Signed-off-by: Kerry Archibald --- src/DecryptionFailureTracker.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/DecryptionFailureTracker.ts b/src/DecryptionFailureTracker.ts index e950ff4a46f..2bb522e7fe9 100644 --- a/src/DecryptionFailureTracker.ts +++ b/src/DecryptionFailureTracker.ts @@ -122,6 +122,14 @@ export class DecryptionFailureTracker { return DecryptionFailureTracker.internalInstance; } + // loadTrackedEvents() { + // this.trackedEvents = new Set(JSON.parse(localStorage.getItem('mx-decryption-failure-event-ids')) || []); + // } + + // saveTrackedEvents() { + // localStorage.setItem('mx-decryption-failure-event-ids', JSON.stringify([...this.trackedEvents])); + // } + public eventDecrypted(e: MatrixEvent, err: MatrixError): void { if (err) { this.addDecryptionFailure(new DecryptionFailure(e.getId(), err.errcode));