Copying Infested Code

Background

As some of you may know, my current role consists of me building and maintaining automations. The thing about writing code is that at some point, maintenance has to be done on the code to keep it up to date and to add future enhancements and functionality.

What Kind of Pest (The Problem)

Squirrels and rats both chew on wood. However they do not eat the same stuff, so you have to be selective with the bait that you choose. For most of the automations that I build, I use Selenium WebDriver as the automations are for web-based applications.

Recently, we have noticed that number of automations have not been running due to “connection refused” exceptions. The connection is being refused from the automation jar not connecting to the Gecko Driver (the interface between WebDriver automation and the browser). Applying knowledge that I have of computers, normally you only see “connection refused” messages when one or more of the scenarios occur

• too many devices trying to connect at the same time, also referred to as Denial of Service Attack or Distributed Denial of Service Attack
• a firewall is rejecting the connection
• wrong password when trying to authenticate.

Tracking Down The Pest (The Analysis)

I was able to immediately rule out scenarios 2 and 3 because the automation would work majority of the time. In other words, we know it is not a rat based on the evidence. Therefore, through the process of elimination, it must be a squirrel. Majority being that out of 7 days of the week that it was used, it would fail only on 1 of those days. I then did some research on the internet that provided a number of causes and solutions for the exception occurring. Some of the causes included

• the Gecko Driver process not being able to write to a log file because existing log file already exists (not in our case)
• having concurrent WebDriver automations running at the same time (which wasn’t our case as we have done several at the same time in the past)
• to the hosts file not having the localhost IP address entered (which we did not, but nslookup and ping commands both returned valid IP addresses)
• having bad code.

I got to that last reason in the list, bad code, and I put more effort into looking into the code as we have experienced (2+ years) and inexperienced developers on our team.

Found The Nest (Found The Source Of The Problem)

After search through multiple automation projects, shaking my head at some of the exact examples of bad code mentioned in Robert Martin’s Clean Code, I hit the bullseye. Several of the automations had the same code.

Yes... copied and pasted. First person got it working, so the next person got theirs working by doing exactly what the first person did. There's that saying about if you see one bug, there's probably another one not too far way. This would be it. To make it worse, it was multiplying everytime the code was copy and pasted. This is what I found:

WebDriver driver = null;

try{    
    driver = new FirefoxDriver();
    driver.get("http://thealmostengineer.com");

    // other WebDriver steps go here 

    driver.quit();
    System.exit(0);
} catch (Exception exception) {
    exception.printStackTrace();
    System.exit(1);
}

Now this is just a snippet of the actual code. Obviously, I cannot put the entire code here because of confidentiality. Do you notice the problem with this code?

The problem here is that if an exception occurs between the try and driver.quit();, the browser and as a result, the Gecko Driver process never exits. What happens when you have too many processes running? The system will hang. What happens when there are too many connections open to a process? Some of those connections are refused.

Spraying The Bug Spray (The Solution)

To now rid the varmint of our code, I had to fix the defect that I identified. The driver.quit();bashould always be executed regardless of whether the exception occurs. The reason being is that you do not want to the browser nor the Gecko Driver process running as they will hang up your system or result in the "connection refused" exceptions. So I changed the above code to the code below:

WebDriver driver = null;
int exitCode = 1;

try{    
    driver = new FirefoxDriver();
    driver.get("http://thealmostengineer.com");

    // other WebDriver steps go here 

    exitCode = 0;
} catch (Exception exception) {
    exception.printStackTrace();
}

if (driver != null)
{
    driver.quit();
}

System.exit(exitCode);

The changes that I made:

• the driver.quit(); has been moved outside of the try-catch block. That way it will always execute whether an exception occurs or not. Could a finallyblock been used? Yes. However, for the sake of simplifying the code I did not use one.
• I moved the System.exit call to the end of the code. It is best to have only one way to exit a function. If you have multiple ways to exit (or return) out of a function, code that needs to exit may not get executed because of the multiple ways out.
• I created a new integer variable called exitCode. Following standards for program exit code, 1 or greater means that something went wrong during execution. 0 means that everything went well during execution. Therefore, the variable defaults to 1 because you do not want your code to say that all went well when it actually did not. It is better to have a false failure and it get noticed than to have a false success and it go undetected.

Conclusion

If you are going to copy the code of others, please be sure to review it first before using it as it may have defects already in it. If you do not know what the code does, then spend a few moments searching for those commands or snippets online so that you learn what they do. Using code somebody else wrote without confirming that it is not defective, is like building a chair from wood and not checking to see if it is rotting or has termites.