From 9b3eda2b3dcd4430418474f0666ed58c817f680e Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 12:15:44 +0200 Subject: [PATCH 1/6] Add analytics exception --- .../wordpress/android/analytics/AnalyticsException.java | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java new file mode 100644 index 000000000000..dd15b8fe49b7 --- /dev/null +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java @@ -0,0 +1,7 @@ +package org.wordpress.android.analytics; + +public class AnalyticsException extends Exception { + public AnalyticsException(String detailMessage) { + super(detailMessage); + } +} From 426ef88a9be564de20bff091c285b5da09898fe7 Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 12:16:27 +0200 Subject: [PATCH 2/6] Add method that ckecks if the event is valid (not null for now) --- .../org/wordpress/android/analytics/Tracker.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/Tracker.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/Tracker.java index e1b35f1252de..fd76e215b3cb 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/Tracker.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/Tracker.java @@ -74,4 +74,16 @@ String getWordPressComUserName() { void setWordPressComUserName(String userName) { mWpcomUserName = userName; } + + public static boolean isValidEvent(Stat stat) { + if (stat == null) { + try { + throw new AnalyticsException("Trying to track a null stat event!"); + } catch (AnalyticsException e) { + AppLog.e(AppLog.T.STATS, e); + } + return false; + } + return true; + } } From 54f5533131ad8148b19947b7eb8ec42054e9e97b Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 12:17:03 +0200 Subject: [PATCH 3/6] Check if the event is valid before bumping it --- .../android/analytics/AnalyticsTrackerMixpanel.java | 10 ++++++++-- .../android/analytics/AnalyticsTrackerNosara.java | 8 ++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerMixpanel.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerMixpanel.java index 6555f8081ee1..31d68f8f68cd 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerMixpanel.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerMixpanel.java @@ -61,6 +61,10 @@ public void track(AnalyticsTracker.Stat stat) { @Override public void track(AnalyticsTracker.Stat stat, Map properties) { + if (!isValidEvent(stat)) { + return; + } + AnalyticsTrackerMixpanelInstructionsForStat instructions = instructionsForStat(stat); if (instructions == null) { @@ -242,8 +246,10 @@ public void clearAllData() { } } - private AnalyticsTrackerMixpanelInstructionsForStat instructionsForStat( - AnalyticsTracker.Stat stat) { + private AnalyticsTrackerMixpanelInstructionsForStat instructionsForStat(AnalyticsTracker.Stat stat) { + if (!isValidEvent(stat)) { + return null; + } AnalyticsTrackerMixpanelInstructionsForStat instructions; switch (stat) { case APPLICATION_OPENED: diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java index c6aa6bd0526b..bbd4bfe234dd 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java @@ -41,6 +41,10 @@ public void track(AnalyticsTracker.Stat stat, Map properties) { return; } + if (!isValidEvent(stat)) { + return; + } + String eventName = getEventNameForStat(stat); if (eventName == null) { AppLog.w(AppLog.T.STATS, "There is NO match for the event " + stat.name() + "stat"); @@ -251,6 +255,10 @@ public void registerPushNotificationToken(String regId) { } public static String getEventNameForStat(AnalyticsTracker.Stat stat) { + if (!isValidEvent(stat)) { + return null; + } + switch (stat) { case APPLICATION_OPENED: return "application_opened"; From ceab9fc75c75bb7936dd0695c0c42bb474011724 Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 12:33:13 +0200 Subject: [PATCH 4/6] Rename variable --- .../org/wordpress/android/analytics/AnalyticsException.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java index dd15b8fe49b7..ccf0132cc603 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsException.java @@ -1,7 +1,7 @@ package org.wordpress.android.analytics; public class AnalyticsException extends Exception { - public AnalyticsException(String detailMessage) { - super(detailMessage); + public AnalyticsException(String message) { + super(message); } } From 53b83013ca4deb0d171ab169decc6285c64b6664 Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 12:49:49 +0200 Subject: [PATCH 5/6] Check null key values on Properties passed to events --- .../analytics/AnalyticsTrackerNosara.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java index bbd4bfe234dd..ebf372b68d2f 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java @@ -163,24 +163,30 @@ public void track(AnalyticsTracker.Stat stat, Map properties) { // create the merged JSON Object of properties // Properties defined by the user have precedence over the default ones pre-defined at "event level" - final JSONObject propertiesToJSON; + JSONObject propertiesToJSON = null; if (properties != null && properties.size() > 0) { - propertiesToJSON = new JSONObject(properties); - for (String key : predefinedEventProperties.keySet()) { - try { - if (propertiesToJSON.has(key)) { - AppLog.w(AppLog.T.STATS, "The user has defined a property named: '" + key + "' that will override" + - "the same property pre-defined at event level. This may generate unexpected behavior!!"); - AppLog.w(AppLog.T.STATS, "User value: " + propertiesToJSON.get(key).toString() + " - pre-defined value: " + - predefinedEventProperties.get(key).toString()); - } else { - propertiesToJSON.put(key, predefinedEventProperties.get(key)); + try { + propertiesToJSON = new JSONObject(properties); + for (String key : predefinedEventProperties.keySet()) { + try { + if (propertiesToJSON.has(key)) { + AppLog.w(AppLog.T.STATS, "The user has defined a property named: '" + key + "' that will override" + + "the same property pre-defined at event level. This may generate unexpected behavior!!"); + AppLog.w(AppLog.T.STATS, "User value: " + propertiesToJSON.get(key).toString() + " - pre-defined value: " + + predefinedEventProperties.get(key).toString()); + } else { + propertiesToJSON.put(key, predefinedEventProperties.get(key)); + } + } catch (JSONException e) { + AppLog.e(AppLog.T.STATS, "Error while merging user-defined properties with pre-defined properties", e); } - } catch (JSONException e) { - AppLog.e(AppLog.T.STATS, "Error while merging user-defined properties with pre-defined properties", e); } + } catch (NullPointerException e) { + AppLog.e(AppLog.T.STATS, "A property passed to the event " + eventName + " has null key!", e); } - } else{ + } + + if (propertiesToJSON == null) { propertiesToJSON = new JSONObject(predefinedEventProperties); } @@ -191,8 +197,6 @@ public void track(AnalyticsTracker.Stat stat, Map properties) { } } - - @Override public void endSession() { this.flush(); From e9ab8604d4dc8be56dcefbfae809cc4fd6a82870 Mon Sep 17 00:00:00 2001 From: Danilo Ercoli Date: Wed, 12 Apr 2017 15:15:39 +0200 Subject: [PATCH 6/6] Check if the user is null before bumping the event into Tracks --- .../android/analytics/AnalyticsTrackerNosara.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java index ebf372b68d2f..924f888d14e4 100644 --- a/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java +++ b/libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java @@ -160,6 +160,16 @@ public void track(AnalyticsTracker.Stat stat, Map properties) { userType = TracksClient.NosaraUserType.ANON; } + // It seems that we're tracking some events with user = null. Make sure we're catching the error here. + if (user == null) { + try { + throw new AnalyticsException("Trying to track analytics with an null user!"); + // TODO add CrashlyticsUtils.logException or track this error in Nosara by using a special test user. + } catch (AnalyticsException e) { + AppLog.e(AppLog.T.STATS, e); + } + return; + } // create the merged JSON Object of properties // Properties defined by the user have precedence over the default ones pre-defined at "event level"