search

nativescript-community / https

Secure HTTP client with SSL pinning for Nativescript - iOS/Android
https://nativescript-community.github.io/https/
Other
51 stars 42 forks source link

Allow for variable timeout #49

Closed bobbyngwu closed 4 years ago

bobbyngwu commented 4 years ago

This simply allows for a configurable timeout to be applied instead of using the default 10 seconds

EddyVerbruggen commented 4 years ago

Looks good, thank you!

I'll add it to the docs as well.

EddyVerbruggen commented 4 years ago

The optional timeout property now expects a number of seconds.

Also changed the Android implementation a little, because the original PR interfered with SSL pinning.

bobbyngwu commented 4 years ago

Hi Eddy, thought about that implementation but then figured out that the Client variable will always be returned if already set.

However, going by the new changes you have now made, the getClient() in the request function https.android.ts: 121 does not pass a timeout and as such the 10 seconds will always apply regardless of whatever timeout anyone has set, or have I missed something?

EddyVerbruggen commented 4 years ago

@bobbyngwu The timeout is applied at https://github.com/EddyVerbruggen/nativescript-https/blob/ea44ec0b07bb45c1434dd4159cec72a35cc4a6ea/src/https.android.ts#L144-L149 and the client instance is returned afterward. Or do I miss something?

bobbyngwu commented 4 years ago

Yes, except for the fact that line 158 where the getClient() function is called does not pass the httpsRequestOption timeout value which invariably means that the default 10 seconds will always be set?

EddyVerbruggen commented 4 years ago

I don't really know how this slipped through my fingers, but in 1.2.1 that's now corrected.

Do you have a good test environment for this? I looked at httpbin.org for instance, but didn't immediately see a "response delay" feature.

bobbyngwu commented 4 years ago

Thanks Eddy, will have a look, thanks for all your hard work on this. I do have a local test environment and I am also throttling as well with an emulator, that was how I spotted it in the first place :)