apply timeout correctly on android

[broglep-work]

Also set connectTimeout. Without this, the default timeout of 0 is in effect which causes request to fail after a long time

[jstevej]

I had the same issue, and this fix worked for me. Here's my fork in case it is useful to anyone while we wait for this pull request to be accepted.

https://github.com/jstevej/cordova-plugin-advanced-http

[silkimen]

Hi Pascal, the setRequestTimeout method is meant to set:

• the "readTimeout" on Android, see https://developer.android.com/reference/java/net/URLConnection#setReadTimeout(int)
• and the "timeoutIntervalForRequest" on iOS, see https://developer.apple.com/documentation/foundation/nsurlsessionconfiguration/1408259-timeoutintervalforrequest?language=objc

So, it's behaving correctly. Basically, you're asking for a new feature to apply the "connect" timeout, right?
But I understand that it's confusing, because the documentation isn't clear about that. I'll need to fix this in the docu.

[varadig]

What about this issue?
I use the plugin to scan the network from .1 to .255 and if the response is valuable, I use them.
On iOS the scan is working pretty well, but the same code on Android is wait a long time, while try the next ip address.

The relevant code:

    cordova.plugin.http.sendRequest(
      "http://" + ips + ip + "/GetValue",
      { method: "get", timeout: 0.03 },

      (response) => {
        var result = response.data;
     
        } else {
           //retry with the next ip address
        }
      },
      (error) => {
        console.log("scan error on http://" + ips + ip + "/GetValue:" + error);

        if (ip > 255) {
          //scan is done;
        } else {
          //retry with the next ip address
        }
      }
    );

[YouYue123]

I am facing the same issue. It it because the inconsistency between how Android and iOS handling the initial network connection in this library.

In iOS side, I found you are using NSMutableURLRequest as underlining request class. The timeout from cordova interface is set to timeoutinterval property.

From Apple's document this property seems for both connect timeout and read timeout from URLConnection's point of view. The documentation is not very clear to me. But from my experiment iOS works in this way.

That means for this library iOS actually handles connect timeout, but Android dose not.

How about making 2 API's. One is for read timeout. Another one is for connect timeout.

But the separations only works for Android. iOS will use connect timeout as a universal timeout. I did not found an equivalent API from iOS for read timeout and connect timeout separately.

@silkimen What's ur idea to do it? I could make a PR for it

[YouYue123]

FYI I found this Stackoverflow answer.

It seems that NSMutableURLRequest timeoutinterval is neither URLConnection's read timeout nor connect timeout.

It is just an interval timer to cancel request if there is no data event during a period. From this point of view, I think timeoutinterval is more like setting both read timeout and connect timeout in iOS

[silkimen]

Hey guys, sorry for (very) late response. 😓 I've read the Apple documents again and also the SO answer. Seems like I'm misunderstanding the API description. 🤔

@YouYue123 that behavior you're describing is actually what I expected to be a "read timeout", but exclusively for data events AFTER having established the initial connection. Initial connection (handshake and establishing socket) should be handled by "connect timeout".

So you're right Android and iOS do implement different timeout concepts. But I'm not sure if it's a good idea to change the API to set the connect timeout implicitly on Android without offering other options. This is a breaking change!

I'm increasing this value in some cases because I'm expecting that the server needs more time to process the request. But I do not want to increase the connect timeout (which is 60 seconds on Android AFAIK).

So I do see following two options to fix that:

1. add one new method setConnectTimeout() (only for Android) and do not change setRequestTimeout() --> no breaking change, but will will potentially lead to misunderstandings
2. add two new methods setConnectTimeout() and setReadTimeout() (both only for Android) and change setRequestTimeout() to set both values for Android --> breaking change, but feels more correct

What do you guys think?

[YouYue123]

@silkimen I prefer the 2nd option. Simply because it can help to eliminate unexpected understanding of the API behaviours. We might need to semver it to indicate the breaking changes. If this also sounds good to u I can make the change quickly

How's ur thinking

[silkimen]

@YouYue123 I do also prefer the 2nd option. I'd appreciate if you update this PR.

[YouYue123]

@silkimen Okie working on it. Will update in this week

[YouYue123]

I realized I don't have permission to modify this PR. Created a new one