Mholloway has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/371727 )
Change subject: Hygiene: Require a RemoteLogException in logRemoteError[IfProd] ...................................................................... Hygiene: Require a RemoteLogException in logRemoteError[IfProd] We have a RemoteLogException class that's intended to extract useful information for an exception for remote logging. The logRemoteError and logRemoteErrorIfProd methods seem to expect this implicitly, but they are currently defined to accept generic Throwables. This updates their signatures to require a RemoteLogException, to avoid a situation in which errors are reported but without any useful diagnostic information.[1] [1] E.g., https://rink.hockeyapp.net/manage/apps/226650/app_versions/126/crash_reasons/180971093?type=overview Change-Id: I19366656db8d0af1832566cc4ec5a50c2fed6a67 --- M app/src/main/java/org/wikipedia/auth/AccountUtil.java M app/src/main/java/org/wikipedia/json/TabUnmarshaller.java M app/src/main/java/org/wikipedia/page/PageFragment.java M app/src/main/java/org/wikipedia/page/PageFragmentLoadState.java M app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java M app/src/main/java/org/wikipedia/util/log/L.java M app/src/main/java/org/wikipedia/views/FacePostprocessor.java 7 files changed, 23 insertions(+), 17 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia refs/changes/27/371727/1 diff --git a/app/src/main/java/org/wikipedia/auth/AccountUtil.java b/app/src/main/java/org/wikipedia/auth/AccountUtil.java index 4aae2d0..04eac2b 100644 --- a/app/src/main/java/org/wikipedia/auth/AccountUtil.java +++ b/app/src/main/java/org/wikipedia/auth/AccountUtil.java @@ -13,6 +13,7 @@ import org.wikipedia.R; import org.wikipedia.WikipediaApp; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.json.GsonMarshaller; import org.wikipedia.json.GsonUnmarshaller; import org.wikipedia.login.LoginResult; @@ -129,7 +130,7 @@ return accounts[0]; } } catch (SecurityException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } return null; } diff --git a/app/src/main/java/org/wikipedia/json/TabUnmarshaller.java b/app/src/main/java/org/wikipedia/json/TabUnmarshaller.java index 54f21fb..4b25f29 100644 --- a/app/src/main/java/org/wikipedia/json/TabUnmarshaller.java +++ b/app/src/main/java/org/wikipedia/json/TabUnmarshaller.java @@ -40,7 +40,7 @@ for (PageBackStackItem item : tab.getBackStack()) { if (TextUtils.isEmpty(item.getTitle().getWikiSite().authority()) || TextUtils.isEmpty(item.getHistoryEntry().getTitle().getWikiSite().authority())) { - L.logRemoteErrorIfProd(new IllegalArgumentException("Format error in serialized tab list.")); + L.logRemoteErrorIfProd(new RemoteLogException(new IllegalArgumentException("Format error in serialized tab list."))); bad = true; break; } diff --git a/app/src/main/java/org/wikipedia/page/PageFragment.java b/app/src/main/java/org/wikipedia/page/PageFragment.java index 4b3773c..6bdbaf9 100755 --- a/app/src/main/java/org/wikipedia/page/PageFragment.java +++ b/app/src/main/java/org/wikipedia/page/PageFragment.java @@ -48,6 +48,7 @@ import org.wikipedia.bridge.CommunicationBridge; import org.wikipedia.bridge.DarkModeMarshaller; import org.wikipedia.concurrency.CallbackTask; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.dataclient.WikiSite; import org.wikipedia.dataclient.okhttp.OkHttpWebViewClient; import org.wikipedia.descriptions.DescriptionEditActivity; @@ -1121,7 +1122,7 @@ builder.setView(textView); builder.show(); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } }); @@ -1140,7 +1141,7 @@ linkHandler.onUrlClick(href, messagePayload.optString("title")); } } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } }); @@ -1155,7 +1156,7 @@ GalleryFunnel.SOURCE_NON_LEAD_IMAGE), Constants.ACTIVITY_REQUEST_GALLERY); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } }); diff --git a/app/src/main/java/org/wikipedia/page/PageFragmentLoadState.java b/app/src/main/java/org/wikipedia/page/PageFragmentLoadState.java index ae22167..96f789c 100644 --- a/app/src/main/java/org/wikipedia/page/PageFragmentLoadState.java +++ b/app/src/main/java/org/wikipedia/page/PageFragmentLoadState.java @@ -22,6 +22,7 @@ import org.wikipedia.auth.AccountUtil; import org.wikipedia.bridge.CommunicationBridge; import org.wikipedia.concurrency.CallbackTask; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.database.contract.PageImageHistoryContract; import org.wikipedia.dataclient.ServiceError; import org.wikipedia.dataclient.mwapi.MwException; @@ -279,7 +280,7 @@ } pageLoadWebViewReady(); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } }); @@ -292,7 +293,7 @@ } pageLoadDisplayNonLeadSection(payload.getInt("index")); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } }); @@ -383,7 +384,7 @@ wrapper.put("sequence", sequenceNumber.get()); bridge.sendMessage("beginNewPage", wrapper); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } @@ -692,7 +693,7 @@ (int) (stagedScrollY / DimenUtil.getDensityScalar())); bridge.sendMessage("displaySection", wrapper); } catch (JSONException e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } } diff --git a/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java b/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java index c4ab2bb..2ef284a 100644 --- a/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java +++ b/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java @@ -7,6 +7,7 @@ import android.text.TextUtils; import org.wikipedia.WikipediaApp; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.database.contract.PageImageHistoryContract; import org.wikipedia.dataclient.WikiSite; import org.wikipedia.dataclient.okhttp.OkHttpConnectionFactory; @@ -145,7 +146,7 @@ if (!ThrowableUtil.isOffline(e)) { // If it's anything but a transient network error, let's log it aggressively, // to make sure we've fixed any other errors with saving pages. - L.logRemoteError(e); + L.logRemoteError(new RemoteLogException(e)); } dao.failDiskTransaction(row); continue; diff --git a/app/src/main/java/org/wikipedia/util/log/L.java b/app/src/main/java/org/wikipedia/util/log/L.java index c5cc850..59706ad 100644 --- a/app/src/main/java/org/wikipedia/util/log/L.java +++ b/app/src/main/java/org/wikipedia/util/log/L.java @@ -4,6 +4,7 @@ import android.support.annotation.Nullable; import android.util.Log; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.util.ReleaseUtil; /** Logging utility like {@link Log} but with implied tags. */ @@ -105,11 +106,11 @@ LEVEL_E.log(msg, t); } - public static void logRemoteErrorIfProd(@NonNull Throwable t) { + public static void logRemoteErrorIfProd(@NonNull RemoteLogException e) { if (ReleaseUtil.isProdRelease()) { - logRemoteError(t); + logRemoteError(e); } else { - throw new RuntimeException(t); + throw new RuntimeException(e); } } @@ -119,10 +120,10 @@ // Favor logRemoteErrorIfProd(). If it's worth consuming bandwidth and developer hours, it's // worth crashing on everything but prod - public static void logRemoteError(@NonNull Throwable t) { - LEVEL_E.log("", t); + public static void logRemoteError(@NonNull RemoteLogException e) { + LEVEL_E.log("", e); if (REMOTE_EXCEPTION_LOGGER != null) { - REMOTE_EXCEPTION_LOGGER.log(t); + REMOTE_EXCEPTION_LOGGER.log(e); } } diff --git a/app/src/main/java/org/wikipedia/views/FacePostprocessor.java b/app/src/main/java/org/wikipedia/views/FacePostprocessor.java index 90e7a2c..90e8703 100644 --- a/app/src/main/java/org/wikipedia/views/FacePostprocessor.java +++ b/app/src/main/java/org/wikipedia/views/FacePostprocessor.java @@ -17,6 +17,7 @@ import org.wikipedia.R; import org.wikipedia.WikipediaApp; +import org.wikipedia.crash.RemoteLogException; import org.wikipedia.util.MathUtil; import org.wikipedia.util.log.L; @@ -48,7 +49,7 @@ try { facePos = detectFace(testBmp); } catch (OutOfMemoryError e) { - L.logRemoteErrorIfProd(e); + L.logRemoteErrorIfProd(new RemoteLogException(e)); } int defaultColor = ContextCompat.getColor(WikipediaApp.getInstance(), R.color.dark_gray); listener.get().onImageLoaded(destBitmap.getHeight(), facePos, -- To view, visit https://gerrit.wikimedia.org/r/371727 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I19366656db8d0af1832566cc4ec5a50c2fed6a67 Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Mholloway <mhollo...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
