Consider renaming core.library._backend_error_handling "backend_error_hiding"

[tkem]

Apologies for the issue title, but I just couldn't resist ;-)

Seriously: Starting with Mopidy v1.1 AFAICS, calls to library providers are wrapped with a _backend_error_handling context manager. The purpose of this, it seems, is to catch all exceptions raised by library methods such as browse() or search(), log them, and pass a default result to the client.

Frankly, I don't call this error handling, I call this error hiding.

Before v1.1 IIRC, most or all exception raised by library providers would be passed to the client, at least with the Web/JavaScript API (using Mopidy.js in my case). Thus it was possible, if an exception was raised by a library extension due to a request timeout or a remote server being down, to provide an error indication to the user. Mopidy Mobile did this, for example, and though the error message displayed wasn't always that meaningful to end users, it gave a clear indication that "something went wrong".

With Mopidy >= v1.1, this feature seems gone. What's even worse, since the client does not even notice that an error occurred, it can't distinguish the "substitute/default" result (usually an empty list/set) from a result received by a successful operation. In Mopidy Mobile's case, this means caching, so that retrying the request (if the user happens to suspect that this empty result is not what's expected) may even have no effect.

I realize that there are situations where exceptions from extensions maybe should better be logged and done with; multi-backend searches (i.e. uri=None) come to mind, where an exception from a single backend arguably shouldn't wreck the whole search result. Anything raised by playback.translate_uri() must also be dealt with internally (error events, anybody?). But I think catching/hiding any errors that directly result from a client request is a severe regression in Mopidy v1.1 and later.

[kingosticks]

Related to #1011

[adamcik]

I think some of the original rational behind this was to shield frontends from random exceptions. So instead of having to handle Exception you should only have to worry about sub-classes of mopidy.exceptions.Error happening.

In other words we probably want to catch random backend errors and reraise them as BackendError at least for the call that don't query multiple backends.

Longer term we need the results from core to indicate partial errors.

There is also an other issue which has been bugging me which is that the HTTP backend would "hide" errors from the log. But that is probably it's own issue.

[tkem]

@adamcik: I get the point, but the code reads otherwise, i.e. no reraise=BackendError.

[adamcik]

Yeah, I'm just stating how I think it should work now, and if I implemented it differently when "fixing" this initially I agree it should be addressed.

[tkem]

Ah, OK, sorry for getting this wrong. I think that catching random TypeErrors or KeyErrors from extensions and transforming them into something more meaningful to a client makes a lot if sense. Or requiring the extension to transform these into proper BackendErrors so they can be passed on to the client might also be an option.

[adamcik]

The other longer term fix for this which would be nice is to have some a message/model as the core return value that contains errors + the result for the remaining backends. This would also if done right help tell frontends what backend everything comes from.

Sadly this is a big breaking change for frontends :(

[tkem]

Hmmm... Would reporting errors as events to clients also be an option? IMHO, there should be a way for clients to get notified of errors that are not the direct result of a client request, e.g. playback errors, anyway. Adding a new event type for this wouldn't probably harm existing front ends or clients.