Skip to content

Commit a1faf08

Browse files
committed
model: Report error details from failed initial API calls.
Given failed calls now provide a textual reason, this information can be passed to returned exceptions, and ultimately to the user. This change applies to the initial connection only. Since ZulipError error texts are now returned, as well as responses from successful network calls which fail at the server, a test case is added for each.
1 parent 75650da commit a1faf08

File tree

2 files changed

+56
-5
lines changed

2 files changed

+56
-5
lines changed

tests/model/test_model.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import Any
44

55
import pytest
6+
from zulip import ZulipError
67

78
from zulipterminal.helper import initial_index, powerset
89
from zulipterminal.model import Model, ServerConnectionFailure
@@ -67,6 +68,49 @@ def test_init(self, model, initial_data, user_profile):
6768
self.classify_unread_counts.assert_called_once_with(model)
6869
assert model.unread_counts == []
6970

71+
def test_init_InvalidAPIKey_response(self, mocker, initial_data):
72+
# Both network calls indicate the same response
73+
mocker.patch('zulipterminal.model.Model.get_messages',
74+
return_value='Invalid API key')
75+
mocker.patch('zulipterminal.model.Model._register_desired_events',
76+
return_value='Invalid API key')
77+
78+
mocker.patch('zulipterminal.model.Model.get_all_users',
79+
return_value=[])
80+
mocker.patch('zulipterminal.model.Model.'
81+
'_stream_info_from_subscriptions',
82+
return_value=({}, set(), [], []))
83+
self.classify_unread_counts = mocker.patch(
84+
'zulipterminal.model.classify_unread_counts',
85+
return_value=[])
86+
87+
with pytest.raises(ServerConnectionFailure) as e:
88+
model = Model(self.controller)
89+
90+
assert str(e.value) == 'Invalid API key (get_messages, register)'
91+
92+
def test_init_ZulipError_exception(self, mocker, initial_data,
93+
exception_text="X"):
94+
# Both network calls fail, resulting in exceptions
95+
mocker.patch('zulipterminal.model.Model.get_messages',
96+
side_effect=ZulipError(exception_text))
97+
mocker.patch('zulipterminal.model.Model._register_desired_events',
98+
side_effect=ZulipError(exception_text))
99+
100+
mocker.patch('zulipterminal.model.Model.get_all_users',
101+
return_value=[])
102+
mocker.patch('zulipterminal.model.Model.'
103+
'_stream_info_from_subscriptions',
104+
return_value=({}, set(), [], []))
105+
self.classify_unread_counts = mocker.patch(
106+
'zulipterminal.model.classify_unread_counts',
107+
return_value=[])
108+
109+
with pytest.raises(ServerConnectionFailure) as e:
110+
model = Model(self.controller)
111+
112+
assert str(e.value) == exception_text + ' (get_messages, register)'
113+
70114
def test_register_initial_desired_events(self, mocker, initial_data):
71115
mocker.patch('zulipterminal.model.Model.get_messages',
72116
return_value='')

zulipterminal/model.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import json
22
import time
3-
from collections import OrderedDict
3+
from collections import OrderedDict, defaultdict
44
from concurrent.futures import Future, ThreadPoolExecutor, wait
55
from threading import Thread
66
from typing import (
7-
Any, Callable, Dict, FrozenSet, Iterable, List, Optional, Set, Tuple,
8-
Union,
7+
Any, Callable, DefaultDict, Dict, FrozenSet, Iterable, List, Optional, Set,
8+
Tuple, Union,
99
)
1010
from urllib.parse import urlparse
1111

@@ -441,8 +441,15 @@ def _update_initial_data(self) -> None:
441441
self.user_full_name = self.initial_data['full_name']
442442
self.server_name = self.initial_data['realm_name']
443443
else:
444-
failures = [name for name, result in results.items() if result]
445-
raise ServerConnectionFailure(", ".join(failures))
444+
failures = defaultdict(list) # type: DefaultDict[str, List[str]]
445+
for name, result in results.items():
446+
if result:
447+
failures[result].append(name)
448+
failure_text = [
449+
"{} ({})".format(error, ", ".join(sorted(calls)))
450+
for error, calls in failures.items()
451+
]
452+
raise ServerConnectionFailure(", ".join(failure_text))
446453

447454
def get_all_users(self) -> List[Dict[str, Any]]:
448455
# Dict which stores the active/idle status of users (by email)

0 commit comments

Comments
 (0)