Open jordal opened 8 years ago
meejah
commented
8 years ago As per https://github.com/crossbario/crossbar/blob/master/crossbar/worker/container.py#L213 it does look like this behavior (killing a component if it raises an exception from a handler) is on purpose -- would be easy to make non-fatal, what does @oberstet think?
oberstet
commented
8 years ago No, this is by design: if you want your component to continue despite the fact that it tried to subscribe and that fails, then catch the exception in your app code and properly treat it:
@defer.inlineCallbacks
def onJoin(self, details):
try:
yield self.subscribe(self.on_event, u'test.pubsub')
except:
self.log.warn('could not subscribe, but somehow I dont care anyways')And uncaught exception should be brought to maximum attention, not simply logged.
jordal
commented
8 years ago @oberstet I think we're talking about something different. My concern is not with the subscribe operation itself, but with an exception raised from the handler while processing pubsub events.
If you still disagree then I'll make one more plea: I think a robust server should, if reasonably possible, not enter an impaired state if the application raises an error while processing a pubsub event. There's no reason to believe that such an error will recur on that pubsub topic and that it's fatal enough to justify aborting the application.
This is loosely analogous to most web frameworks that trap and log unhandled application errors and turn them into 500 status codes, but don't silently stop serving requests to the application thereafter.
Which raises another question: as it currently behaves, the crossbar server doesn't abort---the application component just silently stops working. To bring "maximum attention", wouldn't it be better to abort crossbar rather than keep running with a disabled component? (honest question, not meant to be snarky)
I wrote up a quick decorator that applies the behavior to subscribe event handlers. I'm posting it here in case anyone else finds it useful. This is, of course, no excuse to not catch and handle internal exceptions in your code whenever you can.
def trap_and_log(wrapped):
@functools.wraps(wrapped)
def _wrapper(*args, **kargs):
def print_traceback(failure):
log.error(
"Unhandled exception caught: {f}",
f=txaio.failure_format_traceback(failure))
try:
res = wrapped(*args, **kargs)
except Exception:
# If the function outright raised an exception, then just log it.
failure = txaio.create_failure()
print_traceback(failure)
else:
# If the function returned a Deferred, attach an errback to it.
# Otherwise, return its result.
if isinstance(res, defer.Deferred):
return res.addErrback(print_traceback)
else:
return res
return _wrapperPersonally, I think it's better to log and keep going -- but perhaps this should be an option? That is, by default we log and keep going (I think this is "least surprising"), but we could provide an option that makes an exception from a handler component-fatal (or as @jordal suggested, crossbar-fatal).
These latter could be useful especially in development -- or even in production depending on the setup of the system (e.g. if it can recover on restart, and you've got something monitoring the crossbar process itself).
Instead of a config-option, we could provide a wamp.* notification of some sort, and "something else" could decide what to do (e.g. notify some web service, shut down the component, shut down crossbar, etc) and the "log and keep going" would then just always happen and if you want some different behavior you write something to respond to the notification.
A crossbar twisted-python component breaks if it subscribes to a topic and that subscription handler raises an exception; the component stops serving RPCs and unsubscribes (at least) to the topic for which it raises an exception.
This seems like unwanted/unexpected behavior. If the component provides critical services in a system, even a tiny unforeseen bug could make them all unavailable. I would expect crossbar to simply log a traceback for unhandled exceptions and move on.
Here's my environment:
I've attached code to reproduce it. The only interesting files are
backend.py,frontend.py, and.crossbar/config.json.crossbar-unhandled-exception.zip
If you have
docker-composethen you can easily run it from the directory with: