From 0d117c106aa72f78dd5cdd371849dd8ce3120198 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sat, 28 Mar 2020 17:59:45 +0100 Subject: [PATCH] Fix 404 and 410 API errors being silently discarded in WebUI (#13279) * Fix 404 and 410 API errors being silently discarded in WebUI Fixes #13278 * Return more appropriate error when user replies to a deleted toot * Please CodeClimate * Fix 404/410 errors on fetching account timelines & identity proofs * Refactor error handling * Move error message string to statuses.errors --- app/controllers/api/v1/statuses_controller.rb | 9 ++++++++- app/javascript/mastodon/actions/accounts.js | 3 +++ app/javascript/mastodon/actions/alerts.js | 4 ++-- app/javascript/mastodon/actions/identity_proofs.js | 1 + app/javascript/mastodon/actions/timelines.js | 1 + app/javascript/mastodon/middleware/errors.js | 2 +- config/locales/en.yml | 2 ++ 7 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 2f55e95fd2..93a253cbb2 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -7,6 +7,7 @@ class Api::V1::StatusesController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: [:create, :destroy] before_action :require_user!, except: [:show, :context] before_action :set_status, only: [:show, :context] + before_action :set_thread, only: [:create] override_rate_limit_headers :create, family: :statuses @@ -36,7 +37,7 @@ class Api::V1::StatusesController < Api::BaseController def create @status = PostStatusService.new.call(current_user.account, text: status_params[:status], - thread: status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]), + thread: @thread, media_ids: status_params[:media_ids], sensitive: status_params[:sensitive], spoiler_text: status_params[:spoiler_text], @@ -69,6 +70,12 @@ class Api::V1::StatusesController < Api::BaseController raise ActiveRecord::RecordNotFound end + def set_thread + @thread = status_params[:in_reply_to_id].blank? ? nil : Status.find(status_params[:in_reply_to_id]) + rescue ActiveRecord::RecordNotFound + render json: { error: I18n.t('statuses.errors.in_reply_not_found') }, status: 404 + end + def status_params params.permit( :status, diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index 4af36e9983..cb2c682a45 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -396,6 +396,7 @@ export function fetchFollowersFail(id, error) { type: FOLLOWERS_FETCH_FAIL, id, error, + skipNotFound: true, }; }; @@ -482,6 +483,7 @@ export function fetchFollowingFail(id, error) { type: FOLLOWING_FETCH_FAIL, id, error, + skipNotFound: true, }; }; @@ -571,6 +573,7 @@ export function fetchRelationshipsFail(error) { type: RELATIONSHIPS_FETCH_FAIL, error, skipLoading: true, + skipNotFound: true, }; }; diff --git a/app/javascript/mastodon/actions/alerts.js b/app/javascript/mastodon/actions/alerts.js index cd36d8007b..1670f9c10d 100644 --- a/app/javascript/mastodon/actions/alerts.js +++ b/app/javascript/mastodon/actions/alerts.js @@ -34,11 +34,11 @@ export function showAlert(title = messages.unexpectedTitle, message = messages.u }; }; -export function showAlertForError(error) { +export function showAlertForError(error, skipNotFound = false) { if (error.response) { const { data, status, statusText, headers } = error.response; - if (status === 404 || status === 410) { + if (skipNotFound && (status === 404 || status === 410)) { // Skip these errors as they are reflected in the UI return { type: ALERT_NOOP }; } diff --git a/app/javascript/mastodon/actions/identity_proofs.js b/app/javascript/mastodon/actions/identity_proofs.js index 449debf616..1039839566 100644 --- a/app/javascript/mastodon/actions/identity_proofs.js +++ b/app/javascript/mastodon/actions/identity_proofs.js @@ -27,4 +27,5 @@ export const fetchAccountIdentityProofsFail = (accountId, err) => ({ type: IDENTITY_PROOFS_ACCOUNT_FETCH_FAIL, accountId, err, + skipNotFound: true, }); diff --git a/app/javascript/mastodon/actions/timelines.js b/app/javascript/mastodon/actions/timelines.js index 0546686559..cdd2111f8e 100644 --- a/app/javascript/mastodon/actions/timelines.js +++ b/app/javascript/mastodon/actions/timelines.js @@ -149,6 +149,7 @@ export function expandTimelineFail(timeline, error, isLoadingMore) { timeline, error, skipLoading: !isLoadingMore, + skipNotFound: timeline.startsWith('account:'), }; }; diff --git a/app/javascript/mastodon/middleware/errors.js b/app/javascript/mastodon/middleware/errors.js index 3cebb42e00..0a65fd3211 100644 --- a/app/javascript/mastodon/middleware/errors.js +++ b/app/javascript/mastodon/middleware/errors.js @@ -8,7 +8,7 @@ export default function errorsMiddleware() { const isFail = new RegExp(`${defaultFailSuffix}$`, 'g'); if (action.type.match(isFail)) { - dispatch(showAlertForError(action.error)); + dispatch(showAlertForError(action.error, action.skipNotFound)); } } diff --git a/config/locales/en.yml b/config/locales/en.yml index 13f22d9226..ef21fa54a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1078,6 +1078,8 @@ en: disallowed_hashtags: one: 'contained a disallowed hashtag: %{tags}' other: 'contained the disallowed hashtags: %{tags}' + errors: + in_reply_not_found: The status you are trying to reply to does not appear to exist. language_detection: Automatically detect language open_in_web: Open in web over_character_limit: character limit of %{max} exceeded