BenFradet / RiotSharp

C# wrapper for the Riot Games API
http://benfradet.github.io/RiotSharp/
MIT License
300 stars 145 forks source link

Null Reference Exception #109

Closed Eniplay closed 9 years ago

Eniplay commented 9 years ago

Hello,

I have a problem. SOMETIMES, when i start a request my programm gives back an error message. Here is the code:

        var api = RiotApi.GetInstance("KEY");
                var summoner = api.GetSummoner(RiotSharp.Region.euw, name);//Here is the Error

                string sum_lv = summoner.Level.ToString();
                string sum_pp = summoner.ProfileIconId.ToString();

                pictureBox1.ImageLocation = "http://ddragon.leagueoflegends.com/cdn/5.2.2/img/profileicon/" + sum_pp + ".png";
                pictureBox1.Visible = true;
                textBox2.Text = sum_lv;

The Error tells me this:"A first chance exception of type 'System.NullReferenceException' occurred in RiotSharp.dll". What is the problem? Thanks for help :) ~Hermann

BenFradet commented 9 years ago

Hey, do you know on which name the exception occurs?

Eniplay commented 9 years ago

i get it from a textbox. you write the summoner name in there, press the button and that whats in this textbox, will be now in the string name.

BenFradet commented 9 years ago

Yeah I gathered that, but don't you have any example name raising an exception?

Eniplay commented 9 years ago

well sometimes, when i start the programm, enter a name and press the button i get All informations. no problem. but next time i start the programm, exacctly this problem occurs. It doesnt matter if i write instead of "name" the summoner name itself.

StevenReitsma commented 9 years ago

This might be due to a WebException in Requester.GetResponse. I believe this has to do with the fact that the HTTP service on EUW is very unstable. While I'm writing this the EUW API HTTP service is offline while the HTTPS API is functioning normally.

I'm not sure where https://github.com/BenFradet/RiotSharp/commit/2710dfba19ec104c6034d2a0c969b84d48b7b1e7 went, but it might be reasonable to merge this into master.

AbhayaGH commented 9 years ago

Hi there,

I got this 'System.NullReferenceException' when server refused the connection. Replacing 'http' by 'https' solved the problem. However, in the requester, the 'HandleWebException' throws this nullreference when this is not a WebException but a SocketException one.

I think a test if 'response' is not null before analysing 'HttpWebResponse.StatusCode' could send the original exception instead of the null one.

Sorry if i don't propose a patch but i just created my account to post here and i'm not confident with GitHub so i wouldn't like to mess your code... ^^

Eniplay commented 9 years ago

Hey, So i should include an exception, if

var summoner = api.GetSummoner(RiotSharp.Region.euw, name);

gives back an null value?

Eniplay commented 9 years ago

well, now it shows me this message:

503

StevenReitsma commented 9 years ago

@AbhayaGH if the server refuses a connection the GetResponse method will still throw a WebException, not a SocketException. Although I still agree that there is some sort of uncaught NullReferenceException somewhere where it shouldn't be. @Eniplay the EUW API is still offline, you can see this on the Riot status page. This has nothing to do with RiotSharp. But yes, putting a try-catch block around GetSummoner is what you want to do anyway. This is by design and not a bug.

AbhayaGH commented 9 years ago

@StevenReitsma You are right and i was not clear enough, GetResponse throw a WebException if server refuses to connect. However, in this case, the WebException.Response is null. So when the HandleWebException method tries to extract the WebException.Response.StatusCode, it throws a NullReferenceException. The WebException has in this case an InnerException of type SocketException.

@Eniplay You can use the Riot API web page to test the request (you will get JSON response in return) and then found out where does the problem come from.

StevenReitsma commented 9 years ago

Ah! That makes sense. I'll try and see what I can do.

EDIT: This should work right?

[...]
HttpWebResponse response = (HttpWebResponse)ex.Response;

if (response == null)
    throw new RiotSharpException("Server did not send a response", ex);

switch (response.StatusCode)
{
[...]
AbhayaGH commented 9 years ago

Looks good i did something like this in the version i have, in case exception is not connection refused:

[...] HttpWebResponse response = (HttpWebResponse)ex.Response;

if (response == null) throw new RiotSharpException(ex.Message, ex);

switch (response.StatusCode) { [...]

BenFradet commented 9 years ago

Yup, I'll try to fix that in master during the week

h1pp0 commented 9 years ago

Hello,

I just started using this wonder framework today and I noticed there is an issue with the NA region url.

The current implementation sends the requests over http. However, all http requests to the NA server over http fail with "Unable to connect to the remote server".

Changing the http to https in line 40 and 45 of Requester.cs fixes the issue for NA region.

Edit: I'm a bit late, pull request #111 has already been posted to fix this issue

BenFradet commented 9 years ago

Hey, see #111.

BenFradet commented 9 years ago

The nuget is updated.