refactor: model: Use str for ThreadPool function return values.

This allows for more information to be returned beyond the success/fail
from a bool, particularly useful in the case of failure.

Return values are changed as follows:
* True -> ''
* False -> 'Some error response'

Note that this flips the truthiness of the return type, leading to
addition/removal of negation to retain previous behavior.

Tests and typing amended, including addition of a missing 'type:' in a
variable type hint.

Author: neiljp

tests/model/test_model.py

@@ -22,7 +22,8 @@ def mock_external_classes(self, mocker: Any) -> None:
 
     @pytest.fixture
     def model(self, mocker, initial_data, user_profile):
-        mocker.patch('zulipterminal.model.Model.get_messages')
+        mocker.patch('zulipterminal.model.Model.get_messages',
+                     return_value='')
         self.client.register.return_value = initial_data
         mocker.patch('zulipterminal.model.Model.get_all_users',
                      return_value=[])
@@ -67,7 +68,8 @@ def test_init(self, model, initial_data, user_profile):
         assert model.unread_counts == []
 
     def test_register_initial_desired_events(self, mocker, initial_data):
-        mocker.patch('zulipterminal.model.Model.get_messages')
+        mocker.patch('zulipterminal.model.Model.get_messages',
+                     return_value='')
         mocker.patch('zulipterminal.model.Model.get_all_users')
         mocker.patch('zulipterminal.model.Model.fetch_all_topics')
         self.client.register.return_value = initial_data
@@ -252,10 +254,11 @@ def test_get_message_ids_in_current_narrow(self, mocker, model,
 
     @pytest.mark.parametrize("response, expected_index, return_value", [
         ({'result': 'success', 'topics': [{'name': 'Foo'}, {'name': 'Boo'}]},
-         {23: ['Foo', 'Boo']}, True),
+         {23: ['Foo', 'Boo']}, ''),
         ({'result': 'success', 'topics': []},
-         {23: []}, True),
-        ({'result': 'failure', 'topics': []}, {23: []}, False)
+         {23: []}, ''),
+        ({'result': 'failure', 'msg': 'Some Error', 'topics': []},
+         {23: []}, 'Some Error')
     ])
     def test_get_topics_in_streams(self, mocker, response, model, return_value,
                                    expected_index) -> None:
@@ -533,7 +536,7 @@ def test__update_initial_data(self, model, initial_data):
 
     def test__update_initial_data_raises_exception(self, mocker, initial_data):
         # Initialize Model
-        mocker.patch('zulipterminal.model.Model.get_messages')
+        mocker.patch('zulipterminal.model.Model.get_messages', return_value='')
         mocker.patch('zulipterminal.model.Model.get_all_users',
                      return_value=[])
         mocker.patch('zulipterminal.model.Model.'
@@ -568,7 +571,7 @@ def test__group_info_from_realm_user_groups(self, model,
 
     def test_get_all_users(self, mocker, initial_data, user_list, user_dict,
                            user_id):
-        mocker.patch('zulipterminal.model.Model.get_messages')
+        mocker.patch('zulipterminal.model.Model.get_messages', return_value='')
         self.client.register.return_value = initial_data
         mocker.patch('zulipterminal.model.Model.'
                      '_stream_info_from_subscriptions',

zulipterminal/model.py

@@ -330,7 +330,7 @@ def update_stream_message(self, topic: str, msg_id: int,
 
     def get_messages(self, *,
                      num_after: int, num_before: int,
-                     anchor: Optional[int]) -> bool:
+                     anchor: Optional[int]) -> str:
         # anchor value may be specific message (int) or next unread (None)
         first_anchor = anchor is None
         anchor_value = anchor if anchor is not None else 0
@@ -356,10 +356,10 @@ def get_messages(self, *,
                 # 'found_newest' flag. Instead, we use this logic:
                 query_range = num_after + num_before + 1
                 self.found_newest = len(response['messages']) < query_range
-            return True
-        return False
+            return ""
+        return response['msg']
 
-    def get_topics_in_stream(self, stream_list: Iterable[int]) -> bool:
+    def get_topics_in_stream(self, stream_list: Iterable[int]) -> str:
         """
         Fetch all topics with specified stream_id's and
         index their names (Version 1)
@@ -371,15 +371,15 @@ def get_topics_in_stream(self, stream_list: Iterable[int]) -> bool:
                 self.index['topics'][stream_id] = [topic['name'] for
                                                    topic in response['topics']]
             else:
-                return False
-        return True
+                return response['msg']
+        return ""
 
     @staticmethod
-    def exception_safe_result(future: 'Future[bool]') -> bool:
+    def exception_safe_result(future: 'Future[str]') -> str:
         try:
             return future.result()
-        except zulip.ZulipError:
-            return False
+        except zulip.ZulipError as e:
+            return str(e)
 
     def fetch_all_topics(self, workers: int) -> None:
         """
@@ -392,17 +392,17 @@ def fetch_all_topics(self, workers: int) -> None:
                 i: executor.submit(self.get_topics_in_stream,
                                    list_of_streams[i::workers])
                 for i in range(workers)
-            }  # type: Dict[int, Future[bool]]
+            }  # type: Dict[int, Future[str]]
             wait(thread_objects.values())
 
         results = {
             str(name): self.exception_safe_result(thread_object)
             for name, thread_object in thread_objects.items()
-        }  # type: Dict[str, bool]
-        if not all(results.values()):
+        }  # type: Dict[str, str]
+        if any(results.values()):
             failures = ['fetch_topics[{}]'.format(name)
                         for name, result in results.items()
-                        if not result]
+                        if result]
             raise ServerConnectionFailure(", ".join(failures))
 
     def is_muted_stream(self, stream_id: int) -> bool:
@@ -426,22 +426,22 @@ def _update_initial_data(self) -> None:
                                                 anchor=None),
                 'register': executor.submit(self._register_desired_events,
                                             fetch_data=True),
-            }  # Dict[str, Future[bool]]
+            }  # type: Dict[str, Future[str]]
 
             # Wait for threads to complete
             wait(futures.values())
 
         results = {
             name: self.exception_safe_result(future)
             for name, future in futures.items()
-        }  # type: Dict[str, bool]
-        if all(results.values()):
+        }  # type: Dict[str, str]
+        if not any(results.values()):
             self.user_id = self.initial_data['user_id']
             self.user_email = self.initial_data['email']
             self.user_full_name = self.initial_data['full_name']
             self.server_name = self.initial_data['realm_name']
         else:
-            failures = [name for name, result in results.items() if not result]
+            failures = [name for name, result in results.items() if result]
             raise ServerConnectionFailure(", ".join(failures))
 
     def get_all_users(self) -> List[Dict[str, Any]]:
@@ -869,7 +869,7 @@ def update_rendered_view(self, msg_id: int) -> None:
                     self.controller.update_screen()
                     return
 
-    def _register_desired_events(self, *, fetch_data: bool=False) -> bool:
+    def _register_desired_events(self, *, fetch_data: bool=False) -> str:
         fetch_types = None if not fetch_data else [
             'realm',
             'presence',
@@ -886,17 +886,17 @@ def _register_desired_events(self, *, fetch_data: bool=False) -> bool:
                                             fetch_event_types=fetch_types,
                                             client_gravatar=True,
                                             apply_markdown=True)
-        except zulip.ZulipError:
-            return False
+        except zulip.ZulipError as e:
+            return str(e)
 
         if response['result'] == 'success':
             if fetch_data:
                 self.initial_data.update(response)
             self.max_message_id = response['max_message_id']
             self.queue_id = response['queue_id']
             self.last_event_id = response['last_event_id']
-            return True
-        return False
+            return ""
+        return response['msg']
 
     @asynch
     def poll_for_events(self) -> None: