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

@@ -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

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));
     });
   };

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);
+  }
+}

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 });
 

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) => ({

app/javascript/mastodon/actions/push_notifications/registerer.js

@@ -116,14 +116,11 @@ export function register () {
             pushNotificationsSetting.remove(me);
           }
 
-          try {
+          return getRegistration()
-            getRegistration()
+            .then(getPushSubscription)
-              .then(getPushSubscription)
+            .then(unsubscribe);
-              .then(unsubscribe);
+        })
-          } catch (e) {
+        .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);
   };
 }

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() {

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) {

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);
     });
   }
 

app/javascript/mastodon/middleware/errors.js

@@ -1,34 +1,14 @@
-import { defineMessages } from 'react-intl';
+import { showAlertForError } from '../actions/alerts';
-import { showAlert } 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) {
+        dispatch(showAlertForError(action.error));
-          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));
-        }
       }
     }
 

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);
 }

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",

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"