Show error messages when connections fail

[mediaminister]

Trying to provide error messages in Kodi GUI for all network problems improving #385

[dagwieers]

I am not convinced we want to add a failmessage to the URL-accessing function, it somehow feels too limited or unneeded. So (without having looked at what would be needed to cover this, enlighten me ;-)). We probably can describe 2 kind of issues:

• Issues that does not require specific context (all kinds of connection issues, potentially some HTTP errors, like 401)
• Issues that require specific error messages because they provide context (and potentially resolution)

For the first set of issues, I think we can get away with generic error handling and messages. The cause/resolution can also be generally be applied. This would either be caught using exception-handling, or testing HTTP error code within the calling function.

For the second set of issues, usually HTTP errors, there may be different courses of action:

• It may require (a slightly different) retry or alternative workaround to continue without escalating to the user
• There could be different errors requiring different resolution/message, so a single failmessage feels too limited

Could we instead raise HTTP errors as exceptions, and leave the handling/error message up to the exception-handling? This would allow both actions or escalation to the user. The interface could then simply consist of a list of HTTP errors that are caught in the calling code, and our URL-handling code would simply fall back to generic handling unless told otherwise.

So instead of failmessage= we could have raise_errors=[400, 413, 415, 431] and those would then be exposed as CustomHTTPError(XXX).

[mediaminister]

I removed failmessage and implemented the raise_errors list. Raising the HTTPError again if the exc.code is in the raise_errors list works fine. I think there is no need for a CustomHTTPError.

Currently the raise_errors list is only used in Tokenresolver._get_fresh_token

[dagwieers]

Raising the HTTPError again if the exc.code is in the raise_errors list works fine. I think there is no need for a CustomHTTPError.

I wasn't sure if a custom exception would be easier to catch and select the exact error caught in a single catch-statement. So we could have a catch-statement per error-case. But that would require a custom exception per HTTP error I guess...

[dagwieers]

I like this implementation. The connection errors I would like to make more specific (we have quite a few specific ones listed in #385), which would mean the error messages could give more context. But merging this as a first version would be perfect.

Once we release this as part of v2.4.0 I think we need to ask our users to report any generic add-on errors (those that are not caught) or send us feedback on issues that do not match with the error statement so we can further refine error handling.

Another approach would be to handle any uncaught exception from the root and provide a generic error to report this error to us with full debug log. I am not sure if that is a good approach in general for the majority of our users. Some add-ons (e.g. Netflix) have this as an option.

[dagwieers]

Thinking about this further, having this part of a more common framework for all add-ons to reuse would be really useful too.

cc @michaelarnauts

[mediaminister]

But merging this as a first version would be perfect.

Feel free to merge this. I tested this and it works fine. Kodi add-on errors shouldn't happen anymore. Refining the error messages is something we can do one by one as they happen.