So using sys.excepthook to catch errors only works once.
The error bubbles through the event processing loop, breaking it and hanging the application. Instead, there is now a .exception(e) method on XMLStream which may be overridden or reassigned that will receive all unhandled exceptions (read: not XMPPError) from event and stream handlers.
This commit is contained in:
parent
d8d9e8df16
commit
2a2ac73845
6 changed files with 112 additions and 39 deletions
|
@ -64,8 +64,7 @@ class RootStanza(StanzaBase):
|
|||
# log the error
|
||||
log.exception('Error handling {%s}%s stanza' %
|
||||
(self.namespace, self.name))
|
||||
# Finally raise the exception, so it can be handled (or not)
|
||||
# at a higher level by using sys.excepthook.
|
||||
raise e
|
||||
# Finally raise the exception to a global exception handler
|
||||
self.stream.exception(e)
|
||||
|
||||
register_stanza_plugin(RootStanza, Error)
|
||||
|
|
|
@ -764,7 +764,6 @@ class XMLStream(object):
|
|||
Event handlers and the send queue will be threaded
|
||||
regardless of this parameter's value.
|
||||
"""
|
||||
self._thread_excepthook()
|
||||
self.scheduler.process(threaded=True)
|
||||
|
||||
def start_thread(name, target):
|
||||
|
@ -1052,30 +1051,16 @@ class XMLStream(object):
|
|||
self.event_queue.put(('quit', None, None))
|
||||
return
|
||||
|
||||
def _thread_excepthook(self):
|
||||
def exception(self, exception):
|
||||
"""
|
||||
If a threaded event handler raises an exception, there is no way to
|
||||
catch it except with an excepthook. Currently, each thread has its own
|
||||
excepthook, but ideally we could use the main sys.excepthook.
|
||||
Process an unknown exception.
|
||||
|
||||
Modifies threading.Thread to use sys.excepthook when an exception
|
||||
is not caught.
|
||||
Meant to be overridden.
|
||||
|
||||
Arguments:
|
||||
exception -- An unhandled exception object.
|
||||
"""
|
||||
init_old = threading.Thread.__init__
|
||||
|
||||
def init(self, *args, **kwargs):
|
||||
init_old(self, *args, **kwargs)
|
||||
run_old = self.run
|
||||
|
||||
def run_with_except_hook(*args, **kw):
|
||||
try:
|
||||
run_old(*args, **kw)
|
||||
except (KeyboardInterrupt, SystemExit):
|
||||
raise
|
||||
except:
|
||||
sys.excepthook(*sys.exc_info())
|
||||
self.run = run_with_except_hook
|
||||
threading.Thread.__init__ = init
|
||||
pass
|
||||
|
||||
|
||||
# To comply with PEP8, method names now use underscores.
|
||||
|
|
|
@ -12,7 +12,6 @@ class TestStreamExceptions(SleekTest):
|
|||
"""
|
||||
|
||||
def tearDown(self):
|
||||
sys.excepthook = sys.__excepthook__
|
||||
self.stream_close()
|
||||
|
||||
def testExceptionReply(self):
|
||||
|
@ -23,8 +22,6 @@ class TestStreamExceptions(SleekTest):
|
|||
msg['body'] = 'Body changed'
|
||||
raise XMPPError(clear=False)
|
||||
|
||||
|
||||
sys.excepthook = lambda *args, **kwargs: None
|
||||
self.stream_start()
|
||||
self.xmpp.add_event_handler('message', message)
|
||||
|
||||
|
@ -44,6 +41,49 @@ class TestStreamExceptions(SleekTest):
|
|||
</message>
|
||||
""")
|
||||
|
||||
def testExceptionContinueWorking(self):
|
||||
"""Test that Sleek continues to respond after an XMPPError is raised."""
|
||||
|
||||
def message(msg):
|
||||
msg.reply()
|
||||
msg['body'] = 'Body changed'
|
||||
raise XMPPError(clear=False)
|
||||
|
||||
self.stream_start()
|
||||
self.xmpp.add_event_handler('message', message)
|
||||
|
||||
self.recv("""
|
||||
<message>
|
||||
<body>This is going to cause an error.</body>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.send("""
|
||||
<message type="error">
|
||||
<body>This is going to cause an error.</body>
|
||||
<error type="cancel" code="500">
|
||||
<undefined-condition
|
||||
xmlns="urn:ietf:params:xml:ns:xmpp-stanzas" />
|
||||
</error>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.recv("""
|
||||
<message>
|
||||
<body>This is going to cause an error.</body>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.send("""
|
||||
<message type="error">
|
||||
<body>This is going to cause an error.</body>
|
||||
<error type="cancel" code="500">
|
||||
<undefined-condition
|
||||
xmlns="urn:ietf:params:xml:ns:xmpp-stanzas" />
|
||||
</error>
|
||||
</message>
|
||||
""")
|
||||
|
||||
def testXMPPErrorException(self):
|
||||
"""Test raising an XMPPError exception."""
|
||||
|
||||
|
@ -153,9 +193,8 @@ class TestStreamExceptions(SleekTest):
|
|||
def catch_error(*args, **kwargs):
|
||||
raised_errors.append(True)
|
||||
|
||||
sys.excepthook = catch_error
|
||||
|
||||
self.stream_start()
|
||||
self.xmpp.exception = catch_error
|
||||
self.xmpp.add_event_handler('message', message)
|
||||
|
||||
self.recv("""
|
||||
|
@ -178,6 +217,58 @@ class TestStreamExceptions(SleekTest):
|
|||
|
||||
self.assertEqual(raised_errors, [True], "Exception was not raised: %s" % raised_errors)
|
||||
|
||||
def testUnknownException(self):
|
||||
"""Test Sleek continues to respond after an unknown exception."""
|
||||
|
||||
raised_errors = []
|
||||
|
||||
def message(msg):
|
||||
raise ValueError("Did something wrong")
|
||||
|
||||
def catch_error(*args, **kwargs):
|
||||
raised_errors.append(True)
|
||||
|
||||
self.stream_start()
|
||||
self.xmpp.exception = catch_error
|
||||
self.xmpp.add_event_handler('message', message)
|
||||
|
||||
self.recv("""
|
||||
<message>
|
||||
<body>This is going to cause an error.</body>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.send("""
|
||||
<message type="error">
|
||||
<error type="cancel" code="500">
|
||||
<undefined-condition
|
||||
xmlns="urn:ietf:params:xml:ns:xmpp-stanzas" />
|
||||
<text xmlns="urn:ietf:params:xml:ns:xmpp-stanzas">
|
||||
SleekXMPP got into trouble.
|
||||
</text>
|
||||
</error>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.recv("""
|
||||
<message>
|
||||
<body>This is going to cause an error.</body>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.send("""
|
||||
<message type="error">
|
||||
<error type="cancel" code="500">
|
||||
<undefined-condition
|
||||
xmlns="urn:ietf:params:xml:ns:xmpp-stanzas" />
|
||||
<text xmlns="urn:ietf:params:xml:ns:xmpp-stanzas">
|
||||
SleekXMPP got into trouble.
|
||||
</text>
|
||||
</error>
|
||||
</message>
|
||||
""")
|
||||
|
||||
self.assertEqual(raised_errors, [True, True], "Exceptions were not raised: %s" % raised_errors)
|
||||
|
||||
|
||||
suite = unittest.TestLoader().loadTestsFromTestCase(TestStreamExceptions)
|
||||
|
|
|
@ -12,7 +12,6 @@ class TestStreamDisco(SleekTest):
|
|||
"""
|
||||
|
||||
def tearDown(self):
|
||||
sys.excepthook = sys.__excepthook__
|
||||
self.stream_close()
|
||||
|
||||
def testInfoEmptyDefaultNode(self):
|
||||
|
@ -531,11 +530,6 @@ class TestStreamDisco(SleekTest):
|
|||
|
||||
raised_exceptions = []
|
||||
|
||||
def catch_exception(*args, **kwargs):
|
||||
raised_exceptions.append(True)
|
||||
|
||||
sys.excepthook = catch_exception
|
||||
|
||||
self.stream_start(mode='client',
|
||||
plugins=['xep_0030', 'xep_0059'])
|
||||
|
||||
|
@ -544,8 +538,14 @@ class TestStreamDisco(SleekTest):
|
|||
iterator=True)
|
||||
results.amount = 10
|
||||
|
||||
def run_test():
|
||||
try:
|
||||
results.next()
|
||||
except StopIteration:
|
||||
raised_exceptions.append(True)
|
||||
|
||||
t = threading.Thread(name="get_items_iterator",
|
||||
target=results.next)
|
||||
target=run_test)
|
||||
t.start()
|
||||
|
||||
self.send("""
|
||||
|
|
|
@ -13,7 +13,6 @@ class TestStreamExtendedDisco(SleekTest):
|
|||
"""
|
||||
|
||||
def tearDown(self):
|
||||
sys.excepthook = sys.__excepthook__
|
||||
self.stream_close()
|
||||
|
||||
def testUsingExtendedInfo(self):
|
||||
|
|
|
@ -13,7 +13,6 @@ class TestStreamDirectInvite(SleekTest):
|
|||
"""
|
||||
|
||||
def tearDown(self):
|
||||
sys.excepthook = sys.__excepthook__
|
||||
self.stream_close()
|
||||
|
||||
def testReceiveInvite(self):
|
||||
|
|
Loading…
Reference in a new issue