Merge pull request #5626 from wordpress-mobile/issue/check-null-analytics-events

Check null analytics events

Author: maxme

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 message) {
+        super(message);
+    }
+}

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<String, ?> 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:

libs/analytics/WordPressAnalytics/src/main/java/org/wordpress/android/analytics/AnalyticsTrackerNosara.java

@@ -41,6 +41,10 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> 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");
@@ -156,27 +160,43 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> 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"
-        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);
         }
 
@@ -187,8 +207,6 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> properties) {
         }
     }
 
-
-
     @Override
     public void endSession() {
         this.flush();
@@ -251,6 +269,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";

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;
+    }
 }