try/except clauses should usually catch some kind of exception

[maguro]

try/except clauses should usually catch some kind of exception or else it's not possible to stop willie without a kill -9 in certain circumstances.

[elad661]

Can you provide examples/test cases for this error?

[maguro]

It's usually a poor practice to have a try/except clause because if a KeyboardInterrupt raised it effectively eats that exception the program does not stop. At the very least it should catch Exception.

We've had problems with tools not stopping unless we sent a -9 signal to stop them because of this.

[elad661]

We catch KeyboardInterrupt in __init__, and we provide other ways to tell the bot to exit as well. I don't see any problem.

[maguro]

I think we can agree that it's generally a poor practice and I will point out that the problem is not systemic with Willie. However, there are a few areas that may be of concern, e.g. coretasks.py.

[elad661]

Everything in coretasks is threaded. The main thread is listening to KeyboardInterrupt, not the child threads.

[maguro]

Yes and I notice that the threads don't seem to be daemon threads.

With that said, I will still hold onto my opinion that it's a sloppy practice unless you have a concrete specific strategy in mind; such cases are rare. You are entitled to hold your own opinion that it's perfectly acceptable to catch all exceptions and ignore them.

[ari-koivula]

@maguro yes, we can agree that it's bad and only specific exceptions should be raised whenever possible. Both to make it clear what exception is being handled and to avoid eating exceptions that shouldn't be handled at that level. The scope of the try-except clause should also be as small as possible, to make it clear where the exception is expected.

This is basic stuff and there is an open issue to make the code conform to pep8, which describes at least some of this.

And I too have lamented the overuse of threads. It might be a good idea to move to a worker thread model, where there is only as many threads as is sensible, which would take jobs from a queue and execute them. Modules can always maintain their own threads if they really need to.

[ari-koivula]

I just spent 3 hours hunting down the reason why coverage.py wasn't working correctly. The trace function was being set to None for no apparent reason. Well, it turned out blank try-except statements in Willie.Trigger.new were the culprit, or at least hid the reason. These things need to die.

[dgw]

Nothing about this is likely to change without significant pressure. Until and unless that pressure appears, there is no reason to keep this issue open.