Better error handling in new API

[ogarcia]

At this moment every error given by *sonic server are showed in Ultrasonic as a A network error occurred. Please check the server address or try again later.

This is insufficient in some cases where the server are providing an error message to client. For example, Supysonic doesn't support video, and when you try to access to video api the server gives the following message.

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "message": "Video streaming not supported",
   "code": 0
  }
 }
}

The error messages are always equal, the status value is failed and it gives a message and an error code. I think that Ultrasonic must support this messages and show them to user to avoid false positive bugs.

Some samples:

getGenres is unsupported in Supysonic.

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "message": "Not implemented",
   "code": 0
  }
 }
}

A user/password error.

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "message": "Unauthorized",
   "code": 40
  }
 }
}

[Tapchicoma]

Added more specific messages for different error codes.

As for generic code (0) messages - I think that it is better to not rely on them and just show API generic error message, as different API implementations can implement them differently. Also it solves the problem of this messages localization.

Feel free to reopen if you think this behaviour should be different.

[ogarcia]

Show generic error message is a good idea, but I consider that if the server gives a message is a good idea show them. Some like this:

Generic api error. <- Translated String
Server string in "message" if any

Or adding a new string Server message: translated:

Generic api error. <- Translated String
Server message: Server string in "message" if any

Obviously only show the message if server provides message string. I think that this may help to user to know why the server is failing.

I think that this is a good idea for the code 0 and for unknown_api_error. What do you think?

[Tapchicoma]

R.string.unknown_api_error is fallback message if api introduce new error code and app will not yet support it.

For generic api error app can show following: Generic api error: <server-message>. In this case Generic api error: will be localized and server-message will be shown as is.

[Tapchicoma]

Added showing generic error message.

[ogarcia]

@Tapchicoma I find an issue with this in the SubsonicErrorDeserializer class.

If the server returns some like this:

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "message": "Video streaming not supported",
   "code": 0
  }
 }
}

The deserialize function picks as message text the code becase it waits give the code before message. This supposition is incorrect because in json the pairs "key": "value" can be in any order.

In fact, is equal valid this:

{
  "subsonic-response": {
    "error": {
      "message": "Video streaming not supported",
      "code": 0
    },
    "version": "1.8.0",
    "status": "failed"
  }
}

Or this:

{
  "subsonic-response": {
    "version": "1.8.0",
    "error": {
      "message": "Video streaming not supported",
      "code": 0
    },
    "status": "failed"
  }
}

[ogarcia]

And I think that we must manage responses without message like this:

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "code": 0
  }
 }
}

Or with blank message like this:

{
 "subsonic-response": {
  "status": "failed",
  "version": "1.8.0",
  "error": {
   "message": "",
   "code": 0
  }
 }
}

In this cases, I think that we must response: Generic api error: no message given from server and no message given from server can be a translatable string.

[Tapchicoma]

Fixed raised issues.