gemmapeter.co.uk

Came across a tricky bug a few weeks ago whilst working on some extremely fragile code.

This was the code:

settings.php

$mail_settings = array(
    'server' => 'mail.example.com',
    'port' => 25
);

index.php

function send_email($recipient, $message)
{
    include_once('settings.php');
    $foo = new MailSender($mail_settings);
    $foo->send($message, $recipient); //this was line number in the error message
}
 
send_email('admin@example.com', 'Admin message');
send_email('user@example.com', 'User message');

The tricky part was debugging it, it threw an error on the second call of the send_email function. That’s odd it ran fine the first time and the only thing I could see that had changed was the input to the function, surely that must be the problem? I commented out the second function call error went away, commented it back in and the error was back. This fitted with my hypothesis that the input on the second one was causing the error. But after looking inside the MailSender class I couldn’t see any reason why the function would error there.

I called Kev to give the problem a fresh set of eyes because sometimes when you are working on things like this you end up not being able to see the thing you’re looking for if you stare at the code for too long.

We took apart the function. We created a new file and replaced each function call with the code which was inside the function and were surprised when it worked perfectly. We were confused as to how the exact same code when placed inside a function would have any different effect.

After looking in the wrong places for a while we finally decided to re-examine our assumptions about what the commenting out was telling us. It wasn’t telling us that the problem is in how we were calling the function the second time around, it was the fact that we were calling the function twice.

After realising that it was obvious what was causing the error.

• Mailsender needs $mail_settings to work
• variables from settings.php were included with an include once
• include once will not include a file that has already been included
• the second function call will not have access to variables from settings.php since they are a) out of scope from first function call b) won’t be included again due to the include once

After changing the include_once to an include the function worked as expected.

This made me think of some questions. How would I unit test this? Should I create a unit test that runs each function n times? How could I ensure that the test wouldn’t fail at n+1 times? How can I be sure my test will be accurate without running the function an infinite amount of times?

Of course the real solution would be to remove the hidden dependencies caused by creating variables in an included file but to do that to this particular inherited project would require a complete rewrite.