Fix error handling within iterator #362
Conversation
When no error message needs to be passed, use 'return' to exit the generator as recommended in PEP-479. When an error message needs to be passed, raise a RuntimeError exception instead to ease the data flow. No need to catch exception in the iterator as we will deal globally with exceptions raised at higher level.
The logic within 'util.defer' is catching all exceptions and emit it as a result without discriminating between finished tasks and errors. Therefore we could simply check if the result is an instance of Exception to figure out if the process should exit.
Using try-catch to transmit error messages from the iterator to the process callback would be nicer, but some issues remains: - It doesn't account for non-error messages (e.g. "Stopped") - It doesn't discriminate between error which should be caught and error which should raise. Therefore it is simpler to keep using StopIteration to exit the iterator while transmitting messages to display, and change the syntax to conform with Python 2.7 (No 'return' with argument allowed inside generators) and Python 3.7 (generators shouldn't raise StopIteration).
|
I reverted to using the logic suggested in #353 (comment). Removing usage of |
Do you mean that just calling |
Yes that's what I mean. I guess it could be because targets default to pyblish-qml/pyblish_qml/host.py Line 100 in 18826c8 pyblish-qml/pyblish_qml/ipc/server.py Line 206 in 18826c8 |
Can confirm this is happening, it happened in the latest release 1.11.1 and works as intended in 1.11.0. I've pushed a fix for it now. That's beside the point in this PR however. Let me know when you're ready for another review. |
Thanks!
Ready now :) |
…g_within_iterator
|
Just merged back latest change to this branch, let me know if there are any issues still! |
This resolves #353
Tested with PySide2 and Python 2.7, Python 3.7 and Python 3.8 with the demo window:
(took me a while to figure out that I had to pass the 'default' target to see something!)
I didn't deal with unit tests as I am unfamiliar with nose, please let me know if you want me to handle it.