jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/349284 )
Change subject: Crash fix: Remove reliance on a Context to detect a 404 ...................................................................... Crash fix: Remove reliance on a Context to detect a 404 This should fix the top crash from the 4/19 beta: https://rink.hockeyapp.net/manage/apps/226650/app_versions/93/crash_reasons/165970817 Updates ThrowableUtil.is404 to drop its alternate branch which draws upon ThrowableUtil.getAppError. 404s are only expected in the context of missing pages or summaries for them, both of which we get with Retrofit, not java-mwapi. Retrofit throws HttpStatusExceptions, which we don't need a Context to check. If we get 404s from the MediaWiki API (as opposed to the expected 200s with error messages), we've got bigger problems, and in any case, java-mwapi will be gone Real Soon Now (TM). Perhaps more controversially, deprecates getAppError and its supporting methods as well as the AppError class itself. Our philsophy around user error messages has shifted since this code was written in favor of standard, predefined error views. Using these legacy methods requires passing in a Context, which can be hazardous. Further, they're already nearly unused. Change-Id: I53c944b2b96ac2ca9e36191b582fb13d5f5c2455 --- M app/src/main/java/org/wikipedia/page/PageFragment.java M app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java M app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorType.java M app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorView.java M app/src/main/java/org/wikipedia/util/ThrowableUtil.java M app/src/main/java/org/wikipedia/views/WikiErrorView.java 6 files changed, 19 insertions(+), 20 deletions(-) Approvals: Dbrant: Looks good to me, but someone else must approve Niedzielski: Looks good to me, approved jenkins-bot: Verified diff --git a/app/src/main/java/org/wikipedia/page/PageFragment.java b/app/src/main/java/org/wikipedia/page/PageFragment.java index f166dc1..cfc2d6a 100755 --- a/app/src/main/java/org/wikipedia/page/PageFragment.java +++ b/app/src/main/java/org/wikipedia/page/PageFragment.java @@ -937,7 +937,7 @@ disableActionTabs(caught); - refreshView.setEnabled(!ThrowableUtil.is404(getContext(), caught)); + refreshView.setEnabled(!ThrowableUtil.is404(caught)); errorState = true; if (callback() != null) { callback().onPageLoadError(getTitle()); diff --git a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java index fab9599..b405117 100755 --- a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java +++ b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java @@ -227,7 +227,7 @@ errorContainer.setVisibility(View.VISIBLE); errorContainer.setError(caught); errorContainer.setCallback(this); - LinkPreviewErrorType errorType = LinkPreviewErrorType.get(getContext(), caught); + LinkPreviewErrorType errorType = LinkPreviewErrorType.get(caught); overlayView.setPrimaryButtonText(getResources().getString(errorType.buttonText())); overlayView.setCallback(errorType.buttonAction(errorContainer)); if (errorType != LinkPreviewErrorType.OFFLINE) { diff --git a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorType.java b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorType.java index e80eeac..1339693 100644 --- a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorType.java +++ b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorType.java @@ -1,6 +1,5 @@ package org.wikipedia.page.linkpreview; -import android.content.Context; import android.support.annotation.DrawableRes; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -61,8 +60,8 @@ @NonNull abstract LinkPreviewOverlayView.Callback buttonAction(@NonNull LinkPreviewErrorView errorView); - @NonNull public static LinkPreviewErrorType get(@NonNull Context context, @Nullable Throwable caught) { - if (caught != null && ThrowableUtil.is404(context, caught)) { + @NonNull public static LinkPreviewErrorType get(@Nullable Throwable caught) { + if (caught != null && ThrowableUtil.is404(caught)) { return LinkPreviewErrorType.PAGE_MISSING; } if (caught != null && ThrowableUtil.isOffline(caught)) { diff --git a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorView.java b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorView.java index 46faa82..692f8f8 100644 --- a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorView.java +++ b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewErrorView.java @@ -54,7 +54,7 @@ } public void setError(@Nullable Throwable caught) { - LinkPreviewErrorType errorType = LinkPreviewErrorType.get(getContext(), caught); + LinkPreviewErrorType errorType = LinkPreviewErrorType.get(caught); icon.setImageDrawable(ContextCompat.getDrawable(getContext(), errorType.icon())); // HACK: This message is delivered in one piece in a link preview but as a separate primary diff --git a/app/src/main/java/org/wikipedia/util/ThrowableUtil.java b/app/src/main/java/org/wikipedia/util/ThrowableUtil.java index d81e7ee..e2d8ed2 100644 --- a/app/src/main/java/org/wikipedia/util/ThrowableUtil.java +++ b/app/src/main/java/org/wikipedia/util/ThrowableUtil.java @@ -45,7 +45,15 @@ return false; } - @NonNull + /** + * DEPRECATED: This is a rarely-used function intended to sift through server error responses + * and pass through the relevant bits along with standardized strings in certain cases. + * + * Getting the handful of canned strings depends on processing contexts that might be null by + * the time we make it here. Further, we're moving away from using raw server messages in favor + * of statically defined XML error views, which are safer. This should no longer be used. + */ + @NonNull @Deprecated public static AppError getAppError(@NonNull Context context, @NonNull Throwable e) { Throwable inner = ThrowableUtil.getInnermostThrowable(e); AppError result; @@ -81,16 +89,8 @@ || caught instanceof SocketTimeoutException; } - public static boolean is404(@NonNull Context ctx, @NonNull Throwable caught) { - return is404(caught) || is404(ThrowableUtil.getAppError(ctx, caught)); - } - - @SuppressWarnings("checkstyle:magicnumber") private static boolean is404(@NonNull Throwable caught) { + @SuppressWarnings("checkstyle:magicnumber") public static boolean is404(@NonNull Throwable caught) { return caught instanceof HttpStatusException && ((HttpStatusException) caught).code() == 404; - } - - private static boolean is404(@NonNull ThrowableUtil.AppError e) { - return e.getDetail() != null && e.getDetail().contains("404"); } private static boolean isNetworkError(@NonNull Throwable e) { @@ -101,7 +101,7 @@ || ThrowableUtil.throwableContainsException(e, SSLException.class); } - @NonNull + @NonNull @Deprecated private static String getApiError(@NonNull Context context, @NonNull ApiException e) { String text; if ("missingtitle".equals(e.getCode()) || "invalidtitle".equals(e.getCode())) { @@ -113,7 +113,7 @@ } // TODO: migrate this to ApiException.toString() - @NonNull + @NonNull @Deprecated private static String getApiErrorMessage(@NonNull Context c, @NonNull ApiException e) { String text; if (e.getInfo() != null) { @@ -129,7 +129,7 @@ return text; } - public static class AppError { + @Deprecated public static class AppError { private String error; private String detail; public AppError(@NonNull String error, @Nullable String detail) { diff --git a/app/src/main/java/org/wikipedia/views/WikiErrorView.java b/app/src/main/java/org/wikipedia/views/WikiErrorView.java index 5877fd1..3ab9a0b 100644 --- a/app/src/main/java/org/wikipedia/views/WikiErrorView.java +++ b/app/src/main/java/org/wikipedia/views/WikiErrorView.java @@ -84,7 +84,7 @@ caught = caught.getCause(); } - if (caught != null && is404(context, caught)) { + if (caught != null && is404(caught)) { return ErrorType.PAGE_MISSING; } if (caught != null && isOffline(caught)) { -- To view, visit https://gerrit.wikimedia.org/r/349284 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I53c944b2b96ac2ca9e36191b582fb13d5f5c2455 Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Mholloway <[email protected]> Gerrit-Reviewer: Dbrant <[email protected]> Gerrit-Reviewer: Mholloway <[email protected]> Gerrit-Reviewer: Niedzielski <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
