From 2c51bc0ca5a4c3a4bb140b4b40dabdda859ebb94 Mon Sep 17 00:00:00 2001 From: unarist Date: Mon, 2 Apr 2018 21:51:02 +0900 Subject: [PATCH] Add missing rejection handling for Promises (#7008) * Add eslint-plugin-promise to detect uncaught rejections * Move alert generation for errors to actions/alert * Add missing rejection handling for Promises * Use catch() instead of onReject on then() Then it will catches rejection from onFulfilled. This detection can be disabled by `allowThen` option, though. --- .eslintrc.yml | 3 ++ app/javascript/mastodon/actions/accounts.js | 2 +- app/javascript/mastodon/actions/alerts.js | 25 ++++++++++++++++ app/javascript/mastodon/actions/compose.js | 7 ++++- app/javascript/mastodon/actions/lists.js | 3 +- .../actions/push_notifications/registerer.js | 15 ++++------ app/javascript/mastodon/actions/settings.js | 5 +++- .../mastodon/containers/status_container.js | 6 +++- .../features/ui/components/embed_modal.js | 3 ++ app/javascript/mastodon/middleware/errors.js | 24 ++------------- app/javascript/mastodon/storage/modifier.js | 30 ++++++++++++++----- package.json | 1 + yarn.lock | 4 +++ 13 files changed, 84 insertions(+), 44 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index cf276a16f..576e5b70a 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -13,6 +13,7 @@ plugins: - react - jsx-a11y - import +- promise parserOptions: sourceType: module @@ -152,3 +153,5 @@ rules: - "app/javascript/**/__tests__/**" import/no-unresolved: error import/no-webpack-loader-syntax: error + + promise/catch-or-return: error diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index 7cacff909..28ae56763 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -103,7 +103,7 @@ export function fetchAccount(id) { dispatch(importFetchedAccount(response.data)); })).then(() => { dispatch(fetchAccountSuccess()); - }, error => { + }).catch(error => { dispatch(fetchAccountFail(id, error)); }); }; diff --git a/app/javascript/mastodon/actions/alerts.js b/app/javascript/mastodon/actions/alerts.js index f37fdeeb6..3f5d7ef46 100644 --- a/app/javascript/mastodon/actions/alerts.js +++ b/app/javascript/mastodon/actions/alerts.js @@ -1,3 +1,10 @@ +import { defineMessages } from 'react-intl'; + +const messages = defineMessages({ + unexpectedTitle: { id: 'alert.unexpected.title', defaultMessage: 'Oops!' }, + unexpectedMessage: { id: 'alert.unexpected.message', defaultMessage: 'An unexpected error occurred.' }, +}); + export const ALERT_SHOW = 'ALERT_SHOW'; export const ALERT_DISMISS = 'ALERT_DISMISS'; export const ALERT_CLEAR = 'ALERT_CLEAR'; @@ -22,3 +29,21 @@ export function showAlert(title, message) { message, }; }; + +export function showAlertForError(error) { + if (error.response) { + const { data, status, statusText } = error.response; + + let message = statusText; + let title = `${status}`; + + if (data.error) { + message = data.error; + } + + return showAlert(title, message); + } else { + console.error(error); + return showAlert(messages.unexpectedTitle, messages.unexpectedMessage); + } +} diff --git a/app/javascript/mastodon/actions/compose.js b/app/javascript/mastodon/actions/compose.js index 2138f9426..59aa6f98d 100644 --- a/app/javascript/mastodon/actions/compose.js +++ b/app/javascript/mastodon/actions/compose.js @@ -1,11 +1,12 @@ import api from '../api'; -import { CancelToken } from 'axios'; +import { CancelToken, isCancel } from 'axios'; import { throttle } from 'lodash'; import { search as emojiSearch } from '../features/emoji/emoji_mart_search_light'; import { tagHistory } from '../settings'; import { useEmoji } from './emojis'; import { importFetchedAccounts } from './importer'; import { updateTimeline } from './timelines'; +import { showAlertForError } from './alerts'; let cancelFetchComposeSuggestionsAccounts; @@ -291,6 +292,10 @@ const fetchComposeSuggestionsAccounts = throttle((dispatch, getState, token) => }).then(response => { dispatch(importFetchedAccounts(response.data)); dispatch(readyComposeSuggestionsAccounts(token, response.data)); + }).catch(error => { + if (!isCancel(error)) { + dispatch(showAlertForError(error)); + } }); }, 200, { leading: true, trailing: true }); diff --git a/app/javascript/mastodon/actions/lists.js b/app/javascript/mastodon/actions/lists.js index 12d60e3a3..12cb17159 100644 --- a/app/javascript/mastodon/actions/lists.js +++ b/app/javascript/mastodon/actions/lists.js @@ -1,5 +1,6 @@ import api from '../api'; import { importFetchedAccounts } from './importer'; +import { showAlertForError } from './alerts'; export const LIST_FETCH_REQUEST = 'LIST_FETCH_REQUEST'; export const LIST_FETCH_SUCCESS = 'LIST_FETCH_SUCCESS'; @@ -236,7 +237,7 @@ export const fetchListSuggestions = q => (dispatch, getState) => { api(getState).get('/api/v1/accounts/search', { params }).then(({ data }) => { dispatch(importFetchedAccounts(data)); dispatch(fetchListSuggestionsReady(q, data)); - }); + }).catch(error => dispatch(showAlertForError(error))); }; export const fetchListSuggestionsReady = (query, accounts) => ({ diff --git a/app/javascript/mastodon/actions/push_notifications/registerer.js b/app/javascript/mastodon/actions/push_notifications/registerer.js index 51e68cad1..f17d929a6 100644 --- a/app/javascript/mastodon/actions/push_notifications/registerer.js +++ b/app/javascript/mastodon/actions/push_notifications/registerer.js @@ -116,14 +116,11 @@ export function register () { pushNotificationsSetting.remove(me); } - try { - getRegistration() - .then(getPushSubscription) - .then(unsubscribe); - } catch (e) { - - } - }); + return getRegistration() + .then(getPushSubscription) + .then(unsubscribe); + }) + .catch(console.warn); } else { console.warn('Your browser does not support Web Push Notifications.'); } @@ -143,6 +140,6 @@ export function saveSettings() { if (me) { pushNotificationsSetting.set(me, data); } - }); + }).catch(console.warn); }; } diff --git a/app/javascript/mastodon/actions/settings.js b/app/javascript/mastodon/actions/settings.js index b96383daa..5634a11ef 100644 --- a/app/javascript/mastodon/actions/settings.js +++ b/app/javascript/mastodon/actions/settings.js @@ -1,5 +1,6 @@ import api from '../api'; import { debounce } from 'lodash'; +import { showAlertForError } from './alerts'; export const SETTING_CHANGE = 'SETTING_CHANGE'; export const SETTING_SAVE = 'SETTING_SAVE'; @@ -23,7 +24,9 @@ const debouncedSave = debounce((dispatch, getState) => { const data = getState().get('settings').filter((_, path) => path !== 'saved').toJS(); - api(getState).put('/api/web/settings', { data }).then(() => dispatch({ type: SETTING_SAVE })); + api(getState).put('/api/web/settings', { data }) + .then(() => dispatch({ type: SETTING_SAVE })) + .catch(error => dispatch(showAlertForError(error))); }, 5000, { trailing: true }); export function saveSettings() { diff --git a/app/javascript/mastodon/containers/status_container.js b/app/javascript/mastodon/containers/status_container.js index 8ba1015b5..4579bd132 100644 --- a/app/javascript/mastodon/containers/status_container.js +++ b/app/javascript/mastodon/containers/status_container.js @@ -27,6 +27,7 @@ import { initReport } from '../actions/reports'; import { openModal } from '../actions/modal'; import { defineMessages, injectIntl, FormattedMessage } from 'react-intl'; import { boostModal, deleteModal } from '../initial_state'; +import { showAlertForError } from '../actions/alerts'; const messages = defineMessages({ deleteConfirm: { id: 'confirmations.delete.confirm', defaultMessage: 'Delete' }, @@ -83,7 +84,10 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ }, onEmbed (status) { - dispatch(openModal('EMBED', { url: status.get('url') })); + dispatch(openModal('EMBED', { + url: status.get('url'), + onError: error => dispatch(showAlertForError(error)), + })); }, onDelete (status) { diff --git a/app/javascript/mastodon/features/ui/components/embed_modal.js b/app/javascript/mastodon/features/ui/components/embed_modal.js index d440a8826..52aab00d0 100644 --- a/app/javascript/mastodon/features/ui/components/embed_modal.js +++ b/app/javascript/mastodon/features/ui/components/embed_modal.js @@ -10,6 +10,7 @@ export default class EmbedModal extends ImmutablePureComponent { static propTypes = { url: PropTypes.string.isRequired, onClose: PropTypes.func.isRequired, + onError: PropTypes.func.isRequired, intl: PropTypes.object.isRequired, } @@ -35,6 +36,8 @@ export default class EmbedModal extends ImmutablePureComponent { iframeDocument.body.style.margin = 0; this.iframe.width = iframeDocument.body.scrollWidth; this.iframe.height = iframeDocument.body.scrollHeight; + }).catch(error => { + this.props.onError(error); }); } diff --git a/app/javascript/mastodon/middleware/errors.js b/app/javascript/mastodon/middleware/errors.js index 72e5631e6..3cebb42e0 100644 --- a/app/javascript/mastodon/middleware/errors.js +++ b/app/javascript/mastodon/middleware/errors.js @@ -1,34 +1,14 @@ -import { defineMessages } from 'react-intl'; -import { showAlert } from '../actions/alerts'; +import { showAlertForError } from '../actions/alerts'; const defaultFailSuffix = 'FAIL'; -const messages = defineMessages({ - unexpectedTitle: { id: 'alert.unexpected.title', defaultMessage: 'Oops!' }, - unexpectedMessage: { id: 'alert.unexpected.message', defaultMessage: 'An unexpected error occurred.' }, -}); - export default function errorsMiddleware() { return ({ dispatch }) => next => action => { if (action.type && !action.skipAlert) { const isFail = new RegExp(`${defaultFailSuffix}$`, 'g'); if (action.type.match(isFail)) { - if (action.error.response) { - const { data, status, statusText } = action.error.response; - - let message = statusText; - let title = `${status}`; - - if (data.error) { - message = data.error; - } - - dispatch(showAlert(title, message)); - } else { - console.error(action.error); - dispatch(showAlert(messages.unexpectedTitle, messages.unexpectedMessage)); - } + dispatch(showAlertForError(action.error)); } } diff --git a/app/javascript/mastodon/storage/modifier.js b/app/javascript/mastodon/storage/modifier.js index 1bec04d0f..4773d07a9 100644 --- a/app/javascript/mastodon/storage/modifier.js +++ b/app/javascript/mastodon/storage/modifier.js @@ -9,6 +9,12 @@ const limit = 1024; // https://webkit.org/status/#specification-service-workers const asyncCache = window.caches ? caches.open('mastodon-system') : Promise.reject(); +function printErrorIfAvailable(error) { + if (error) { + console.warn(error); + } +} + function put(name, objects, onupdate, oncreate) { return asyncDB.then(db => new Promise((resolve, reject) => { const putTransaction = db.transaction(name, 'readwrite'); @@ -77,7 +83,9 @@ function evictAccountsByRecords(records) { function evict(toEvict) { toEvict.forEach(record => { - asyncCache.then(cache => accountAssetKeys.forEach(key => cache.delete(records[key]))); + asyncCache + .then(cache => accountAssetKeys.forEach(key => cache.delete(records[key]))) + .catch(printErrorIfAvailable); accountsMovedIndex.getAll(record.id).onsuccess = ({ target }) => evict(target.result); @@ -90,11 +98,11 @@ function evictAccountsByRecords(records) { } evict(records); - }); + }).catch(printErrorIfAvailable); } export function evictStatus(id) { - return evictStatuses([id]); + evictStatuses([id]); } export function evictStatuses(ids) { @@ -110,7 +118,7 @@ export function evictStatuses(ids) { idIndex.getKey(id).onsuccess = ({ target }) => target.result && store.delete(target.result); }); - }); + }).catch(printErrorIfAvailable); } function evictStatusesByRecords(records) { @@ -127,7 +135,9 @@ export function putAccounts(records) { const oldURL = target.result[key]; if (newURL !== oldURL) { - asyncCache.then(cache => cache.delete(oldURL)); + asyncCache + .then(cache => cache.delete(oldURL)) + .catch(printErrorIfAvailable); } }); @@ -145,10 +155,14 @@ export function putAccounts(records) { oncomplete(); }).then(records => { evictAccountsByRecords(records); - asyncCache.then(cache => cache.addAll(newURLs)); - }); + asyncCache + .then(cache => cache.addAll(newURLs)) + .catch(printErrorIfAvailable); + }).catch(printErrorIfAvailable); } export function putStatuses(records) { - put('statuses', records).then(evictStatusesByRecords); + put('statuses', records) + .then(evictStatusesByRecords) + .catch(printErrorIfAvailable); } diff --git a/package.json b/package.json index dfba49afc..d4de3a157 100644 --- a/package.json +++ b/package.json @@ -129,6 +129,7 @@ "eslint": "^4.15.0", "eslint-plugin-import": "^2.8.0", "eslint-plugin-jsx-a11y": "^5.1.1", + "eslint-plugin-promise": "^3.7.0", "eslint-plugin-react": "^7.5.1", "jest": "^21.2.1", "raf": "^3.4.0", diff --git a/yarn.lock b/yarn.lock index a306ebf55..866b24c7a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2433,6 +2433,10 @@ eslint-plugin-jsx-a11y@^5.1.1: emoji-regex "^6.1.0" jsx-ast-utils "^1.4.0" +eslint-plugin-promise@^3.7.0: + version "3.7.0" + resolved "https://registry.yarnpkg.com/eslint-plugin-promise/-/eslint-plugin-promise-3.7.0.tgz#f4bde5c2c77cdd69557a8f69a24d1ad3cfc9e67e" + eslint-plugin-react@^7.5.1: version "7.5.1" resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.5.1.tgz#52e56e8d80c810de158859ef07b880d2f56ee30b"