fix!: WebSocketApp.run_forever() returning None after close() i…

…s called (#788)

* fix!: `WebSocketApp.run_forever()` returning `None` after `close()` is called

BREAKING CHANGE: the return value of `WebSocketApp.run_forever()` will no longer return the (undocumented) `None` value.

* fix(tests): replace the defunct `wss://stream.meetup.com/2/rsvps` with `wss://api.bitfinex.com/ws/2` in several tests

* test: added two new local server tests to cover the changes in #785

* Remove newline to fix lint error

websocket/_app.py

@@ -272,7 +272,8 @@ def run_forever(self, sockopt=None, sslopt=None,
         Returns
         -------
         teardown: bool
-            False if caught KeyboardInterrupt, True if other exception was raised during a loop
+            False if the `WebSocketApp` is closed or caught KeyboardInterrupt,
+            True if any other exception was raised during a loop.
         """
 
         if ping_timeout is not None and ping_timeout <= 0:
@@ -380,6 +381,7 @@ def check():
                 return True
 
             dispatcher.read(self.sock.sock, read, check)
+            return False
         except (Exception, KeyboardInterrupt, SystemExit) as e:
             self._callback(self.on_error, e)
             if isinstance(e, SystemExit):

websocket/tests/test_app.py

@@ -21,6 +21,7 @@
 
 import os
 import os.path
+import threading
 import websocket as ws
 import ssl
 import unittest
@@ -45,11 +46,13 @@ def setUp(self):
         WebSocketAppTest.keep_running_open = WebSocketAppTest.NotSetYet()
         WebSocketAppTest.keep_running_close = WebSocketAppTest.NotSetYet()
         WebSocketAppTest.get_mask_key_id = WebSocketAppTest.NotSetYet()
+        WebSocketAppTest.on_error_data = WebSocketAppTest.NotSetYet()
 
     def tearDown(self):
         WebSocketAppTest.keep_running_open = WebSocketAppTest.NotSetYet()
         WebSocketAppTest.keep_running_close = WebSocketAppTest.NotSetYet()
         WebSocketAppTest.get_mask_key_id = WebSocketAppTest.NotSetYet()
+        WebSocketAppTest.on_error_data = WebSocketAppTest.NotSetYet()
 
     @unittest.skipUnless(TEST_WITH_LOCAL_SERVER, "Tests using local websocket server are disabled")
     def testKeepRunning(self):
@@ -97,6 +100,34 @@ def on_message(wsapp, message):
         app = ws.WebSocketApp('ws://127.0.0.1:' + LOCAL_WS_SERVER_PORT, on_open=on_open, on_message=on_message)
         app.run_forever(dispatcher="Dispatcher")
 
+    @unittest.skipUnless(TEST_WITH_LOCAL_SERVER, "Tests using local websocket server are disabled")
+    def testRunForeverTeardownCleanExit(self):
+        """ The WebSocketApp.run_forever() method should return `False` when the application ends gracefully.
+        """
+        app = ws.WebSocketApp('ws://127.0.0.1:' + LOCAL_WS_SERVER_PORT)
+        threading.Timer(interval=0.2, function=app.close).start()
+        teardown = app.run_forever()
+        self.assertEqual(teardown, False)
+
+    @unittest.skipUnless(TEST_WITH_LOCAL_SERVER, "Tests using local websocket server are disabled")
+    def testRunForeverTeardownExceptionalExit(self):
+        """ The WebSocketApp.run_forever() method should return `True` when the application ends with an exception.
+        It should also invoke the `on_error` callback before exiting.
+        """
+
+        def break_it():
+            # Deliberately break the WebSocketApp by closing the inner socket.
+            app.sock.close()
+
+        def on_error(_, err):
+            WebSocketAppTest.on_error_data = str(err)
+
+        app = ws.WebSocketApp('ws://127.0.0.1:' + LOCAL_WS_SERVER_PORT, on_error=on_error)
+        threading.Timer(interval=0.2, function=break_it).start()
+        teardown = app.run_forever(ping_timeout=0.1)
+        self.assertEqual(teardown, True)
+        self.assertTrue(len(WebSocketAppTest.on_error_data) > 0)
+
     @unittest.skipUnless(TEST_WITH_INTERNET, "Internet-requiring tests are disabled")
     def testSockMaskKey(self):
         """ A WebSocketApp should forward the received mask_key function down
@@ -106,7 +137,7 @@ def testSockMaskKey(self):
         def my_mask_key_func():
             return "\x00\x00\x00\x00"
 
-        app = ws.WebSocketApp('wss://stream.meetup.com/2/rsvps', get_mask_key=my_mask_key_func)
+        app = ws.WebSocketApp('wss://api-pub.bitfinex.com/ws/1', get_mask_key=my_mask_key_func)
 
         # if numpy is installed, this assertion fail
         # Note: We can't use 'is' for comparing the functions directly, need to use 'id'.

websocket/tests/test_websocket.py

@@ -201,14 +201,16 @@ def testRecv(self):
     @unittest.skipUnless(TEST_WITH_INTERNET, "Internet-requiring tests are disabled")
     def testIter(self):
         count = 2
-        for _ in ws.create_connection('wss://testnet-explorer.binance.org/ws/block'):
+        s = ws.create_connection('wss://api.bitfinex.com/ws/2')
+        s.send('{"event": "subscribe", "channel": "ticker"}')
+        for _ in s:
             count -= 1
             if count == 0:
                 break
 
     @unittest.skipUnless(TEST_WITH_INTERNET, "Internet-requiring tests are disabled")
     def testNext(self):
-        sock = ws.create_connection('wss://testnet-explorer.binance.org/ws/block')
+        sock = ws.create_connection('wss://api.bitfinex.com/ws/2')
         self.assertEqual(str, type(next(sock)))
 
     def testInternalRecvStrict(self):