LanguagesClient#GetAllAsync throws deserialization error when json records have null values

[shayaantx]

Hi,

I saw https://github.com/jellyfin/jellyfin/issues/3201 and wanted to see if I could put up a fix for this. Specifically a fix that would ignore any languages that have missing id fields. I'm not sure if these is the "right" fix, but it seems correct to ignore a json record completely if we cannot deseralize it, instead of just failing to deserialize all the records (see below exception thrown).

I didn't see a way to ignore deserializing records without writing a custom json deserializer, so I figured something like below would be ideal (plus modifying the JsonObject properties on LanguageDtos) and making Id property optional.

If this suggest fix is alright, let me know, and I can put up a PR.

public async Task<TvDbResponse<Language[]>> GetAllAsync(CancellationToken cancellationToken)
{
    var request = new ApiRequest("GET", "/languages");
    var response = await this.ApiClient.SendRequestAsync(request, cancellationToken).ConfigureAwait(false);
    TvDbResponse<Language[]> parsedResponse = this.Parser.Parse<TvDbResponse<Language[]>>(response, ErrorMessages.Languages.GetAllAsync);
    if (parsedResponse.Data != null)
    {
        parsedResponse.Data = Array.FindAll(parsedResponse.Data, language => language.Id != null);
    }
    return parsedResponse;
}

[2020-06-03 09:27:41.045 -04:00] [ERR] [97] ProviderManager: "TvdbSeriesImageProvider" failed in GetImageInfos for type "Series"
System.AggregateException: One or more errors occurred. (Error converting value {null} to type 'System.Int32'. Path 'data[180].id', line 1, position 13279.)
 ---> Newtonsoft.Json.JsonSerializationException: Error converting value {null} to type 'System.Int32'. Path 'data[180].id', line 1, position 13279.
 ---> System.InvalidCastException: Null object cannot be converted to a value type.
   at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   --- End of inner exception stack trace ---
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureType(JsonReader reader, Object value, CultureInfo culture, JsonContract contract, Type targetType)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateList(IList list, JsonReader reader, JsonArrayContract contract, JsonProperty containerProperty, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateList(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, Object existingValue, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at TvDbSharper.Clients.LanguagesClient.GetAllAsync(CancellationToken cancellationToken)
   at MediaBrowser.Providers.TV.TheTVDB.TvDbClientManager.TryGetValue[T](String key, String language, Func`1 resultFactory)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at MediaBrowser.Providers.TV.TheTVDB.TvdbSeriesImageProvider.GetImages(Image[] images, String preferredLanguage)
   at MediaBrowser.Providers.TV.TheTVDB.TvdbSeriesImageProvider.GetImages(BaseItem item, CancellationToken cancellationToken)
   at MediaBrowser.Providers.Manager.ProviderManager.GetImages(BaseItem item, CancellationToken cancellationToken, IRemoteImageProvider provider, List`1 preferredLanguages, Nullable`1 type)

[HristoKolev]

Hello @shayaantx, thank you for letting me know about this issue.

Here is the thing, I don't want to go on a rant here but this is just absurd.

thetvdb api v2/3 is possibly the worst HTTP API that I have used. It's even worse than their old xmlrpc thing. I don't know if anyone is getting paid to develop and support it but even if it's a side project for someone - it's just really poorly implemented.

Having the Id field of the language objects be null? Really? This is a bug on their end and they should fix it. I don't want to make the field nullable, this is stupid.

With that said, I have always known what I'm working with and every time they do something stupid I have implemented a workaround. The best thing to do here is to make Language.Id nullable.

I don't see a reason to ignore any language data. This library should not manipulate any data, the intent is to have a thin C# wrapper around the HTTP Api.

I'll deal with this tomorrow, if you have any more thoughts about this, please share.

[shayaantx]

@HristoKolev totally understand, I wasn't really sure what would be the desired fix here (just started using jellyfin, noticed this problem, and thought about attempting a fix) . I agree the api should not be returning null on fields like "id" (which should never be null in general) and this should be something they address.

I don't see a problem with just making it nullable, I used the approach in the PR instead because I don't really know how a null id within the Language record will be interpreted by jellyfin, so I opted to just ignore invalid records instead.

If I can be of any assistance let me know

Thanks

[isaksamsten]

According to this the issue will be fixed on tvdb:s end. Something about ignoring languages without legacy id. But who knows when.

[HristoKolev]

"fixed" in v3.2.0.