Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lots of exceptions - SocketException : The requested address is not valid in this context #174

Closed
firatgs opened this issue Oct 13, 2023 · 8 comments

Comments

@firatgs
Copy link

firatgs commented Oct 13, 2023

Hello.

We are experimenting BestHttp 2.0 plugin for 2 weeks now. Some users has been experiencing this exception too many times. When I go trough the Crashlytics logs, I can see that most of the Http Requests has been delivered as expected. But after sometimes, this exception occurs rest of the session. Btw, it is a handled Exception. We just log the exceptions over the Crashlytics. The issue here is that we should not be getting this.

Non-fatal Exception: java.lang.Exception: SocketException : The requested address is not valid in this context
       at BestHTTP.PlatformSupport.TcpClient.General.TcpClient.Connect(BestHTTP.PlatformSupport.TcpClient.General.TcpClient)
       at BestHTTP.Connections.TCPConnector.Connect(BestHTTP.Connections.TCPConnector)
       at BestHTTP.Connections.HTTPConnection.ThreadFunc(BestHTTP.Connections.HTTPConnection)
       at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext)
       at XXX
       at XXX
       at BestHTTP.Core.RequestEventHelper:HandleRequestStateChange(BestHTTP.Core)
       at BestHTTP.Core.RequestEventHelper:ProcessQueue(BestHTTP.Core)
       at BestHTTP.HTTPManager:OnUpdate(BestHTTP)
       at BestHTTP.HTTPUpdateDelegator:CallOnUpdate(BestHTTP)

I'm able to backtrace the issue to this line in TcpClient.cs:

                    if (address.Equals(IPAddress.Any) ||
                        address.Equals(IPAddress.IPv6Any))
                    {
                        throw new SocketException((int)SocketError.AddressNotAvailable);
                    }

It means, this this line returns invalid address after sometimes.

IPAddress[] addresses = Dns.GetHostAddresses(hostname);

So, I would suggest to cache IPAddress in here, for a session after a successful DNS resolve. This way, problem would significantly reduce.

My environment is:

Unity 2021.3.X LTS
Android

Devices OS
@Benedicht
Copy link
Owner

The version you use is exactly 2.0?

if (address.Equals(IPAddress.Any) || address.Equals(IPAddress.IPv6Any))
{
    throw new SocketException((int)SocketError.AddressNotAvailable);
}

I'm really curious in what cases the DNS query returns with such an address. Anyway, i think it shouldn't throw an exception there as it's already iterating through a (hopefully) set of addresses, so there might be valid ones.
It should throw one after the for loop, if it couldn't find any working one.


The next major version has a DNS cache (with the ability to prefetch ) paired with a very different connection logic to improve both connection speed and reliability.

@firatgs
Copy link
Author

firatgs commented Oct 16, 2023

The next major version has a DNS cache (with the ability to prefetch ) paired with a very different connection logic to improve both connection speed and reliability.

Thank you for the quick response and that is good to hear.

The version you use is exactly 2.0?

No. We are using latest store version, 2.8.5.

I'm really curious in what cases the DNS query returns with such an address. Anyway, i think it shouldn't throw an exception there as it's already iterating through a (hopefully) set of addresses, so there might be valid ones.
It should throw one after the for loop, if it couldn't find any working one.

yeah. I have access to Unity source code and deeply checked about what is going on. It is using basic low-level C code and it seems it is doing its job. The weird thing is that. it works for sometime but it is starting fail after. To be honest, I don't know what is going on. But we are familiar this kind of issues when it is come to the Android and Unity combination.

@firatgs
Copy link
Author

firatgs commented Oct 16, 2023

and I wanted give you a feedback about the current state of API. We were using Unity API starting WWW and finally WebWebRequest. Our entire code base depends on the API returned error and not throwing exceptions.

I mean, if there is any error, then Unity API simply returns an error with a field in response and we are used to that.

Now, using BestHttp, even simple "Cannot resolve of host" error comes with an Exception while calling Request() or with an extra field as Exception in the Response. I know that, BestHttp uses TCP Sockets to achieve Http 2.0 RFC. But as a Unity developer for a long time like us, if there is an Exception, then it feels like something very wrong. On our first experiment with BestHttp, filled handled exceptions but logged to Crashlytics similar to this. We did not quite understand some of that about what is really going on. And then we put extra log data to understand the root causes. Finally, we have come to a decision that, we can handle these errors and simply ignore most because they are simply connectivity issues. We are trying to be careful because we have a really large amount of players.

In the end, my feedback is that Exceptions are scary. Exceptions should be provided since this is a High Level API but it should be in a field like "ErrorDetails" or something. Connectivity errors should be presented as string in Responses. This way, we'll be more comfortable.

@Benedicht
Copy link
Owner

Thanks, feedback is always welcome!

However, I'm not sure I understand it right. The plugin catches/handles those exceptions, there should be no unhandled ones and Crashlytics still logs them?
The plugin stores the exception after catching them, they aren't unhandled. I'm working with Unity about 13 years now, my experience with WWW inspired me to make the plugin 10 years ago, but this is the first time I read about this and I'm quite surprised.
I don't have access to application logs for the games i worked on, but i guess i would have received back some feedback.

Nonetheless, the Exception field is there only to help debugging, it could be a string too, but would it make go away these logs? I'm very puzzled.

Is Request() you mentioned calling something you implemented, something that you implemented? The plugin doesn't have such API. Also, the Exception isn't located in the response object. Maybe are you using a wrapper or something?

@firatgs
Copy link
Author

firatgs commented Oct 17, 2023

Hello, sorry for confusion. My english is not good, so, let me explain in code:

var request = new HTTPRequest(uri, HTTPMethods.Post, (originalRequest, response) =>
{
    switch (originalRequest.State)
    {
        case HTTPRequestStates.Finished:
            OnSuccess.Invoke(response);
            break;

        default:
            if (originalRequest.Exception != null)
            {
                Debug.LogException(originalRequest.Exception);
                OnFailure.Invoke(originalRequest.Exception.Message);
            }
            else
            {
                // a generic error
                OnFailure.Invoke($"An error occured in state: {originalRequest.State}");
            }
            
            break;
    }
});

request.IsKeepAlive = true;
request.DisableCache = true;
request.RawData = postData;
request.Send();

As you can see, we look for an Exception field in Request object when Http State changes. We are logging the exception because Exceptions feels like a severe event happened.

For our experience, it could have been like this.


var request = new HTTPRequest(uri, HTTPMethods.Post, (originalRequest, response) =>
{
    switch (originalRequest.State)
    {
        case HTTPRequestStates.Finished:
            OnSuccess.Invoke(response);
            break;
        
        default:
            if (response.Error != null)
            {
                // simply response resulted in error
                // get connectivity errors in a simple string
                OnFailure.Invoke(response.Error.Message);

                // at this point we'll know, this is only related to connectivity issues.
                // if we still interested then we can also get the exception such as DNS resolve errors, Socket Exceptions etc.
                // Debug.LogException(response.Error.ExceptionDetail);
            }
            else
            {
                // a generic error
                OnFailure.Invoke($"An error occured in state: {originalRequest.State}");
            }

            break;
    }
});

request.IsKeepAlive = true;
request.DisableCache = true;
request.RawData = postData;
request.Send();

@Benedicht
Copy link
Owner

Is the exception in your opening post logged the same way?

@firatgs
Copy link
Author

firatgs commented Oct 19, 2023

We have a little more in our implementation but yes. We are handling all exceptions to be sure not to break anything.

@Benedicht
Copy link
Owner

Closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants