diff --git a/lib/containers/aggregated-reviews-container.js b/lib/containers/aggregated-reviews-container.js index 975e69c5d6..da72a399b4 100644 --- a/lib/containers/aggregated-reviews-container.js +++ b/lib/containers/aggregated-reviews-container.js @@ -24,6 +24,9 @@ export class BareAggregatedReviewsContainer extends React.Component { // only fetch summaries when we specify a summariesRenderer summariesRenderer: PropTypes.func, + + // Report errors during refetch + reportRelayError: PropTypes.func.isRequired, } constructor(props) { @@ -76,8 +79,12 @@ export class BareAggregatedReviewsContainer extends React.Component { commentCursor: null, }, null, - () => { - this.emitter.emit('did-refetch'); + err => { + if (err) { + this.props.reportRelayError('Unable to refresh reviews', err); + } else { + this.emitter.emit('did-refetch'); + } callback(); }, {force: true}, diff --git a/lib/containers/comment-decorations-container.js b/lib/containers/comment-decorations-container.js index 0e9fb37ed9..8e62cb7e5a 100644 --- a/lib/containers/comment-decorations-container.js +++ b/lib/containers/comment-decorations-container.js @@ -20,6 +20,8 @@ export default class CommentDecorationsContainer extends React.Component { commands: PropTypes.object.isRequired, localRepository: PropTypes.object.isRequired, loginModel: GithubLoginModelPropType.isRequired, + + reportRelayError: PropTypes.func.isRequired, }; render() { @@ -46,7 +48,7 @@ export default class CommentDecorationsContainer extends React.Component { } renderWithToken(token, {repoData}) { - if (!token || token === UNAUTHENTICATED || token === INSUFFICIENT) { + if (!token || token === UNAUTHENTICATED || token === INSUFFICIENT || token instanceof Error) { // we're not going to prompt users to log in to render decorations for comments // just let it go and move on with our lives. return null; @@ -177,7 +179,9 @@ export default class CommentDecorationsContainer extends React.Component { } return ( - + {({errors, summaries, commentThreads}) => { if (errors && errors.length > 0) { // eslint-disable-next-line no-console diff --git a/lib/containers/github-tab-container.js b/lib/containers/github-tab-container.js index c68b3324cd..0805632222 100644 --- a/lib/containers/github-tab-container.js +++ b/lib/containers/github-tab-container.js @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import yubikiri from 'yubikiri'; import {GithubLoginModelPropType, RefHolderPropType} from '../prop-types'; -import {autobind} from '../helpers'; import OperationStateObserver, {PUSH, PULL, FETCH} from '../models/operation-state-observer'; import GitHubTabController from '../controllers/github-tab-controller'; import ObserveModel from '../views/observe-model'; @@ -18,12 +17,7 @@ export default class GitHubTabContainer extends React.Component { rootHolder: RefHolderPropType.isRequired, } - constructor(props) { - super(props); - autobind(this, 'fetchRepositoryData', 'renderRepositoryData'); - - this.state = {}; - } + state = {}; static getDerivedStateFromProps(props, state) { if (props.repository !== state.lastRepository) { @@ -36,7 +30,7 @@ export default class GitHubTabContainer extends React.Component { return null; } - fetchRepositoryData(repository) { + fetchRepositoryData = repository => { return yubikiri({ workingDirectory: repository.getWorkingDirectoryPath(), allRemotes: repository.getRemotes(), @@ -59,7 +53,7 @@ export default class GitHubTabContainer extends React.Component { ); } - renderRepositoryData(data) { + renderRepositoryData = data => { if (!data || this.props.repository.isLoading()) { return ( { const token = tokenData && tokenData.token; + if (token instanceof Error) { + return ( + + ); + } + if (token === UNAUTHENTICATED) { return ; } @@ -193,7 +204,9 @@ export default class IssueishDetailContainer extends React.Component { if (props.repository.issueish.__typename === 'PullRequest') { return ( - + {aggregatedReviews => this.renderWithCommentResult(token, repoData, {props, retry}, aggregatedReviews)} ); @@ -247,7 +260,7 @@ export default class IssueishDetailContainer extends React.Component { onTabSelected={this.props.onTabSelected} onOpenFilesTab={this.props.onOpenFilesTab} endpoint={this.props.endpoint} - reportMutationErrors={this.props.reportMutationErrors} + reportRelayError={this.props.reportRelayError} workspace={this.props.workspace} commands={this.props.commands} @@ -283,4 +296,6 @@ export default class IssueishDetailContainer extends React.Component { handleLogin = token => this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token); handleLogout = () => this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount()); + + handleTokenRetry = () => this.props.loginModel.didUpdate(); } diff --git a/lib/containers/remote-container.js b/lib/containers/remote-container.js index 5b8a155b57..3655edbe65 100644 --- a/lib/containers/remote-container.js +++ b/lib/containers/remote-container.js @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import {QueryRenderer, graphql} from 'react-relay'; import {incrementCounter} from '../reporter-proxy'; -import {autobind} from '../helpers'; import { RemotePropType, RemoteSetPropType, BranchSetPropType, OperationStateObserverPropType, EndpointPropType, } from '../prop-types'; @@ -35,13 +34,7 @@ export default class RemoteContainer extends React.Component { onPushBranch: PropTypes.func.isRequired, } - constructor(props) { - super(props); - - autobind(this, 'fetchToken', 'renderWithToken', 'renderWithResult', 'handleLogin', 'handleLogout'); - } - - fetchToken(loginModel) { + fetchToken = loginModel => { return loginModel.getToken(this.props.endpoint.getLoginAccount()); } @@ -53,11 +46,22 @@ export default class RemoteContainer extends React.Component { ); } - renderWithToken(token) { + renderWithToken = token => { if (token === null) { return ; } + if (token instanceof Error) { + return ( + + ); + } + if (token === UNAUTHENTICATED) { return ; } @@ -137,13 +141,15 @@ export default class RemoteContainer extends React.Component { ); } - handleLogin(token) { + handleLogin = token => { incrementCounter('github-login'); this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token); } - handleLogout() { + handleLogout = () => { incrementCounter('github-logout'); this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount()); } + + handleTokenRetry = () => this.props.loginModel.didUpdate(); } diff --git a/lib/containers/reviews-container.js b/lib/containers/reviews-container.js index 45b62d6afa..cf76c4b841 100644 --- a/lib/containers/reviews-container.js +++ b/lib/containers/reviews-container.js @@ -42,7 +42,7 @@ export default class ReviewsContainer extends React.Component { tooltips: PropTypes.object.isRequired, // Action methods - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, } render() { @@ -58,6 +58,17 @@ export default class ReviewsContainer extends React.Component { return ; } + if (token instanceof Error) { + return ( + + ); + } + if (token === UNAUTHENTICATED) { return ; } @@ -173,14 +184,16 @@ export default class ReviewsContainer extends React.Component { } return ( - + {({errors, summaries, commentThreads, refetch}) => { if (errors && errors.length > 0) { return errors.map((err, i) => ( )); } @@ -241,4 +254,6 @@ export default class ReviewsContainer extends React.Component { handleLogin = token => this.props.loginModel.setToken(this.props.endpoint.getLoginAccount(), token); handleLogout = () => this.props.loginModel.removeToken(this.props.endpoint.getLoginAccount()); + + handleTokenRetry = () => this.props.loginModel.didUpdate(); } diff --git a/lib/controllers/emoji-reactions-controller.js b/lib/controllers/emoji-reactions-controller.js index 9adbab1d40..b29d76422d 100644 --- a/lib/controllers/emoji-reactions-controller.js +++ b/lib/controllers/emoji-reactions-controller.js @@ -19,7 +19,7 @@ export class BareEmojiReactionsController extends React.Component { tooltips: PropTypes.object.isRequired, // Action methods - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, } render() { @@ -36,7 +36,7 @@ export class BareEmojiReactionsController extends React.Component { try { await addReactionMutation(this.props.relay.environment, this.props.reactable.id, content); } catch (err) { - this.props.reportMutationErrors('Unable to add reaction emoji', err); + this.props.reportRelayError('Unable to add reaction emoji', err); } }; @@ -44,7 +44,7 @@ export class BareEmojiReactionsController extends React.Component { try { await removeReactionMutation(this.props.relay.environment, this.props.reactable.id, content); } catch (err) { - this.props.reportMutationErrors('Unable to remove reaction emoji', err); + this.props.reportRelayError('Unable to remove reaction emoji', err); } }; } diff --git a/lib/controllers/github-tab-controller.js b/lib/controllers/github-tab-controller.js index 1f66e132f0..dc850c2e7f 100644 --- a/lib/controllers/github-tab-controller.js +++ b/lib/controllers/github-tab-controller.js @@ -4,7 +4,6 @@ import PropTypes from 'prop-types'; import { GithubLoginModelPropType, RefHolderPropType, RemoteSetPropType, BranchSetPropType, OperationStateObserverPropType, } from '../prop-types'; -import {autobind} from '../helpers'; import GitHubTabView from '../views/github-tab-view'; export default class GitHubTabController extends React.Component { @@ -24,11 +23,6 @@ export default class GitHubTabController extends React.Component { isLoading: PropTypes.bool.isRequired, } - constructor(props) { - super(props); - autobind(this, 'handlePushBranch', 'handleRemoteSelect'); - } - render() { const gitHubRemotes = this.props.allRemotes.filter(remote => remote.isGithubRepo()); const currentBranch = this.props.branches.getHeadBranch(); @@ -64,14 +58,14 @@ export default class GitHubTabController extends React.Component { ); } - handlePushBranch(currentBranch, targetRemote) { + handlePushBranch = (currentBranch, targetRemote) => { return this.props.repository.push(currentBranch.getName(), { remote: targetRemote, setUpstream: true, }); } - handleRemoteSelect(e, remote) { + handleRemoteSelect = (e, remote) => { e.preventDefault(); return this.props.repository.setConfig('atomGithub.currentRemote', remote.getName()); } diff --git a/lib/controllers/issueish-detail-controller.js b/lib/controllers/issueish-detail-controller.js index 3d3a3b4828..18875d4feb 100644 --- a/lib/controllers/issueish-detail-controller.js +++ b/lib/controllers/issueish-detail-controller.js @@ -61,7 +61,7 @@ export class BareIssueishDetailController extends React.Component { onTitleChange: PropTypes.func.isRequired, switchToIssueish: PropTypes.func.isRequired, destroy: PropTypes.func.isRequired, - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, // Item context itemType: ItemTypePropType.isRequired, @@ -147,7 +147,7 @@ export class BareIssueishDetailController extends React.Component { openReviews={this.openReviews} switchToIssueish={this.props.switchToIssueish} destroy={this.props.destroy} - reportMutationErrors={this.props.reportMutationErrors} + reportRelayError={this.props.reportRelayError} itemType={this.props.itemType} refEditor={this.props.refEditor} @@ -170,7 +170,7 @@ export class BareIssueishDetailController extends React.Component { issue={repository.issue} switchToIssueish={this.props.switchToIssueish} tooltips={this.props.tooltips} - reportMutationErrors={this.props.reportMutationErrors} + reportRelayError={this.props.reportRelayError} /> ); } diff --git a/lib/controllers/reviews-controller.js b/lib/controllers/reviews-controller.js index 877458382e..2ba9e7263b 100644 --- a/lib/controllers/reviews-controller.js +++ b/lib/controllers/reviews-controller.js @@ -66,7 +66,7 @@ export class BareReviewsController extends React.Component { tooltips: PropTypes.object.isRequired, // Action methods - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, } constructor(props) { @@ -262,7 +262,7 @@ export class BareReviewsController extends React.Component { addEvent('resolve-comment-thread', {package: 'github'}); } catch (err) { this.showThreadID(thread.id); - this.props.reportMutationErrors('Unable to resolve the comment thread', err); + this.props.reportRelayError('Unable to resolve the comment thread', err); } } } @@ -278,7 +278,7 @@ export class BareReviewsController extends React.Component { this.highlightThread(thread.id); addEvent('unresolve-comment-thread', {package: 'github'}); } catch (err) { - this.props.reportMutationErrors('Unable to unresolve the comment thread', err); + this.props.reportRelayError('Unable to unresolve the comment thread', err); } } } @@ -337,7 +337,7 @@ export class BareReviewsController extends React.Component { } } - this.props.reportMutationErrors('Unable to submit your comment', error); + this.props.reportRelayError('Unable to submit your comment', error); } finally { this.setState({postingToThreadID: null}); } diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index ae64fffa77..4cfb211e62 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -289,6 +289,7 @@ export default class RootController extends React.Component { commands={this.props.commandRegistry} localRepository={this.props.repository} loginModel={this.props.loginModel} + reportRelayError={this.reportRelayError} /> ); } @@ -441,7 +442,7 @@ export default class RootController extends React.Component { tooltips={this.props.tooltips} config={this.props.config} - reportMutationErrors={this.reportMutationErrors} + reportRelayError={this.reportRelayError} /> )} @@ -462,7 +463,7 @@ export default class RootController extends React.Component { tooltips={this.props.tooltips} config={this.props.config} commands={this.props.commandRegistry} - reportMutationErrors={this.reportMutationErrors} + reportRelayError={this.reportRelayError} /> )} @@ -925,13 +926,22 @@ export default class RootController extends React.Component { return await Promise.all(editorPromises); } - reportMutationErrors = (friendlyMessage, err) => { + reportRelayError = (friendlyMessage, err) => { const opts = {dismissable: true}; - if (err.errors) { + if (err.network) { + // Offline + opts.icon = 'alignment-unalign'; + opts.description = "It looks like you're offline right now."; + } else if (err.responseText) { + // Transient error like a 500 from the API + opts.description = 'The GitHub API reported a problem.'; + opts.detail = err.responseText; + } else if (err.errors) { + // GraphQL errors opts.detail = err.errors.map(e => e.message).join('\n'); - } else if (err.stack) { - opts.stack = err.stack; + } else { + opts.detail = err.stack; } this.props.notificationManager.addError(friendlyMessage, opts); diff --git a/lib/items/issueish-detail-item.js b/lib/items/issueish-detail-item.js index afb370b763..4c82681d32 100644 --- a/lib/items/issueish-detail-item.js +++ b/lib/items/issueish-detail-item.js @@ -42,7 +42,7 @@ export default class IssueishDetailItem extends Component { config: PropTypes.object.isRequired, // Action methods - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, } static defaultProps = { @@ -124,7 +124,7 @@ export default class IssueishDetailItem extends Component { destroy={this.destroy} itemType={this.constructor} refEditor={this.refEditor} - reportMutationErrors={this.props.reportMutationErrors} + reportRelayError={this.props.reportRelayError} /> ); } diff --git a/lib/items/reviews-item.js b/lib/items/reviews-item.js index 8edb7263c5..a69b7190c4 100644 --- a/lib/items/reviews-item.js +++ b/lib/items/reviews-item.js @@ -27,7 +27,7 @@ export default class ReviewsItem extends React.Component { tooltips: PropTypes.object.isRequired, // Action methods - reportMutationErrors: PropTypes.func.isRequired, + reportRelayError: PropTypes.func.isRequired, } static uriPattern = 'atom-github://reviews/{host}/{owner}/{repo}/{number}?workdir={workdir}' diff --git a/lib/models/github-login-model.js b/lib/models/github-login-model.js index ab95551915..d060a50bfb 100644 --- a/lib/models/github-login-model.js +++ b/lib/models/github-login-model.js @@ -80,9 +80,7 @@ export default class GithubLoginModel { this.checked.set(fingerprint, true); } catch (e) { // Most likely a network error. Do not cache the failure. - // eslint-disable-next-line no-console - console.error(`Unable to validate token scopes against ${account}`, e); - return UNAUTHENTICATED; + return e; } } } @@ -112,17 +110,26 @@ export default class GithubLoginModel { throw new Error('Attempt to check token scopes in specs'); } - const response = await fetch(host, { - method: 'HEAD', - headers: {Authorization: `bearer ${token}`}, - }); + let response; + try { + response = await fetch(host, { + method: 'HEAD', + headers: {Authorization: `bearer ${token}`}, + }); + } catch (e) { + e.network = true; + throw e; + } if (response.status === 401) { return UNAUTHORIZED; } if (response.status !== 200) { - throw new Error(`Unable to check token for OAuth scopes against ${host}: ${await response.text()}`); + const e = new Error(`Unable to check token for OAuth scopes against ${host}`); + e.response = response; + e.responseText = await response.text(); + throw e; } return response.headers.get('X-OAuth-Scopes').split(/\s*,\s*/); diff --git a/lib/models/user-store.js b/lib/models/user-store.js index ccea24f90f..9142ddbd64 100644 --- a/lib/models/user-store.js +++ b/lib/models/user-store.js @@ -124,7 +124,7 @@ export default class UserStore { return null; } const token = await loginModel.getToken(loginAccount); - if (token === UNAUTHENTICATED || token === INSUFFICIENT) { + if (token === UNAUTHENTICATED || token === INSUFFICIENT || token instanceof Error) { return null; } return token; @@ -176,6 +176,7 @@ export default class UserStore { after: cursor, }); + /* istanbul ignore if */ if (response.errors && response.errors.length > 1) { // eslint-disable-next-line no-console console.error(`Error fetching mentionable users:\n${response.errors.map(e => e.message).join('\n')}`); diff --git a/lib/relay-network-layer-manager.js b/lib/relay-network-layer-manager.js index e7035e904f..846875c862 100644 --- a/lib/relay-network-layer-manager.js +++ b/lib/relay-network-layer-manager.js @@ -98,18 +98,27 @@ function createFetchQuery(url) { return async function fetchQuery(operation, variables, _cacheConfig, _uploadables) { const currentToken = tokenPerURL.get(url); - const response = await fetch(url, { - method: 'POST', - headers: { - 'content-type': 'application/json', - 'Authorization': `bearer ${currentToken}`, - 'Accept': 'application/vnd.github.antiope-preview+json', - }, - body: JSON.stringify({ - query: operation.text, - variables, - }), - }); + let response; + try { + response = await fetch(url, { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'Authorization': `bearer ${currentToken}`, + 'Accept': 'application/vnd.github.antiope-preview+json', + }, + body: JSON.stringify({ + query: operation.text, + variables, + }), + }); + } catch (e) { + // A network error was encountered. Mark it so that QueryErrorView and ErrorView can distinguish these, because + // the errors from "fetch" are TypeErrors without much information. + e.network = true; + e.rawStack = e.stack; + throw e; + } try { atom && atom.inDevMode() && logRatelimitApi(response.headers); diff --git a/lib/views/error-view.js b/lib/views/error-view.js index 6d1228bdf6..94f8337214 100644 --- a/lib/views/error-view.js +++ b/lib/views/error-view.js @@ -1,8 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {autobind} from '../helpers'; - export default class ErrorView extends React.Component { static propTypes = { title: PropTypes.string, @@ -19,12 +17,6 @@ export default class ErrorView extends React.Component { preformatted: false, } - constructor(props) { - super(props); - - autobind(this, 'renderDescription'); - } - render() { return (
@@ -44,7 +36,7 @@ export default class ErrorView extends React.Component { ); } - renderDescription(description, key) { + renderDescription = (description, key) => { if (this.props.preformatted) { return ( 
diff --git a/lib/views/issue-detail-view.js b/lib/views/issue-detail-view.js
index 225e190644..e5ff95e1ad 100644
--- a/lib/views/issue-detail-view.js
+++ b/lib/views/issue-detail-view.js
@@ -55,7 +55,7 @@ export class BareIssueDetailView extends React.Component {
     tooltips: PropTypes.object.isRequired,
 
     // Action methods
-    reportMutationErrors: PropTypes.func.isRequired,
+    reportRelayError: PropTypes.func.isRequired,
   }
 
   state = {
@@ -87,7 +87,7 @@ export class BareIssueDetailView extends React.Component {
         
          {
+    }, null, err => {
+      if (err) {
+        this.props.reportRelayError('Unable to refresh issue details', err);
+      }
       this.setState({refreshing: false});
     }, {force: true});
   }
diff --git a/lib/views/offline-view.js b/lib/views/offline-view.js
new file mode 100644
index 0000000000..a5606449f4
--- /dev/null
+++ b/lib/views/offline-view.js
@@ -0,0 +1,35 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+import Octicon from '../atom/octicon';
+
+export default class OfflineView extends React.Component {
+  static propTypes = {
+    retry: PropTypes.func.isRequired,
+  }
+
+  componentDidMount() {
+    window.addEventListener('online', this.props.retry);
+  }
+
+  componentWillUnmount() {
+    window.removeEventListener('online', this.props.retry);
+  }
+
+  render() {
+    return (
+      

+        

+          
+          
Offline

+          

+            You don't seem to be connected to the Internet. When you're back online, we'll try again.
+          

+          

+            
+          

+        

+      

+    );
+  }
+}
diff --git a/lib/views/pr-detail-view.js b/lib/views/pr-detail-view.js
index eade642bab..f15681c2cc 100644
--- a/lib/views/pr-detail-view.js
+++ b/lib/views/pr-detail-view.js
@@ -85,7 +85,7 @@ export class BarePullRequestDetailView extends React.Component {
     openReviews: PropTypes.func.isRequired,
     switchToIssueish: PropTypes.func.isRequired,
     destroy: PropTypes.func.isRequired,
-    reportMutationErrors: PropTypes.func.isRequired,
+    reportRelayError: PropTypes.func.isRequired,
 
     // Item context
     itemType: ItemTypePropType.isRequired,
@@ -169,7 +169,7 @@ export class BarePullRequestDetailView extends React.Component {
             
              {
+    }, null, err => {
+      if (err) {
+        this.props.reportRelayError('Unable to refresh pull request details', err);
+      }
       this.setState({refreshing: false});
     }, {force: true});
   }
diff --git a/lib/views/query-error-tile.js b/lib/views/query-error-tile.js
index e946f9d85e..727f8eaa8a 100644
--- a/lib/views/query-error-tile.js
+++ b/lib/views/query-error-tile.js
@@ -10,6 +10,7 @@ export default class QueryErrorTile extends React.Component {
         status: PropTypes.number.isRequired,
       }),
       responseText: PropTypes.string,
+      network: PropTypes.bool,
       errors: PropTypes.arrayOf(PropTypes.shape({
         message: PropTypes.string.isRequired,
       })),
@@ -34,21 +35,25 @@ export default class QueryErrorTile extends React.Component {
   renderMessages() {
     if (this.props.error.errors) {
       return this.props.error.errors.map((error, index) => {
-        return this.renderMessage(error.message, index);
+        return this.renderMessage(error.message, index, 'alert');
       });
     }
 
     if (this.props.error.response) {
-      return this.renderMessage(this.props.error.responseText, '0');
+      return this.renderMessage(this.props.error.responseText, '0', 'alert');
     }
 
-    return this.renderMessage(this.props.error.toString(), '0');
+    if (this.props.error.network) {
+      return this.renderMessage('Offline', '0', 'alignment-unalign');
+    }
+
+    return this.renderMessage(this.props.error.toString(), '0', 'alert');
   }
 
-  renderMessage(body, key) {
+  renderMessage(body, key, icon) {
     return (
       

-        
+        
         {body}
       

     );
diff --git a/lib/views/query-error-view.js b/lib/views/query-error-view.js
index da0d38e67e..4bc94cbe56 100644
--- a/lib/views/query-error-view.js
+++ b/lib/views/query-error-view.js
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 
 import GithubLoginView from './github-login-view';
 import ErrorView from './error-view';
+import OfflineView from './offline-view';
 
 export default class QueryErrorView extends React.Component {
   static propTypes = {
@@ -40,6 +41,10 @@ export default class QueryErrorView extends React.Component {
       return this.renderGraphQLErrors(e.errors);
     }
 
+    if (e.network) {
+      return this.renderNetworkError();
+    }
+
     return (
       ;
+  }
+
   render401() {
     return (
       

diff --git a/lib/views/reviews-view.js b/lib/views/reviews-view.js
index 7668fba5de..576cdde62f 100644
--- a/lib/views/reviews-view.js
+++ b/lib/views/reviews-view.js
@@ -95,7 +95,7 @@ export default class ReviewsView extends React.Component {
     resolveThread: PropTypes.func.isRequired,
     unresolveThread: PropTypes.func.isRequired,
     addSingleComment: PropTypes.func.isRequired,
-    reportMutationErrors: PropTypes.func.isRequired,
+    reportRelayError: PropTypes.func.isRequired,
   }
 
   constructor(props) {
@@ -287,7 +287,7 @@ export default class ReviewsView extends React.Component {
           
         
       

@@ -581,7 +581,7 @@ export default class ReviewsView extends React.Component {
diff --git a/test/containers/aggregated-reviews-container.test.js b/test/containers/aggregated-reviews-container.test.js index 99f839d84e..278c342c13 100644 --- a/test/containers/aggregated-reviews-container.test.js +++ b/test/containers/aggregated-reviews-container.test.js @@ -18,6 +18,7 @@ describe('AggregatedReviewsContainer', function() { relay: { refetch: () => {}, }, + reportRelayError: () => {}, ...override, }; @@ -224,4 +225,55 @@ describe('AggregatedReviewsContainer', function() { assert.isTrue(cb0.called); assert.isTrue(cb1.called); }); + + it('reports an error encountered during refetch', function() { + const e = new Error('kerpow'); + const refetchStub = sinon.stub().callsFake((...args) => args[2](e)); + const reportRelayError = sinon.spy(); + + let refetchFn = null; + const children = ({refetch}) => { + refetchFn = refetch; + return
; + }; + + const wrapper = shallow(buildApp({ + children, + relay: {refetch: refetchStub}, + reportRelayError, + })); + + const summariesAccumulator = wrapper.find(ReviewSummariesAccumulator); + + const cb0 = sinon.spy(); + summariesAccumulator.prop('onDidRefetch')(cb0); + + const summariesWrapper = summariesAccumulator.renderProp('children')({ + error: null, + summaries: [], + loading: false, + }); + + const threadAccumulator = summariesWrapper.find(ReviewThreadsAccumulator); + + const cb1 = sinon.spy(); + threadAccumulator.prop('onDidRefetch')(cb1); + + const threadWrapper = threadAccumulator.renderProp('children')({ + errors: [], + commentThreads: [], + loading: false, + }); + + assert.isTrue(threadWrapper.exists('.done')); + assert.isNotNull(refetchFn); + + const done = sinon.spy(); + refetchFn(done); + + assert.isTrue(refetchStub.called); + assert.isTrue(reportRelayError.calledWith(sinon.match.string, e)); + assert.isFalse(cb0.called); + assert.isFalse(cb1.called); + }); }); diff --git a/test/containers/comment-decorations-container.test.js b/test/containers/comment-decorations-container.test.js index 90c664407a..539e1895bd 100644 --- a/test/containers/comment-decorations-container.test.js +++ b/test/containers/comment-decorations-container.test.js @@ -40,6 +40,7 @@ describe('CommentDecorationsContainer', function() { workspace={workspace} localRepository={localRepository} loginModel={loginModel} + reportRelayError={() => {}} {...overrideProps} /> ); diff --git a/test/containers/issueish-detail-container.test.js b/test/containers/issueish-detail-container.test.js index 3b1a33617b..417f65fc70 100644 --- a/test/containers/issueish-detail-container.test.js +++ b/test/containers/issueish-detail-container.test.js @@ -92,6 +92,19 @@ describe('IssueishDetailContainer', function() { assert.match(tokenWrapper.find('p').text(), /re-authenticate/); }); + it('renders an offline message if the user cannot connect to the Internet', function() { + sinon.spy(loginModel, 'didUpdate'); + + const wrapper = shallow(buildApp()); + const e = new Error('wat'); + const tokenWrapper = wrapper.find(ObserveModel).renderProp('children')({token: e}); + + assert.isTrue(tokenWrapper.exists('QueryErrorView')); + + tokenWrapper.find('QueryErrorView').prop('retry')(); + assert.isTrue(loginModel.didUpdate.called); + }); + it('passes the token to the login model on login', async function() { sinon.stub(loginModel, 'setToken').resolves(); @@ -104,6 +117,15 @@ describe('IssueishDetailContainer', function() { assert.isTrue(loginModel.setToken.calledWith('https://github.enterprise.horse', '4321')); }); + it('passes an existing token from the login model', async function() { + sinon.stub(loginModel, 'getToken').resolves('1234'); + + const wrapper = shallow(buildApp({ + endpoint: getEndpoint('github.enterprise.horse'), + })); + assert.deepEqual(await wrapper.find(ObserveModel).prop('fetchData')(loginModel), {token: '1234'}); + }); + it('renders a spinner while repository data is being fetched', function() { const wrapper = shallow(buildApp()); const tokenWrapper = wrapper.find(ObserveModel).renderProp('children')({token: '1234'}); diff --git a/test/containers/remote-container.test.js b/test/containers/remote-container.test.js index af61e71485..beda1e34f2 100644 --- a/test/containers/remote-container.test.js +++ b/test/containers/remote-container.test.js @@ -96,6 +96,18 @@ describe('RemoteContainer', function() { assert.match(tokenWrapper.find('GithubLoginView').find('p').text(), /sufficient/); }); + it('renders an offline view if the user is offline', function() { + sinon.spy(model, 'didUpdate'); + + const wrapper = shallow(buildApp()); + const e = new Error('oh no'); + const tokenWrapper = wrapper.find('ObserveModel').renderProp('children')(e); + assert.isTrue(tokenWrapper.exists('QueryErrorView')); + + tokenWrapper.find('QueryErrorView').prop('retry')(); + assert.isTrue(model.didUpdate.called); + }); + it('renders an error message if the GraphQL query fails', function() { const wrapper = shallow(buildApp()); const tokenWrapper = wrapper.find('ObserveModel').renderProp('children')('1234'); diff --git a/test/containers/reviews-container.test.js b/test/containers/reviews-container.test.js index 25c74488a5..f7ec0d53d8 100644 --- a/test/containers/reviews-container.test.js +++ b/test/containers/reviews-container.test.js @@ -65,7 +65,7 @@ describe('ReviewsContainer', function() { config: atomEnv.config, commands: atomEnv.commands, tooltips: atomEnv.tooltips, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...override, }; @@ -93,6 +93,18 @@ describe('ReviewsContainer', function() { assert.match(tokenWrapper.find('GithubLoginView > p').text(), /re-authenticate/); }); + it('shows a message if the network is unavailable', function() { + sinon.spy(loginModel, 'didUpdate'); + + const wrapper = shallow(buildApp()); + const e = new Error('er'); + const tokenWrapper = wrapper.find('ObserveModel').renderProp('children')(e); + + assert.isTrue(tokenWrapper.exists('QueryErrorView')); + tokenWrapper.find('QueryErrorView').prop('retry')(); + assert.isTrue(loginModel.didUpdate.called); + }); + it('gets the token from the login model', async function() { await loginModel.setToken('https://github.enterprise.horse', 'neigh'); @@ -135,6 +147,32 @@ describe('ReviewsContainer', function() { assert.isTrue(resultWrapper.exists('LoadingView')); }); + it('shows an error view if the review comment aggregation fails', function() { + const {multiFilePatch} = multiFilePatchBuilder().build(); + + const wrapper = shallow(buildApp()); + const tokenWrapper = wrapper.find('ObserveModel').renderProp('children')('shhh'); + const patchWrapper = tokenWrapper.find('PullRequestPatchContainer').renderProp('children')(null, multiFilePatch); + + const repoWrapper = patchWrapper.find('ObserveModel').renderProp('children')(repoData); + const relayWrapper = repoWrapper.find(QueryRenderer).renderProp('render')({ + error: null, + props: queryData, + retry: () => {}, + }); + + const e0 = new Error('oh shit'); + const e1 = new Error('not again'); + const reviewsWrapper = relayWrapper.find(AggregatedReviewsContainer).renderProp('children')({ + errors: [e0, e1], + summaries: [], + commentThreads: [], + refetch: () => {}, + }); + + assert.deepEqual(reviewsWrapper.find('ErrorView').map(w => w.prop('descriptions')), [[e0.stack], [e1.stack]]); + }); + it('passes the patch to the controller', function() { const wrapper = shallow(buildApp({ owner: 'secret', diff --git a/test/controllers/emoji-reactions-controller.test.js b/test/controllers/emoji-reactions-controller.test.js index 232e9769fc..a5d4ff95ff 100644 --- a/test/controllers/emoji-reactions-controller.test.js +++ b/test/controllers/emoji-reactions-controller.test.js @@ -32,7 +32,7 @@ describe('EmojiReactionsController', function() { }, reactable: issueBuilder(reactableQuery).build(), tooltips: atomEnv.tooltips, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...override, }; @@ -48,7 +48,7 @@ describe('EmojiReactionsController', function() { describe('adding a reaction', function() { it('fires the add reaction mutation', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addReactionQuery.operation.name, @@ -64,15 +64,15 @@ describe('EmojiReactionsController', function() { const reactable = issueBuilder(reactableQuery).id('issue0').build(); relayEnv.getStore().getSource().set('issue0', {...createRecord('issue0', 'Issue'), ...reactable}); - const wrapper = shallow(buildApp({reactable, reportMutationErrors})); + const wrapper = shallow(buildApp({reactable, reportRelayError})); await wrapper.find(EmojiReactionsView).prop('addReaction')('ROCKET'); - assert.isFalse(reportMutationErrors.called); + assert.isFalse(reportRelayError.called); }); it('reports errors encountered', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addReactionQuery.operation.name, @@ -86,11 +86,11 @@ describe('EmojiReactionsController', function() { const reactable = issueBuilder(reactableQuery).id('issue1').build(); relayEnv.getStore().getSource().set('issue1', {...createRecord('issue1', 'Issue'), ...reactable}); - const wrapper = shallow(buildApp({reactable, reportMutationErrors})); + const wrapper = shallow(buildApp({reactable, reportRelayError})); await wrapper.find(EmojiReactionsView).prop('addReaction')('EYES'); - assert.isTrue(reportMutationErrors.calledWith( + assert.isTrue(reportRelayError.calledWith( 'Unable to add reaction emoji', sinon.match({errors: [{message: 'oh no'}]})), ); @@ -99,7 +99,7 @@ describe('EmojiReactionsController', function() { describe('removing a reaction', function() { it('fires the remove reaction mutation', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: removeReactionQuery.operation.name, @@ -115,15 +115,15 @@ describe('EmojiReactionsController', function() { const reactable = issueBuilder(reactableQuery).id('issue0').build(); relayEnv.getStore().getSource().set('issue0', {...createRecord('issue0', 'Issue'), ...reactable}); - const wrapper = shallow(buildApp({reactable, reportMutationErrors})); + const wrapper = shallow(buildApp({reactable, reportRelayError})); await wrapper.find(EmojiReactionsView).prop('removeReaction')('THUMBS_DOWN'); - assert.isFalse(reportMutationErrors.called); + assert.isFalse(reportRelayError.called); }); it('reports errors encountered', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: removeReactionQuery.operation.name, @@ -137,11 +137,11 @@ describe('EmojiReactionsController', function() { const reactable = issueBuilder(reactableQuery).id('issue1').build(); relayEnv.getStore().getSource().set('issue1', {...createRecord('issue1', 'Issue'), ...reactable}); - const wrapper = shallow(buildApp({reactable, reportMutationErrors})); + const wrapper = shallow(buildApp({reactable, reportRelayError})); await wrapper.find(EmojiReactionsView).prop('removeReaction')('CONFUSED'); - assert.isTrue(reportMutationErrors.calledWith( + assert.isTrue(reportRelayError.calledWith( 'Unable to remove reaction emoji', sinon.match({errors: [{message: 'wtf'}]})), ); diff --git a/test/controllers/issueish-detail-controller.test.js b/test/controllers/issueish-detail-controller.test.js index f74f9777d7..d22dc17f75 100644 --- a/test/controllers/issueish-detail-controller.test.js +++ b/test/controllers/issueish-detail-controller.test.js @@ -64,7 +64,7 @@ describe('IssueishDetailController', function() { onTitleChange: () => {}, switchToIssueish: () => {}, destroy: () => {}, - reportMutationErrors: () => {}, + reportRelayError: () => {}, itemType: IssueishDetailItem, refEditor: new RefHolder(), diff --git a/test/controllers/reviews-controller.test.js b/test/controllers/reviews-controller.test.js index 471e6ec3aa..e9c3b3c6a9 100644 --- a/test/controllers/reviews-controller.test.js +++ b/test/controllers/reviews-controller.test.js @@ -79,7 +79,7 @@ describe('ReviewsController', function() { config: atomEnv.config, commands: atomEnv.commands, tooltips: atomEnv.tooltips, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...override, }; @@ -300,7 +300,7 @@ describe('ReviewsController', function() { }); it('creates a notification when the review cannot be created', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addPrReviewMutation.operation.name, @@ -314,7 +314,7 @@ describe('ReviewsController', function() { }).resolve(); const pullRequest = pullRequestBuilder(pullRequestQuery).id('pr0').build(); - const wrapper = shallow(buildApp({pullRequest, reportMutationErrors})) + const wrapper = shallow(buildApp({pullRequest, reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); @@ -325,13 +325,13 @@ describe('ReviewsController', function() { 'body', 'thread0', 'comment1', 'file1.txt', 20, {didSubmitComment, didFailComment}, ); - assert.isTrue(reportMutationErrors.calledWith('Unable to submit your comment')); + assert.isTrue(reportRelayError.calledWith('Unable to submit your comment')); assert.isFalse(didSubmitComment.called); assert.isTrue(didFailComment.called); }); it('creates a notification and deletes the review when the comment cannot be added', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addPrReviewMutation.operation.name, @@ -368,7 +368,7 @@ describe('ReviewsController', function() { }).resolve(); const pullRequest = pullRequestBuilder(pullRequestQuery).id('pr0').build(); - const wrapper = shallow(buildApp({pullRequest, reportMutationErrors})) + const wrapper = shallow(buildApp({pullRequest, reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); @@ -379,13 +379,13 @@ describe('ReviewsController', function() { 'body', 'thread0', 'comment1', 'file2.txt', 5, {didSubmitComment, didFailComment}, ); - assert.isTrue(reportMutationErrors.calledWith('Unable to submit your comment')); + assert.isTrue(reportRelayError.calledWith('Unable to submit your comment')); assert.isTrue(didSubmitComment.called); assert.isTrue(didFailComment.called); }); it('includes errors from the review deletion', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addPrReviewMutation.operation.name, @@ -424,7 +424,7 @@ describe('ReviewsController', function() { }).resolve(); const pullRequest = pullRequestBuilder(pullRequestQuery).id('pr0').build(); - const wrapper = shallow(buildApp({pullRequest, reportMutationErrors})) + const wrapper = shallow(buildApp({pullRequest, reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); @@ -434,7 +434,7 @@ describe('ReviewsController', function() { }); it('creates a notification when the review cannot be submitted', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: addPrReviewMutation.operation.name, @@ -474,7 +474,7 @@ describe('ReviewsController', function() { }).resolve(); const pullRequest = pullRequestBuilder(pullRequestQuery).id('pr0').build(); - const wrapper = shallow(buildApp({pullRequest, reportMutationErrors})) + const wrapper = shallow(buildApp({pullRequest, reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); @@ -485,7 +485,7 @@ describe('ReviewsController', function() { 'body', 'thread0', 'comment1', 'file1.txt', 10, {didSubmitComment, didFailComment}, ); - assert.isTrue(reportMutationErrors.calledWith('Unable to submit your comment')); + assert.isTrue(reportRelayError.calledWith('Unable to submit your comment')); assert.isTrue(didSubmitComment.called); assert.isTrue(didFailComment.called); }); @@ -493,7 +493,7 @@ describe('ReviewsController', function() { describe('resolving threads', function() { it('hides the thread, then fires the mutation', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: resolveThreadMutation.operation.name, @@ -502,7 +502,7 @@ describe('ReviewsController', function() { }, }, op => relayResponseBuilder(op).build()).resolve(); - const wrapper = shallow(buildApp({reportMutationErrors})) + const wrapper = shallow(buildApp({reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); await wrapper.find(ReviewsView).prop('showThreadID')('thread0'); @@ -512,13 +512,13 @@ describe('ReviewsController', function() { await wrapper.find(ReviewsView).prop('resolveThread')({id: 'thread0', viewerCanResolve: true}); assert.isFalse(wrapper.find(ReviewsView).prop('threadIDsOpen').has('thread0')); - assert.isFalse(reportMutationErrors.called); + assert.isFalse(reportRelayError.called); }); it('is a no-op if the viewer cannot resolve the thread', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); - const wrapper = shallow(buildApp({reportMutationErrors})) + const wrapper = shallow(buildApp({reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); await wrapper.find(ReviewsView).prop('showThreadID')('thread0'); @@ -526,11 +526,11 @@ describe('ReviewsController', function() { await wrapper.find(ReviewsView).prop('resolveThread')({id: 'thread0', viewerCanResolve: false}); assert.isTrue(wrapper.find(ReviewsView).prop('threadIDsOpen').has('thread0')); - assert.isFalse(reportMutationErrors.called); + assert.isFalse(reportRelayError.called); }); it('re-shows the thread and creates a notification when the thread cannot be resolved', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: resolveThreadMutation.operation.name, @@ -539,7 +539,7 @@ describe('ReviewsController', function() { }, }, op => relayResponseBuilder(op).addError('boom').build()).resolve(); - const wrapper = shallow(buildApp({reportMutationErrors})) + const wrapper = shallow(buildApp({reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); await wrapper.find(ReviewsView).prop('showThreadID')('thread0'); @@ -547,7 +547,7 @@ describe('ReviewsController', function() { await wrapper.find(ReviewsView).prop('resolveThread')({id: 'thread0', viewerCanResolve: true}); assert.isTrue(wrapper.find(ReviewsView).prop('threadIDsOpen').has('thread0')); - assert.isTrue(reportMutationErrors.calledWith('Unable to resolve the comment thread')); + assert.isTrue(reportRelayError.calledWith('Unable to resolve the comment thread')); }); }); @@ -572,19 +572,19 @@ describe('ReviewsController', function() { }); it("is a no-op if the viewer can't unresolve the thread", async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); - const wrapper = shallow(buildApp({reportMutationErrors})) + const wrapper = shallow(buildApp({reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); await wrapper.find(ReviewsView).prop('unresolveThread')({id: 'thread1', viewerCanUnresolve: false}); - assert.isFalse(reportMutationErrors.called); + assert.isFalse(reportRelayError.called); }); it('creates a notification if the thread cannot be unresolved', async function() { - const reportMutationErrors = sinon.spy(); + const reportRelayError = sinon.spy(); expectRelayQuery({ name: unresolveThreadMutation.operation.name, @@ -593,13 +593,13 @@ describe('ReviewsController', function() { }, }, op => relayResponseBuilder(op).addError('ow').build()).resolve(); - const wrapper = shallow(buildApp({reportMutationErrors})) + const wrapper = shallow(buildApp({reportRelayError})) .find(PullRequestCheckoutController) .renderProp('children')(noop); await wrapper.find(ReviewsView).prop('unresolveThread')({id: 'thread1', viewerCanUnresolve: true}); - assert.isTrue(reportMutationErrors.calledWith('Unable to unresolve the comment thread')); + assert.isTrue(reportRelayError.calledWith('Unable to unresolve the comment thread')); }); }); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index 36a2912766..91295710de 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -1372,4 +1372,64 @@ describe('RootController', function() { }); }); + describe('reportRelayError', function() { + let instance; + + beforeEach(function() { + instance = shallow(app).instance(); + sinon.stub(notificationManager, 'addError'); + }); + + it('creates a notification for a network error', function() { + const error = new Error('cat tripped over the ethernet cable'); + error.network = true; + + instance.reportRelayError('friendly message', error); + + assert.isTrue(notificationManager.addError.calledWith('friendly message', { + dismissable: true, + icon: 'alignment-unalign', + description: "It looks like you're offline right now.", + })); + }); + + it('creates a notification for an API HTTP error', function() { + const error = new Error('GitHub is down'); + error.responseText = "I just don't feel like it"; + + instance.reportRelayError('friendly message', error); + + assert.isTrue(notificationManager.addError.calledWith('friendly message', { + dismissable: true, + description: 'The GitHub API reported a problem.', + detail: "I just don't feel like it", + })); + }); + + it('creates a notification for GraphQL errors', function() { + const error = new Error("Your query wasn't good enough"); + error.errors = [ + {message: 'First of all'}, + {message: 'and another thing'}, + ]; + + instance.reportRelayError('friendly message', error); + + assert.isTrue(notificationManager.addError.calledWith('friendly message', { + dismissable: true, + detail: 'First of all\nand another thing', + })); + }); + + it('falls back to a stack-trace error', function() { + const error = new Error('idk'); + + instance.reportRelayError('friendly message', error); + + assert.isTrue(notificationManager.addError.calledWith('friendly message', { + dismissable: true, + detail: error.stack, + })); + }); + }); }); diff --git a/test/fixtures/props/issueish-pane-props.js b/test/fixtures/props/issueish-pane-props.js index 13137a7bcc..1a48bb509c 100644 --- a/test/fixtures/props/issueish-pane-props.js +++ b/test/fixtures/props/issueish-pane-props.js @@ -272,7 +272,7 @@ export function issueDetailViewProps(opts, overrides = {}) { }, switchToIssueish: () => {}, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...overrides, }; diff --git a/test/integration/file-patch.test.js b/test/integration/file-patch.test.js index b000e59790..d2d3a0d9e1 100644 --- a/test/integration/file-patch.test.js +++ b/test/integration/file-patch.test.js @@ -612,13 +612,7 @@ describe('integration: file patches', function() { triggerChange(); }); - describe('unstaged', function() { - before(function() { - if (process.platform === 'win32') { - this.skip(); - } - }); - + describe.skip('unstaged', function() { beforeEach(async function() { await clickFileInGitTab('unstaged', 'symlink.txt'); }); diff --git a/test/items/issueish-detail-item.test.js b/test/items/issueish-detail-item.test.js index a9857a8f11..781beb0f9b 100644 --- a/test/items/issueish-detail-item.test.js +++ b/test/items/issueish-detail-item.test.js @@ -36,7 +36,7 @@ describe('IssueishDetailItem', function() { tooltips: atomEnv.tooltips, config: atomEnv.config, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...override, }; return ( diff --git a/test/items/reviews-item.test.js b/test/items/reviews-item.test.js index d374c183e2..a6eb6d185b 100644 --- a/test/items/reviews-item.test.js +++ b/test/items/reviews-item.test.js @@ -36,7 +36,7 @@ describe('ReviewsItem', function() { config: atomEnv.config, commands: atomEnv.commands, tooltips: atomEnv.tooltips, - reportMutationErrors: () => {}, + reportRelayError: () => {}, ...override, }; diff --git a/test/models/github-login-model.test.js b/test/models/github-login-model.test.js index e4d2f754be..83fd6ed7e5 100644 --- a/test/models/github-login-model.test.js +++ b/test/models/github-login-model.test.js @@ -89,13 +89,20 @@ describe('GithubLoginModel', function() { assert.strictEqual(loginModel.getScopes.callCount, 1); }); + it('detects and reports network errors', async function() { + const e = new Error('You unplugged your ethernet cable'); + sinon.stub(loginModel, 'getScopes').rejects(e); + assert.strictEqual(await loginModel.getToken('https://api.github.com'), e); + }); + it('does not cache network errors', async function() { - sinon.stub(loginModel, 'getScopes').rejects(new Error('You unplugged your ethernet cable')); + const e = new Error('You unplugged your ethernet cable'); + sinon.stub(loginModel, 'getScopes').rejects(e); - assert.strictEqual(await loginModel.getToken('https://api.github.com'), UNAUTHENTICATED); + assert.strictEqual(await loginModel.getToken('https://api.github.com'), e); assert.strictEqual(loginModel.getScopes.callCount, 1); - assert.strictEqual(await loginModel.getToken('https://api.github.com'), UNAUTHENTICATED); + assert.strictEqual(await loginModel.getToken('https://api.github.com'), e); assert.strictEqual(loginModel.getScopes.callCount, 2); }); }); diff --git a/test/models/user-store.test.js b/test/models/user-store.test.js index 0092288e3d..3f35804de8 100644 --- a/test/models/user-store.test.js +++ b/test/models/user-store.test.js @@ -433,6 +433,16 @@ describe('UserStore', function() { assert.isNull(getToken); }); + it('returns null if network is offline', async function() { + const loginModel = new GithubLoginModel(InMemoryStrategy); + const e = new Error('eh'); + sinon.stub(loginModel, 'getToken').returns(Promise.resolve(e)); + + store = new UserStore({repository, loginModel, config}); + const getToken = await store.getToken(loginModel, 'https://api.github.com'); + assert.isNull(getToken); + }); + it('return token if token is sufficient and model is truthy', async function() { const loginModel = new GithubLoginModel(InMemoryStrategy); sinon.stub(loginModel, 'getScopes').returns(Promise.resolve(['repo', 'read:org', 'user:email'])); diff --git a/test/views/error-view.test.js b/test/views/error-view.test.js index f36a1c93a6..45bcaa81f2 100644 --- a/test/views/error-view.test.js +++ b/test/views/error-view.test.js @@ -24,13 +24,24 @@ describe('ErrorView', function() { it('renders descriptions', function() { const wrapper = shallow(buildApp()); - const descriptions = wrapper.find('.github-Message-description'); + const descriptions = wrapper.find('p.github-Message-description'); assert.lengthOf(descriptions, 3); assert.strictEqual(descriptions.at(0).text(), 'something'); assert.strictEqual(descriptions.at(1).text(), 'went'); assert.strictEqual(descriptions.at(2).text(), 'wrong'); }); + it('renders preformatted descriptions', function() { + const wrapper = shallow(buildApp({ + preformatted: true, + descriptions: ['abc\ndef', 'ghi\njkl'], + })); + const descriptions = wrapper.find('pre.github-Message-description'); + assert.lengthOf(descriptions, 2); + assert.strictEqual(descriptions.at(0).text(), 'abc\ndef'); + assert.strictEqual(descriptions.at(1).text(), 'ghi\njkl'); + }); + it('renders retry button that if retry prop is passed', function() { const retrySpy = sinon.spy(); const wrapper = shallow(buildApp({retry: retrySpy})); diff --git a/test/views/issue-detail-view.test.js b/test/views/issue-detail-view.test.js index c4c872edb9..f211660cf2 100644 --- a/test/views/issue-detail-view.test.js +++ b/test/views/issue-detail-view.test.js @@ -83,6 +83,24 @@ describe('IssueDetailView', function() { assert.isFalse(wrapper.find('Octicon[icon="repo-sync"]').hasClass('refreshing')); }); + it('reports errors encountered during refresh', function() { + let callback = null; + const relayRefetch = sinon.stub().callsFake((_0, _1, cb) => { + callback = cb; + }); + const reportRelayError = sinon.spy(); + const wrapper = shallow(buildApp({relayRefetch}, {reportRelayError})); + + wrapper.find('Octicon[icon="repo-sync"]').simulate('click', {preventDefault: () => {}}); + + const e = new Error('ouch'); + callback(e); + assert.isTrue(reportRelayError.calledWith(sinon.match.string, e)); + + wrapper.update(); + assert.isFalse(wrapper.find('Octicon[icon="repo-sync"]').hasClass('refreshing')); + }); + it('disregardes a double refresh', function() { let callback = null; const relayRefetch = sinon.stub().callsFake((_0, _1, cb) => { diff --git a/test/views/offline-view-test.js b/test/views/offline-view-test.js new file mode 100644 index 0000000000..a850aed460 --- /dev/null +++ b/test/views/offline-view-test.js @@ -0,0 +1,46 @@ +import React from 'react'; +import {shallow} from 'enzyme'; + +import OfflineView from '../../lib/views/offline-view'; + +describe('OfflineView', function() { + let listeners; + + beforeEach(function() { + listeners = {}; + + sinon.stub(window, 'addEventListener').callsFake((eventName, callback) => { + listeners[eventName] = callback; + }); + sinon.stub(window, 'removeEventListener').callsFake((eventName, callback) => { + if (listeners[eventName] === callback) { + delete listeners[eventName]; + } else { + throw new Error('Wrong callback'); + } + }); + }); + + it('triggers a retry callback when the retry button is clicked', function() { + const retry = sinon.spy(); + const wrapper = shallow(); + + wrapper.find('.btn').simulate('click'); + assert.isTrue(retry.called); + }); + + it('triggers a retry callback when the network status changes', function() { + const retry = sinon.spy(); + shallow(); + + listeners.online(); + assert.strictEqual(retry.callCount, 1); + }); + + it('unregisters the network status listener on unmount', function() { + const wrapper = shallow( {}} />); + assert.isDefined(listeners.online); + wrapper.unmount(); + assert.isUndefined(listeners.online); + }); +}); diff --git a/test/views/pr-detail-view.test.js b/test/views/pr-detail-view.test.js index d2c8a0ade8..c25fcb3606 100644 --- a/test/views/pr-detail-view.test.js +++ b/test/views/pr-detail-view.test.js @@ -60,7 +60,7 @@ describe('PullRequestDetailView', function() { openReviews: () => {}, switchToIssueish: () => {}, destroy: () => {}, - reportMutationErrors: () => {}, + reportRelayError: () => {}, itemType: IssueishDetailItem, refEditor: new RefHolder(), @@ -288,6 +288,25 @@ describe('PullRequestDetailView', function() { assert.isFalse(wrapper.find('Octicon[icon="repo-sync"]').hasClass('refreshing')); }); + it('reports errors encountered during refetch', function() { + let callback = null; + const refetch = sinon.stub().callsFake((_0, _1, cb) => { + callback = cb; + }); + const reportRelayError = sinon.spy(); + const wrapper = shallow(buildApp({relay: {refetch}, reportRelayError})); + + wrapper.find('Octicon[icon="repo-sync"]').simulate('click', {preventDefault: () => {}}); + assert.isTrue(wrapper.find('Octicon[icon="repo-sync"]').hasClass('refreshing')); + + const e = new Error('nope'); + callback(e); + assert.isTrue(reportRelayError.calledWith(sinon.match.string, e)); + + wrapper.update(); + assert.isFalse(wrapper.find('Octicon[icon="repo-sync"]').hasClass('refreshing')); + }); + it('disregards a double refresh', function() { let callback = null; const refetch = sinon.stub().callsFake((_0, _1, cb) => { diff --git a/test/views/query-error-tile.test.js b/test/views/query-error-tile.test.js new file mode 100644 index 0000000000..f38130bc17 --- /dev/null +++ b/test/views/query-error-tile.test.js @@ -0,0 +1,72 @@ +import React from 'react'; +import {shallow} from 'enzyme'; + +import QueryErrorTile from '../../lib/views/query-error-tile'; + +describe('QueryErrorTile', function() { + beforeEach(function() { + sinon.stub(console, 'error'); + }); + + it('logs the full error information to the console', function() { + const e = new Error('wat'); + e.rawStack = e.stack; + + shallow(); + + // eslint-disable-next-line no-console + assert.isTrue(console.error.calledWith(sinon.match.string, e)); + }); + + it('displays each GraphQL error', function() { + const e = new Error('wat'); + e.rawStack = e.stack; + e.errors = [ + {message: 'okay first of all'}, + {message: 'and another thing'}, + ]; + + const wrapper = shallow(); + const messages = wrapper.find('.github-QueryErrorTile-message'); + assert.lengthOf(messages, 2); + + assert.strictEqual(messages.at(0).find('Octicon').prop('icon'), 'alert'); + assert.match(messages.at(0).text(), /okay first of all/); + + assert.strictEqual(messages.at(1).find('Octicon').prop('icon'), 'alert'); + assert.match(messages.at(1).text(), /and another thing/); + }); + + it('displays the text of a failed HTTP response', function() { + const e = new Error('500'); + e.rawStack = e.stack; + e.response = {}; + e.responseText = 'The server is on fire'; + + const wrapper = shallow(); + const message = wrapper.find('.github-QueryErrorTile-message'); + assert.strictEqual(message.find('Octicon').prop('icon'), 'alert'); + assert.match(message.text(), /The server is on fire$/); + }); + + it('displays an offline message', function() { + const e = new Error('cat pulled out the ethernet cable'); + e.rawStack = e.stack; + e.network = true; + + const wrapper = shallow(); + const message = wrapper.find('.github-QueryErrorTile-message'); + assert.strictEqual(message.find('Octicon').prop('icon'), 'alignment-unalign'); + assert.match(message.text(), /Offline/); + }); + + it('falls back to displaying the raw error message', function() { + const e = new TypeError('oh no'); + e.rawStack = e.stack; + + const wrapper = shallow(); + const message = wrapper.find('.github-QueryErrorTile-message'); + assert.strictEqual(message.find('Octicon').prop('icon'), 'alert'); + assert.match(message.text(), /TypeError: oh no/); + }); +}); diff --git a/test/views/query-error-view.test.js b/test/views/query-error-view.test.js index a78c907694..cea04a0666 100644 --- a/test/views/query-error-view.test.js +++ b/test/views/query-error-view.test.js @@ -9,6 +9,7 @@ describe('QueryErrorView', function() { {}} + openDevTools={() => {}} {...overrideProps} /> ); @@ -41,6 +42,18 @@ describe('QueryErrorView', function() { })); }); + it('recognizes network errors', function() { + const error = new Error('network error'); + error.network = true; + error.rawStack = error.stack; + const retry = sinon.spy(); + + const wrapper = shallow(buildApp({error, retry})); + const ev = wrapper.find('OfflineView'); + ev.prop('retry')(); + assert.isTrue(retry.called); + }); + it('renders the error response directly for an unrecognized error status', function() { const error = new Error('GraphQL error'); error.response = { @@ -55,4 +68,15 @@ describe('QueryErrorView', function() { return n.prop('descriptions').includes('response text') && n.prop('preformatted'); })); }); + + it('falls back to rendering the message and stack', function() { + const error = new Error('the message'); + error.rawStack = error.stack; + + const wrapper = shallow(buildApp({error})); + const ev = wrapper.find('ErrorView'); + assert.strictEqual(ev.prop('title'), 'the message'); + assert.deepEqual(ev.prop('descriptions'), [error.stack]); + assert.isTrue(ev.prop('preformatted')); + }); }); diff --git a/test/views/reviews-view.test.js b/test/views/reviews-view.test.js index efacc4a504..dfa53c7da1 100644 --- a/test/views/reviews-view.test.js +++ b/test/views/reviews-view.test.js @@ -63,7 +63,7 @@ describe('ReviewsView', function() { resolveThread: () => {}, unresolveThread: () => {}, addSingleComment: () => {}, - reportMutationErrors: () => {}, + reportRelayError: () => {}, refetch: () => {}, ...override, };