patterncsharpMajor
IsInternetAvailable
Viewed 0 times
isinternetavailablestackoverflowprogramming
Problem
I've written the following code to determine if an Internet connection is available in mind of this article at Microsoft's technet.
In short, there are two methods of prove:
Is there anything I can improve or is it good as it stands now?
In short, there are two methods of prove:
- Check if the response of http://www.msftncsi.com/ncsi.txt is Microsoft NCSI.
- If not, check if the IP of dns.msftncsi.com is 131.107.255.255.
//using System;
//using System.Linq;
//using System.Net;
public static bool IsInternetAvailable()
{
try
{
string ncsi = new WebClient().DownloadString("http://www.msftncsi.com/ncsi.txt");
if (ncsi == "Microsoft NCSI")
{
return true;
}
}
catch { }
try
{
IPHostEntry iphost = Dns.GetHostEntry("dns.msftncsi.com");
if (iphost != null)
{
foreach (var item in iphost.AddressList)
{
if (item.ToString() == "131.107.255.255")
{
return true;
}
}
}
}
catch { }
return false;
}Is there anything I can improve or is it good as it stands now?
Solution
preface: This answer is more of a design/architecture related meta-answer about this type of check and might be seen as missing the point of a code review by some. QUESTIONS about design and architecture are off-topic, but I can't find any meta discussions about ANSWERS that address these.
I think a generic
Moving beyond "Are my servers available", your servers being available doesn't mean that your webservices are functional. Your webservices might not be available while your servers themselves are happily reporting their functional status. Or (less likely) your servers heartbeat monitor is reporting broken, but your webservice still works.
Finally, since webservices are inherently time sensitive and the internet is a fickle mistress precariously balanced network of interconnected and interdependent fragilities, service status can change between the time that you check availability and the time that you need availability.
Because of these remarks, here is my opinion:
Combining these 3, I'd say that an
To give feedback on your implementation:
Catching EVERYTHING in a generic
*: If we leave out malicious intent, that is. Technically, someone could Man in the Middle your request since it's requesting an http resource and return a string long enough to crash your program. But that's easily mitigated by connecting to an https resource.
I think a generic
IsInternetAvailable check misses the point. 99% of software that requires internet connectivity realistically only requires connectivity to one server (or one group of servers), i.e. your own servers. Checking that msftncsi is available does not mean that your own backend servers are available. Your own servers might have an outage, or msftncsi might have an outage while your servers are still online.Moving beyond "Are my servers available", your servers being available doesn't mean that your webservices are functional. Your webservices might not be available while your servers themselves are happily reporting their functional status. Or (less likely) your servers heartbeat monitor is reporting broken, but your webservice still works.
Finally, since webservices are inherently time sensitive and the internet is a fickle mistress precariously balanced network of interconnected and interdependent fragilities, service status can change between the time that you check availability and the time that you need availability.
Because of these remarks, here is my opinion:
- A check for
IsInternetAvailablethat checks 3rd party servers but doesn't check your own servers is like a check forIsMyToiletCloggedthat checks that the sink drains but doesn't try to flush the toilet.
- Check for the availability of the services you need, not the server you need.
- Checking the services you need before you actually need them does not guarantee that they will be available when you need them.
Combining these 3, I'd say that an
IsInternetAvailable function should be used as a purely academical exercise, especially a generic mutation like this one. If your software requires a certain webservice, just call that webservice when you need it and degrade gracefully if you get an unexpected result. I cannot determine from this question whether this is an academic exercise or intended to be added to a project in development. If this check is intended for actual development, I urge to you reconsider adding the check. If this check is written purely as an academic perspective, I still think that this answer is worth considering.To give feedback on your implementation:
Catching EVERYTHING in a generic
catch{} block is generally discouraged. In your code, there are only a handful of exceptions that could occur, related to the WebClient and Dns classes and the functions within that you call*. At the very least, you should catch Exception so you can log the type and details in case it matters.*: If we leave out malicious intent, that is. Technically, someone could Man in the Middle your request since it's requesting an http resource and return a string long enough to crash your program. But that's easily mitigated by connecting to an https resource.
Context
StackExchange Code Review Q#115649, answer score: 20
Revisions (0)
No revisions yet.