Turned catch Throwables into a specific exception

jkasten2 · jkasten2 · commit b9d692977751 · 2020-03-22T13:50:00.000-07:00
* Catching Throwable is hiding root case exceptions which is causing unrelated exceptions to throw
  - Such as an NPE due to something not being set or a Thread trying to start twice.
* Only first pass, many more we can clean up.
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/BadgeCountUpdater.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/BadgeCountUpdater.java
@@ -38,6 +38,7 @@
 import android.service.notification.StatusBarNotification;
 import android.support.annotation.RequiresApi;
 
+import com.onesignal.shortcutbadger.ShortcutBadgeException;
 import com.onesignal.shortcutbadger.ShortcutBadger;
 
 import com.onesignal.OneSignalDbContract.NotificationTable;
@@ -62,9 +63,9 @@ private static boolean areBadgeSettingsEnabled(Context context) {
          }
          else
             badgesEnabled = 1;
-      } catch (Throwable t) {
+      } catch (PackageManager.NameNotFoundException e) {
          badgesEnabled = 0;
-         OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error reading meta-data tag 'com.onesignal.BadgeCount'. Disabling badge setting.", t);
+         OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error reading meta-data tag 'com.onesignal.BadgeCount'. Disabling badge setting.", e);
       }
 
       return (badgesEnabled == 1);
@@ -120,10 +121,13 @@ static void updateCount(int count, Context context) {
       if (!areBadgeSettingsEnabled(context))
          return;
 
-      // Can throw if badges are not support on the device.
-      //  Or app does not have a default launch Activity.
       try {
          ShortcutBadger.applyCountOrThrow(context, count);
-      } catch(Throwable t) {}
+      } catch (ShortcutBadgeException e) {
+         // Suppress error as there are normal cases where this will throw
+         // Can throw if:
+         //    - Badges are not support on the device.
+         //    - App does not have a default launch Activity.
+      }
    }
 }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/GooglePlayServicesUpgradePrompt.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/GooglePlayServicesUpgradePrompt.java
@@ -30,8 +30,10 @@ private static boolean isGooglePlayStoreInstalled() {
          PackageManager pm = OneSignal.appContext.getPackageManager();
          PackageInfo info = pm.getPackageInfo(GoogleApiAvailability.GOOGLE_PLAY_SERVICES_PACKAGE, PackageManager.GET_META_DATA);
          String label = (String) info.applicationInfo.loadLabel(pm);
-         return (label != null && !label.equals("Market"));
-      } catch (Throwable e) {}
+         return (!label.equals("Market"));
+      } catch (PackageManager.NameNotFoundException e) {
+         // Google Play Store might not be installed, ignore exception if so
+      }
 
       return false;
    }
@@ -86,13 +88,15 @@ private static void OpenPlayStoreToApp(Activity activity) {
          GoogleApiAvailability apiAvailability = GoogleApiAvailability.getInstance();
          int resultCode = apiAvailability.isGooglePlayServicesAvailable(OneSignal.appContext);
          // Send the Intent to trigger opening the store
-         apiAvailability.getErrorResolutionPendingIntent(
-            activity,
-            resultCode,
-            PLAY_SERVICES_RESOLUTION_REQUEST
-         ).send();
+         PendingIntent pendingIntent =
+             apiAvailability.getErrorResolutionPendingIntent(
+                 activity,
+                 resultCode,
+                 PLAY_SERVICES_RESOLUTION_REQUEST
+             );
+         if (pendingIntent != null)
+            pendingIntent.send();
       } catch (PendingIntent.CanceledException e) {
-      } catch (Throwable e) {
          e.printStackTrace();
       }
    }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/JSONUtils.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/JSONUtils.java
@@ -125,7 +125,7 @@ static String toStringNE(JSONArray jsonArray) {
         try {
             for (int i = 0; i < jsonArray.length(); i++)
                 strArray += "\"" + jsonArray.getString(i) + "\"";
-        } catch (Throwable t) {}
+        } catch (JSONException ignored) {}
 
         return strArray + "]";
     }
@@ -148,7 +148,7 @@ static JSONObject getJSONObjectWithoutBlankValues(JSONObject jsonObject, String
                 value = keyValues.get(key);
                 if (!"".equals(value))
                     toReturn.put(key, value);
-            } catch (Throwable t) {}
+            } catch (JSONException ignored) {}
         }
 
         return toReturn;
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/LocationGMS.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/LocationGMS.java
@@ -164,8 +164,8 @@ else if (locationCoarsePermission == PackageManager.PERMISSION_GRANTED)
                   startGetLocation();
                else
                   fireFailedComplete();
-            } catch (Throwable t) {
-               t.printStackTrace();
+            } catch (PackageManager.NameNotFoundException e) {
+               e.printStackTrace();
             }
          }
          else
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSEmailSubscriptionStateChanges.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSEmailSubscriptionStateChanges.java
@@ -27,6 +27,7 @@
 
 package com.onesignal;
 
+import org.json.JSONException;
 import org.json.JSONObject;
 
 public class OSEmailSubscriptionStateChanges {
@@ -46,9 +47,8 @@ public JSONObject toJSONObject() {
         try {
             mainObj.put("from", from.toJSONObject());
             mainObj.put("to", to.toJSONObject());
-        }
-        catch(Throwable t) {
-            t.printStackTrace();
+        } catch (JSONException e) {
+            e.printStackTrace();
         }
 
         return mainObj;
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotification.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotification.java
@@ -141,8 +141,8 @@ public JSONObject toJSONObject() {
 
          mainObj.put("payload", payload.toJSONObject());
       }
-      catch(Throwable t) {
-         t.printStackTrace();
+      catch(JSONException e) {
+         e.printStackTrace();
       }
 
       return mainObj;
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSObservable.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSObservable.java
@@ -28,6 +28,7 @@
 package com.onesignal;
 
 import java.lang.ref.WeakReference;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
@@ -72,30 +73,39 @@ boolean notifyChange(final StateType state) {
             strongRefObserver = observer;
          
          if (strongRefObserver != null) {
+            Class<?> clazz = strongRefObserver.getClass();
             try {
-               Class<?> clazz = strongRefObserver.getClass();
                final Method method = clazz.getDeclaredMethod(methodName, state.getClass());
                method.setAccessible(true);
                if (fireOnMainThread) {
                   OSUtils.runOnMainUIThread(
-                     new Runnable() {
-                        @Override
-                        public void run() {
-                           try {
-                              method.invoke(strongRefObserver, state);
-                           }
-                           catch (Throwable t) {
-                              t.printStackTrace();
-                           }
-                        }
-                     });
-                  
+                      new Runnable() {
+                          @Override
+                          public void run() {
+                              try {
+                                  method.invoke(strongRefObserver, state);
+                              } catch (IllegalAccessException e) {
+                                  e.printStackTrace();
+                              } catch (InvocationTargetException e) {
+                                  e.printStackTrace();
+                              }
+                          }
+                      }
+                 );
                }
-               else
-                  method.invoke(strongRefObserver, state);
+               else {
+                  try {
+                     method.invoke(strongRefObserver, state);
+                  } catch (IllegalAccessException e) {
+                     e.printStackTrace();
+                  } catch (InvocationTargetException e) {
+                     e.printStackTrace();
+                  }
+               }
+
                notified = true;
-            } catch (Throwable t) {
-               t.printStackTrace();
+            } catch (NoSuchMethodException e) {
+               e.printStackTrace();
             }
          }
       }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSSubscriptionState.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSSubscriptionState.java
@@ -30,6 +30,7 @@
 
 import android.support.annotation.Nullable;
 
+import org.json.JSONException;
 import org.json.JSONObject;
 
 public class OSSubscriptionState implements Cloneable {
@@ -139,8 +140,9 @@ boolean compare(OSSubscriptionState from) {
    protected Object clone() {
       try {
          return super.clone();
-      } catch (Throwable t) {}
-      return null;
+      } catch (CloneNotSupportedException e) {
+         return null;
+      }
    }
    
    public JSONObject toJSONObject() {
@@ -160,8 +162,8 @@ public JSONObject toJSONObject() {
          mainObj.put("userSubscriptionSetting", userSubscriptionSetting);
          mainObj.put("subscribed", getSubscribed());
       }
-      catch(Throwable t) {
-         t.printStackTrace();
+      catch(JSONException e) {
+         e.printStackTrace();
       }
       
       return mainObj;
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalDbHelper.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalDbHelper.java
@@ -29,6 +29,7 @@
 
 import android.content.Context;
 import android.database.Cursor;
+import android.database.sqlite.SQLiteCantOpenDatabaseException;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteException;
 import android.database.sqlite.SQLiteOpenHelper;
@@ -134,9 +135,9 @@ synchronized SQLiteDatabase getWritableDbWithRetries() {
       while(true) {
          try {
             return getWritableDatabase();
-         } catch (Throwable t) {
+         } catch (SQLiteCantOpenDatabaseException e) {
             if (++count >= DB_OPEN_RETRY_MAX)
-               throw t;
+               throw e;
             SystemClock.sleep(count * DB_OPEN_RETRY_BACKOFF);
          }
       }
@@ -147,9 +148,9 @@ synchronized SQLiteDatabase getReadableDbWithRetries() {
       while(true) {
          try {
             return getReadableDatabase();
-         } catch (Throwable t) {
+         } catch (SQLiteCantOpenDatabaseException e) {
             if (++count >= DB_OPEN_RETRY_MAX)
-               throw t;
+               throw e;
             SystemClock.sleep(count * DB_OPEN_RETRY_BACKOFF);
          }
       }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/OutcomeEventsCache.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/OutcomeEventsCache.java
@@ -3,6 +3,7 @@
 import android.content.ContentValues;
 import android.database.Cursor;
 import android.database.sqlite.SQLiteDatabase;
+import android.database.sqlite.SQLiteException;
 import android.support.annotation.WorkerThread;
 
 import com.onesignal.OneSignalDbContract.OutcomeEventsTable;
@@ -28,14 +29,14 @@ synchronized static void deleteOldOutcomeEvent(OutcomeEvent event, OneSignalDbHe
             writableDb.delete(OutcomeEventsTable.TABLE_NAME,
                     OutcomeEventsTable.COLUMN_NAME_TIMESTAMP + " = ?", new String[]{String.valueOf(event.getTimestamp())});
             writableDb.setTransactionSuccessful();
-        } catch (Throwable t) {
-            OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error deleting old outcome event records! ", t);
+        } catch (SQLiteException e) {
+            OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error deleting old outcome event records! ", e);
         } finally {
             if (writableDb != null) {
                 try {
                     writableDb.endTransaction(); // May throw if transaction was never opened or DB is full.
-                } catch (Throwable t) {
-                    OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error closing transaction! ", t);
+                } catch (SQLiteException e) {
+                    OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error closing transaction! ", e);
                 }
             }
         }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/TrackAmazonPurchase.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/TrackAmazonPurchase.java
@@ -28,11 +28,13 @@
 package com.onesignal;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.content.Context;
@@ -72,11 +74,25 @@ class TrackAmazonPurchase {
 
          canTrack = true;
          setListener();
-      } catch (Throwable t) {
-         OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error adding Amazon IAP listener.", t);
+         // Can replace all catches with ReflectiveOperationException win min API is 19
+      } catch (ClassNotFoundException e) {
+         logAmazonIAPListenerError(e);
+      } catch (IllegalAccessException e) {
+         logAmazonIAPListenerError(e);
+      } catch (InvocationTargetException e) {
+         logAmazonIAPListenerError(e);
+      } catch (NoSuchMethodException e) {
+         logAmazonIAPListenerError(e);
+      } catch (NoSuchFieldException e) {
+         logAmazonIAPListenerError(e);
       }
    }
 
+   private static void logAmazonIAPListenerError(Exception e) {
+      OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error adding Amazon IAP listener.", e);
+      e.printStackTrace();
+   }
+
    private void setListener() {
       PurchasingService.registerListener(context, osPurchasingListener);
    }
@@ -90,7 +106,8 @@ void checkListener() {
             osPurchasingListener.orgPurchasingListener = curPurchasingListener;
             setListener();
          }
-      } catch (Throwable t) {
+      } catch (IllegalAccessException e) {
+         e.printStackTrace();
       }
    }
 
@@ -149,27 +166,23 @@ public void onProductDataResponse(final ProductDataResponse response) {
                   OneSignal.sendPurchases(purchasesToReport, false, null);
                   break;
                }
-            } catch (Throwable t) {
-               t.printStackTrace();
+            } catch (JSONException e) {
+               e.printStackTrace();
             }
          } else if (orgPurchasingListener != null)
             orgPurchasingListener.onProductDataResponse(response);
       }
 
       @Override
       public void onPurchaseResponse(PurchaseResponse response) {
-         try {
-            final PurchaseResponse.RequestStatus status = response.getRequestStatus();
+         final PurchaseResponse.RequestStatus status = response.getRequestStatus();
 
-            if (status == PurchaseResponse.RequestStatus.SUCCESSFUL) {
-               currentMarket = response.getUserData().getMarketplace();
+         if (status == PurchaseResponse.RequestStatus.SUCCESSFUL) {
+            currentMarket = response.getUserData().getMarketplace();
 
-               Set<String> productSkus = new HashSet<String>();
-               productSkus.add(response.getReceipt().getSku());
-               lastRequestId = PurchasingService.getProductData(productSkus);
-            }
-         } catch (Throwable t) {
-            t.printStackTrace();
+            Set<String> productSkus = new HashSet<>();
+            productSkus.add(response.getReceipt().getSku());
+            lastRequestId = PurchasingService.getProductData(productSkus);
          }
 
          if (orgPurchasingListener != null)
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/TrackFirebaseAnalytics.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/TrackFirebaseAnalytics.java
@@ -56,7 +56,7 @@ static boolean CanTrack() {
       try {
          FirebaseAnalyticsClass = Class.forName("com.google.firebase.analytics.FirebaseAnalytics");
          return true;
-      } catch (Throwable t) {
+      } catch (ClassNotFoundException e) {
          return false;
       }
    }
diff --git a/OneSignalSDK/onesignal/src/main/java/com/onesignal/UserStateSynchronizer.java b/OneSignalSDK/onesignal/src/main/java/com/onesignal/UserStateSynchronizer.java
diff --git a/OneSignalSDK/unittest/src/test/java/com/test/onesignal/GenerateNotificationRunner.java b/OneSignalSDK/unittest/src/test/java/com/test/onesignal/GenerateNotificationRunner.java
diff --git a/OneSignalSDK/unittest/src/test/java/com/test/onesignal/NotificationLimitManagerRunner.java b/OneSignalSDK/unittest/src/test/java/com/test/onesignal/NotificationLimitManagerRunner.java

Original file line number	Diff line number	Diff line change
`@@ -164,8 +164,8 @@ else if (locationCoarsePermission == PackageManager.PERMISSION_GRANTED)`
`164`	`164`	`startGetLocation();`
`165`	`165`	`else`
`166`	`166`	`fireFailedComplete();`
`167`		`- } catch (Throwable t) {`
`168`		`- t.printStackTrace();`
	`167`	`+ } catch (PackageManager.NameNotFoundException e) {`
	`168`	`+ e.printStackTrace();`
`169`	`169`	`}`
`170`	`170`	`}`
`171`	`171`	`else`
Original file line number	Diff line number	Diff line change
`@@ -141,8 +141,8 @@ public JSONObject toJSONObject() {`
`141`	`141`
`142`	`142`	`mainObj.put("payload", payload.toJSONObject());`
`143`	`143`	`}`
`144`		`- catch(Throwable t) {`
`145`		`- t.printStackTrace();`
	`144`	`+ catch(JSONException e) {`
	`145`	`+ e.printStackTrace();`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`return mainObj;`