From 22f2d6f292be9a5c4b8b52d973e20ac7b21f688d Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 14:08:10 +0100 Subject: [PATCH 1/2] Fix crash when browser doesn't report page change measurement --- src/Analytics.js | 9 ++++++--- src/components/structures/MatrixChat.js | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Analytics.js b/src/Analytics.js index 8b55bdfd64..0d1313f48d 100644 --- a/src/Analytics.js +++ b/src/Analytics.js @@ -164,9 +164,13 @@ class Analytics { } trackPageChange(generationTimeMs) { - if (typeof generationTimeMs !== 'number') { - throw new Error('Analytics.trackPageChange: expected generationTimeMs to be a number'); + if (typeof generationTimeMs === 'number') { + this._paq.push(['setGenerationTimeMs', generationTimeMs]); + } else { + console.warn('Analytics.trackPageChange: expected generationTimeMs to be a number'); + // But continue anyway because we still want to track the change } + if (this.disabled) return; if (this.firstPage) { // De-duplicate first page @@ -175,7 +179,6 @@ class Analytics { return; } this._paq.push(['setCustomUrl', getRedactedUrl()]); - this._paq.push(['setGenerationTimeMs', generationTimeMs]); this._paq.push(['trackPageView']); } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 8df46d2f7c..3005bc86ad 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -413,6 +413,10 @@ export default React.createClass({ performance.clearMarks('riot_MatrixChat_page_change_start'); performance.clearMarks('riot_MatrixChat_page_change_stop'); const measurement = performance.getEntriesByName('riot_MatrixChat_page_change_delta').pop(); + + // In practice, sometimes the entries list is empty, so we get no measurement + if (!measurement) return null; + return measurement.duration; }, From 3ba51ba69586f9a015fdb5dc265445ca0b22ad2c Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 18:25:00 +0100 Subject: [PATCH 2/2] Null check before accessing _paq --- src/Analytics.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Analytics.js b/src/Analytics.js index 0d1313f48d..ec8234eb83 100644 --- a/src/Analytics.js +++ b/src/Analytics.js @@ -164,13 +164,6 @@ class Analytics { } trackPageChange(generationTimeMs) { - if (typeof generationTimeMs === 'number') { - this._paq.push(['setGenerationTimeMs', generationTimeMs]); - } else { - console.warn('Analytics.trackPageChange: expected generationTimeMs to be a number'); - // But continue anyway because we still want to track the change - } - if (this.disabled) return; if (this.firstPage) { // De-duplicate first page @@ -178,6 +171,14 @@ class Analytics { this.firstPage = false; return; } + + if (typeof generationTimeMs === 'number') { + this._paq.push(['setGenerationTimeMs', generationTimeMs]); + } else { + console.warn('Analytics.trackPageChange: expected generationTimeMs to be a number'); + // But continue anyway because we still want to track the change + } + this._paq.push(['setCustomUrl', getRedactedUrl()]); this._paq.push(['trackPageView']); }