Skip to content

Commit 0fc2486

Browse files
chrisbobbegnprice
authored andcommitted
login: Equalize user-facing errors across both api.getUserSettings calls
Also make them a bit more descriptive, de-duplicate the logic for what text to show the user, and add some TODOs for further improvements. src/message/fetchAction.js works OK as a place for this new function, though not perfectly.
1 parent ad0e065 commit 0fc2486

File tree

4 files changed

+108
-28
lines changed

4 files changed

+108
-28
lines changed

src/account/AccountPickScreen.js

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,19 @@ import React, { useContext, useCallback } from 'react';
44
import type { Node } from 'react';
55
import invariant from 'invariant';
66

7-
import * as api from '../api';
7+
import { fetchServerSettings } from '../message/fetchActions';
88
import { TranslationContext } from '../boot/TranslationProvider';
99
import type { RouteProp } from '../react-navigation';
1010
import type { AppNavigationProp } from '../nav/AppNavigator';
1111
import { useGlobalSelector, useGlobalDispatch } from '../react-redux';
12-
import { getAccountStatuses, getAccountsByIdentity } from '../selectors';
12+
import { getAccountStatuses, getAccountsByIdentity, getGlobalSettings } from '../selectors';
1313
import Centerer from '../common/Centerer';
1414
import ZulipButton from '../common/ZulipButton';
1515
import Logo from '../common/Logo';
1616
import Screen from '../common/Screen';
1717
import ViewPlaceholder from '../common/ViewPlaceholder';
1818
import AccountList from './AccountList';
1919
import { accountSwitch, removeAccount } from '../actions';
20-
import type { ServerSettings } from '../api/settings/getServerSettings';
2120
import { showConfirmationDialog, showErrorAlert } from '../utils/info';
2221
import { tryStopNotifications } from '../notification/notifTokens';
2322

@@ -34,6 +33,8 @@ export default function AccountPickScreen(props: Props): Node {
3433
// doing so, of course).
3534
const accountsByIdentity = useGlobalSelector(getAccountsByIdentity);
3635

36+
const globalSettings = useGlobalSelector(getGlobalSettings);
37+
3738
const dispatch = useGlobalDispatch();
3839
const _ = useContext(TranslationContext);
3940

@@ -45,16 +46,25 @@ export default function AccountPickScreen(props: Props): Node {
4546
dispatch(accountSwitch(index));
4647
});
4748
} else {
48-
try {
49-
const serverSettings: ServerSettings = await api.getServerSettings(realm);
50-
navigation.push('auth', { serverSettings });
51-
} catch {
52-
// TODO: show specific error message from error object
53-
showErrorAlert(_('Failed to connect to server: {realm}', { realm: realm.toString() }));
49+
const result = await fetchServerSettings(realm);
50+
if (result.type === 'error') {
51+
showErrorAlert(
52+
_(result.title),
53+
_(result.message),
54+
result.learnMoreButton && {
55+
url: result.learnMoreButton.url,
56+
text:
57+
result.learnMoreButton.text != null ? _(result.learnMoreButton.text) : undefined,
58+
globalSettings,
59+
},
60+
);
61+
return;
5462
}
63+
const serverSettings = result.value;
64+
navigation.push('auth', { serverSettings });
5565
}
5666
},
57-
[accountStatuses, dispatch, navigation, _],
67+
[accountStatuses, globalSettings, dispatch, navigation, _],
5868
);
5969

6070
const handleAccountRemove = useCallback(

src/message/fetchActions.js

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ import type {
99
Stream,
1010
ThunkAction,
1111
UserId,
12+
LocalizableText,
1213
} from '../types';
1314
import * as api from '../api';
14-
import { Server5xxError, NetworkError } from '../api/apiErrors';
15+
import { Server5xxError, NetworkError, ApiError, MalformedResponseError } from '../api/apiErrors';
1516
import {
1617
getAuth,
1718
getRealm,
@@ -34,6 +35,7 @@ import { ALL_PRIVATE_NARROW, apiNarrowOfNarrow, caseNarrow, topicNarrow } from '
3435
import { BackoffMachine, promiseTimeout, TimeoutError } from '../utils/async';
3536
import { addToOutbox } from '../outbox/outboxActions';
3637
import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
38+
import type { ServerSettings } from '../api/settings/getServerSettings';
3739

3840
const messageFetchStart = (
3941
narrow: Narrow,
@@ -397,3 +399,60 @@ export const uploadFile =
397399

398400
dispatch(addToOutbox(destinationNarrow, messageToSend));
399401
};
402+
403+
export async function fetchServerSettings(realm: URL): Promise<
404+
| {| +type: 'success', +value: ServerSettings |}
405+
| {|
406+
+type: 'error',
407+
+title: LocalizableText,
408+
+message: LocalizableText,
409+
+learnMoreButton: {| +url: URL, +text?: LocalizableText |} | void,
410+
|},
411+
> {
412+
try {
413+
return { type: 'success', value: await api.getServerSettings(realm) };
414+
// TODO(#5102): Disallow connecting to ancient servers
415+
} catch (errorIllTyped) {
416+
const error: mixed = errorIllTyped;
417+
418+
const title = 'Could not connect';
419+
let message = undefined;
420+
let learnMoreButton = undefined;
421+
422+
// TODO(#4918): Recognize not-yet-implemented "org-deactivated" error;
423+
// see https://github.com/zulip/zulip/issues/19347
424+
if (error instanceof ApiError && error.message.length > 0) {
425+
// E.g., "Subdomain required".
426+
message = {
427+
text: 'The server at {realm} said:\n\n{message}',
428+
values: { realm: realm.toString(), message: error.message },
429+
};
430+
} else if (error instanceof NetworkError) {
431+
message = {
432+
text: 'Could not connect to {realm}. Please check your network connection and try again.',
433+
values: { realm: realm.toString() },
434+
};
435+
} else if (error instanceof MalformedResponseError && error.httpStatus === 404) {
436+
message = {
437+
text: 'The server at {realm} does not seem to be a Zulip server.',
438+
values: { realm: realm.toString() },
439+
};
440+
learnMoreButton = {
441+
text: 'Find your Zulip server URL',
442+
url: new URL('https://zulip.com/help/logging-in#find-the-zulip-log-in-url'),
443+
};
444+
} else if (error instanceof Server5xxError) {
445+
message = {
446+
text: 'The server at {realm} encountered an error.',
447+
values: { realm: realm.toString() },
448+
};
449+
} else {
450+
message = {
451+
text: 'Failed to connect to server: {realm}',
452+
values: { realm: realm.toString() },
453+
};
454+
}
455+
456+
return { type: 'error', title, message, learnMoreButton };
457+
}
458+
}

src/start/RealmInputScreen.js

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,18 @@ import { useFocusEffect } from '@react-navigation/native';
66

77
import type { RouteProp } from '../react-navigation';
88
import type { AppNavigationProp } from '../nav/AppNavigator';
9-
import type { ServerSettings } from '../api/settings/getServerSettings';
109
import ZulipTextIntl from '../common/ZulipTextIntl';
1110
import Screen from '../common/Screen';
1211
import ZulipButton from '../common/ZulipButton';
1312
import { tryParseUrl } from '../utils/url';
14-
import * as api from '../api';
13+
import { fetchServerSettings } from '../message/fetchActions';
1514
import { ThemeContext } from '../styles/theme';
1615
import { createStyleSheet, HALF_COLOR } from '../styles';
1716
import { showErrorAlert } from '../utils/info';
1817
import { TranslationContext } from '../boot/TranslationProvider';
1918
import type { LocalizableText } from '../types';
19+
import { getGlobalSettings } from '../directSelectors';
20+
import { useGlobalSelector } from '../react-redux';
2021

2122
type Props = $ReadOnly<{|
2223
navigation: AppNavigationProp<'realm-input'>,
@@ -61,6 +62,8 @@ const tryParseInput = (realmInputValue: string): MaybeParsedInput => {
6162
export default function RealmInputScreen(props: Props): Node {
6263
const { navigation, route } = props;
6364

65+
const globalSettings = useGlobalSelector(getGlobalSettings);
66+
6467
const _ = React.useContext(TranslationContext);
6568
const themeContext = React.useContext(ThemeContext);
6669

@@ -95,21 +98,24 @@ export default function RealmInputScreen(props: Props): Node {
9598
}
9699

97100
setProgress(true);
98-
try {
99-
const serverSettings: ServerSettings = await api.getServerSettings(maybeParsedInput.value);
100-
navigation.push('auth', { serverSettings });
101-
Keyboard.dismiss();
102-
} catch (errorIllTyped) {
103-
const err: mixed = errorIllTyped; // https://github.com/facebook/flow/issues/2470
104-
showErrorAlert(_('Cannot connect to server.'));
105-
/* eslint-disable no-console */
106-
console.warn('RealmInputScreen: failed to connect to server:', err);
107-
// $FlowFixMe[incompatible-cast]: assuming caught exception was Error
108-
console.warn((err: Error).stack);
109-
} finally {
110-
setProgress(false);
101+
const result = await fetchServerSettings(maybeParsedInput.value);
102+
setProgress(false);
103+
if (result.type === 'error') {
104+
showErrorAlert(
105+
_(result.title),
106+
_(result.message),
107+
result.learnMoreButton && {
108+
url: result.learnMoreButton.url,
109+
text: result.learnMoreButton.text != null ? _(result.learnMoreButton.text) : undefined,
110+
globalSettings,
111+
},
112+
);
113+
return;
111114
}
112-
}, [navigation, maybeParsedInput, _]);
115+
const serverSettings = result.value;
116+
navigation.push('auth', { serverSettings });
117+
Keyboard.dismiss();
118+
}, [navigation, maybeParsedInput, globalSettings, _]);
113119

114120
const styles = React.useMemo(
115121
() =>

static/translations/messages_en.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@
7979
"Please specify a valid stream.": "Please specify a valid stream.",
8080
"Please choose recipients.": "Please choose recipients.",
8181
"Message is empty.": "Message is empty.",
82+
"Could not connect": "Could not connect",
83+
"The server at {realm} said:\n\n{message}": "The server at {realm} said:\n\n{message}",
84+
"Could not connect to {realm}. Please check your network connection and try again.": "Could not connect to {realm}. Please check your network connection and try again.",
85+
"The server at {realm} does not seem to be a Zulip server.": "The server at {realm} does not seem to be a Zulip server.",
86+
"Find your Zulip server URL": "Find your Zulip server URL",
87+
"The server at {realm} encountered an error.": "The server at {realm} encountered an error.",
8288
"Failed to connect to server: {realm}": "Failed to connect to server: {realm}",
8389
"{_}": "{_}",
8490
"Home": "Home",
@@ -123,7 +129,6 @@
123129
"Chat": "Chat",
124130
"Sign in with {method}": "Sign in with {method}",
125131
"Invalid input": "Invalid input",
126-
"Cannot connect to server.": "Cannot connect to server.",
127132
"Please enter a valid URL.": "Please enter a valid URL.",
128133
"Please enter the server URL, not your email.": "Please enter the server URL, not your email.",
129134
"Wrong email or password. Try again.": "Wrong email or password. Try again.",

0 commit comments

Comments
 (0)