# Decided to update HttpWebRequest to HttpClient

I updated my Instagram API and I read that HttpClient is now the preferred way in .NET Core 3.1. I would like to get a code review, because there are too many examples and too many ways and I'm not quite sure if I did the migration correctly.

One more question: The reason I added _client.Timeout is in case the user is using proxy and it is not responding. I think that it's needed only on the first request. Can I somehow restrict it to the first request? Or a better way?

New code:

public class InstagramAPI
{
private static HttpClient _client;

public string Username { get; private set; }
public string Password { get; private set; }

{
if (_client == null)
{
var handler = new HttpClientHandler();

if (proxy != null)
{
handler.Proxy = new WebProxy(proxy, true);
}

if (handler.SupportsAutomaticDecompression)
{
handler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;
}

_client = new HttpClient(handler)
{
Timeout = TimeSpan.FromSeconds(10)
};
}

throw new ArgumentException("Username cannot be empty or null.");

throw new ArgumentException("Password cannot be empty or null.");

}

{
try
{
// 1st request
HttpRequestMessage httpRequest = new HttpRequestMessage(HttpMethod.Get, "https://www.instagram.com/");

var getsResponse = _client.SendAsync(httpRequest);

string web_key_id = Regex.Match(getResult, "\"key_id\":\"([^\"]*)").Groups[1].Value;
string web_pub_key = Regex.Match(getResult, "\"public_key\":\"([^\"]*)").Groups[1].Value;
string web_key_version = Regex.Match(getResult, "\"version\":\"([^\"]*)").Groups[1].Value;
string csrfToken = Regex.Match(getResult, "\"csrf_token\":\"([^\"]*)").Groups[1].Value;
string rolloutHash = Regex.Match(getResult, "\"rollout_hash\":\"([^\"]*)").Groups[1].Value;

// 2nd request
HttpRequestMessage httpRequest2 = new HttpRequestMessage(HttpMethod.Post, "https://www.instagram.com/accounts/login/ajax/");

httpRequest2.Content = new FormUrlEncodedContent(new[]
{
new KeyValuePair<string, string>("queryParams", "{}"),
new KeyValuePair<string, string>("optIntoOneTap", "false")
});

var postResponse = _client.SendAsync(httpRequest2);

...
}
catch (AggregateException ex)
{
{
// return proxy error
MessageBox.Show("We could not connect. It could a proxy problem, if you were using one.", "Connection error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}
}
catch (Exception ex)
{
MessageBox.Show(ex.Message, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}
}
}


Old code:

void Login(string postData)
{
// 1st request
HttpWebRequest request = WebRequest.Create("https://www.instagram.com/") as HttpWebRequest;
request.Method = "GET";
request.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;

{
}
else
{
// The first request takes an abnormally long time: This occurs because WebRequest by default detects and loads proxy settings the first time it starts,
// which can take quite a while.To stop this, simply set the proxy property (WebRequest.Proxy) to null and it'll bypass the check
request.Proxy = null;
}

request.Host = "www.instagram.com";
request.UserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0";
request.Accept = "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8";

IAsyncResult response = request.BeginGetResponse(new AsyncCallback(ReadCallback), request);

// this line implements the timeout, if there is a timeout, the callback fires and the request becomes aborted
ThreadPool.RegisterWaitForSingleObject(response.AsyncWaitHandle, new WaitOrTimerCallback(TimeoutCallback), request, 2000, true);

// The response came in the allowed time. The work processing will happen in the callback function
_manualResetEvent.WaitOne();

if (RequestResult == null)
{
return ResultType.ProxyError;
}

string web_key_id = Regex.Match(RequestResult, "\"key_id\":\"([^\"]*)").Groups[1].Value;
string web_pub_key = Regex.Match(RequestResult, "\"public_key\":\"([^\"]*)").Groups[1].Value;
string web_key_version = Regex.Match(RequestResult, "\"version\":\"([^\"]*)").Groups[1].Value;
string csrfToken = Regex.Match(RequestResult, "\"csrf_token\":\"([^\"]*)").Groups[1].Value;
string rolloutHash = Regex.Match(RequestResult, "\"rollout_hash\":\"([^\"]*)").Groups[1].Value;

// 2nd request

byte[] data = Encoding.UTF8.GetBytes(postData);

HttpWebRequest request2 = WebRequest.Create("https://www.instagram.com/accounts/login/ajax/") as HttpWebRequest;
request2.Method = "POST";
request2.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;

{
}
else
{
request2.Proxy = null;
}

request2.Host = "www.instagram.com";
request2.UserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0";
request2.Accept = "*/*";
request2.ContentType = "application/x-www-form-urlencoded";
request2.Referer = "https://www.instagram.com/";

request2.ContentLength = data.Length;

try
{
using (Stream stream = request2.GetRequestStream())
{
stream.Write(data, 0, data.Length);
}

using (HttpWebResponse response2 = request2.GetResponseWithoutException() as HttpWebResponse)
{
{
}
}
}
catch (WebException e)
{
if (e.InnerException is SocketException)
{
return ResultType.ProxyError;
}

return ResultType.ProxyError;
}

...
}

private ManualResetEvent _manualResetEvent = new ManualResetEvent(false);

public string RequestResult { get; private set; }

private void TimeoutCallback(object state, bool timedOut)
{
if (timedOut)
{
HttpWebRequest request = (HttpWebRequest)state;
if (request != null)
{
request.Abort();
}
}
}

{
HttpWebRequest request = (HttpWebRequest)asyncResult.AsyncState;

try
{
using (HttpWebResponse response = (HttpWebResponse)request.EndGetResponse(asyncResult))
{
{
}
}
}
catch
{
}

_manualResetEvent.Set();
}
$$$$


var postResponse = _client.SendAsync(httpRequest2);


Never use tasks like this, it leads to dead-locks very easily. Plus it's wasteful, you have an entire framework (HttpClient) designed to keep your application responsive, and instead you freeze it for no good reason.

1. Make your Login function async and have it return a Task<bool> (or even better, an enum that describes exactly why it failed if it did)
2. Properly await those async calls (SendAsync and ReadAsStringAsync)
3. Change Login to LoginAsync to follow the framework naming conventions
4. Get rid of those random MessageBox calls in your class, they do nothing but prevent unit testing and introduce WinForms dependencies in what could otherwise be a nice service class.
• First of all, thank you for your answer! I actually did the await part right after I wrote the question. Fair point about the MessageBox's. Updated the code in the question, thanks again and I'm open for more corrections! – nop Jul 28 '20 at 18:20
• You still have a postResponse.Content.ReadAsStringAsync().Result! – Blindy Jul 28 '20 at 18:22
• Also instead of doing those repeated and error-prone Contains calls, decode the response (I'm assuming it's JSON) and read the structured properties directly. – Blindy Jul 28 '20 at 18:25
• No, I mean use JsonConverter.Deserialize<T>(jsonString), where T is a class you write that contains the fields you're interested in from the response (like string Message; etc), and then use the class to determine the return value instead of doing string operations. – Blindy Jul 28 '20 at 18:52
• And if you're not familiar with JSON parsing, make sure you define a clear enum for the status, don't parse it as a string, that will lead to more if..else if..else if.. – Blindy Jul 28 '20 at 18:54

I don't want to repeat Blindly's observations (and those that have spotted under the comment section).

I would like to suggest other improvement areas:

## Constructor

You are using instance constructor to initialize static member.

• Static constructor should be used to initialize your HttpClient with(out) proxy
• There is no need to use instance constructor
• UserName and Password should be a parameter of the Login

It does seems to me that you want to implement something on the server-side which was designed to support client-side authentication.

• Please check that does Instagram support server-side auth

This error handling is really naive and thin.

• You should definitely look for HttpRequestException.
• You should check the responses' StatusCode before any further processing

Separation helps the understanding and maintainability of a function. Smaller chunks can be easier tested, etc.

• I suggest to separate the two separate requests into two separate functions
• Your Login should be the coordinator for the two functions

You are using a lot of strings to setup requests. Most of them are duplicated.

• Consider to set the Host on the HttpClient level instead of on the HttpWebRequest level
• @nop 1) It does not matter from where the status code is coming. It is the primary indicator of the request processing successfulness. For example if it is 5xx then you shouldn't looking for key_id in the response body. 2) SRP is a kind a good guidance for every length function :D 3) If you need to reuse these constants across multiple classes then move them into a separate class and mark it as static. You don't have to make it as singleton. – Peter Csala Jul 29 '20 at 10:20
• Fair point. Btw, is it only me who cannot find a way to set up proxy only for HttpRequestMessage instead of the whole HttpClient object? In case it is like that, I will probably have to move the HttpClient instance into the LoginAsync method. – nop Jul 29 '20 at 10:33
• What are the advantages of using Polly to set the timeout instead of simply using the property? Polly is nice if you're using the more advanced features (at which point an argument could be made to be consistent and use it for everything), but in this case it's overkill with no benefits. Sidenote: Static constructors are pretty awful from a performance PoV so if one can avoid it, that's a good idea - just use Lazy` instead. – Voo Jul 29 '20 at 12:20