michidk / TwitchCSharp

Twitch C# Wrapper for the Twitch v3 REST API
Apache License 2.0
43 stars 12 forks source link

GetEmoticons() wrong MIME response #9

Closed erictuvesson closed 8 years ago

erictuvesson commented 8 years ago
public TwitchList<Emoticon> GetEmoticons()
{
    var request = GetRequest("chat/emoticons", Method.GET);
    var response = restClient.Execute<TwitchList<Emoticon>>(request);
    return response.Data;
}

The response's ContentType is "text/html" which causes the data to not be parsed, a simple fix to this would be to change the request uri to "chat/emoticon_images".

michidk commented 8 years ago

Thanks! Did you test your fix?

erictuvesson commented 8 years ago

@michidk Yepp, I did in a local git sub module.

Edit: The problem with changing this is that the data is different. I couldn't find a way to force override the rest respons MIME type, is there a way to do this? If not I suppose we could download the data and manually parse it.

michidk commented 8 years ago

@ChillyFlashER Could you please create a pull request?

michidk commented 8 years ago

Oh.. I see... Well there is a way to set the mime type for a request... maybe that works.. (http://stackoverflow.com/questions/17815065/set-content-type-header-using-restsharp)

erictuvesson commented 8 years ago

I tried that earlier and now again just to be sure, but it still responds with a MIME type of "text/html".

michidk commented 8 years ago

text/html as content type should be fine, and the response should be parsed. Which mime type do you expect?

Oh and I just saw that chat/emoticon_images is not implemented, correct? Or did I just overlook it?

erictuvesson commented 8 years ago

The expected response should be has a type of application/json, but it's text/html. Because the response is text/html the rest parser wont be triggered.

In the PR I download the data and converts it to a string then parse it.

Edit: Yes, I did not change it to emoticon_images.

michidk commented 8 years ago

Oh yeah, totally forgot about the json mime type.. but as far as I remember it should work with text/html, too... thats weird.

So all other rest-api-responses return the correct json mime type but this one? This would be a bug at the twitch rest server.

I found two ways to fix that issue (beside yours): First one: request.OnBeforeDeserialization = resp => { resp.ContentType = "application/json"; }; Before executing the request, add this callback to change the content type right before parsing the data.

The second one: client.AddHandler("text/plain", new DynamicJsonDeserializer()); Add a handler to also parse the text/plain mime type as json. (Should be added here: https://github.com/michidk/TwitchCSharp/blob/ef4fc9a08bbd220dd3e2b9f7a25275d0cdbb5179/TwitchCSharp/Clients/TwitchReadOnlyClient.cs#L16)

I prefer the second one. If you want to, you can create another PR.

Edit: I didn't talk about your change with "emoticon_images". I meant, that i noticed, that I forgot to add it to TwitchCSharp at all.

michidk commented 8 years ago

Fixed: https://github.com/michidk/TwitchCSharp/commit/db71e00ac1c3e2fc9ff114f590f6680e1def098e