Where should we raise exceptions from stream protocol handlers?

[riaqn]

import mpv

def open_fn(url):
    print('hello')
    raise ValueError

player = mpv.MPV()
player.register_stream_protocol("test", open_fn)
player.play('test://foo')
player.wait_for_playback()

This just print hello and quits peacefully. I would expect play to raise some exception like LoadFailure

[jaseg]

This is a bit of a tricky one. When registering the stream protocol, we throw libmpv a bunch of dynamically generated ctypes function pointers, that libmpv then calls as it pleases to process the stream. This means that when open_fn in your testcase gets called, it's call stack points straight into libmp, and will not even be on the same thread as wait_for_playback.

We could try to stash away the exception when it happens in open_fn, and then raise it in wait_for_playback, but that might be counter-intuitive given that the exception is ultimately caused by play, but would only be raised later in wait_for_playback. We can't wait for the exception in play since that is just a wrapper around mpv's loadfile command, which is async by nature.

I can't really think of a good semantic where to raise these exceptions. I'm interested to hear if you have any suggestions. In the meantime, my recommendation would be to use local try/except blocks inside the stream protocol handler functions.

[riaqn]

Storing the exception and rasing it in wait_for_playback sounds like a good idea. Async exception is not uncommon and, as terrible as it is, still better than silently discarding exceptions. The latter could really cause a debug hell for users.

[jaseg]

True, swallowing it is definitely worse. I'm still unsure about wait_for_playback though. By design, multiple threads might be waiting inside wait_for_playback at the same time. In that case, which one should raise the exception? For sure it shouldn't pop up in several of them, but choosing one at random also seems bad. I can see two options here:

• Add a catch_remote_exceptions flag to wait_for_playback, wait_for_event and similar methods that explicitly enables them to raise such exceptions. This gives the user direct control of where these bubble up, and I'd just allow them to ask for the same exception in multiple threads.
• Print a very noisy warning with exception information and a stacktrace to the console when such exceptions happen.

I'm kinda partial towards the first option.

[jaseg]

I just pushed a draft of the first option. All of the wait_for_x functions now have a catch_errors arg that is True by default that enables them catching exceptions from both the event loop and stream callbacks. When an exception is raised while nothing is catching it, a warning with a stacktrace is printed. Please try it out, and tell me what you think.

[jaseg]

This is now out in v1.0.3. Feel free to open another issue if you hit any problems with it.