views:

118

answers:

7

A little background on this error: The customer getting this error message in their log file and support hasn't been able to reproduce it yet. So I'm reviewing the code trying to determine what may be happening. I have narrowed it down to this section of the code by reviewing their log file. I didn't write this code, but it's purpose is to ftp a zip file to a remote server. So the question is....

What line(s) this code might throw a "Index was outside the bounds of the array" exception?

FtpLib.FTPFactory ff = new FtpLib.FTPFactory();
try

{
    ff.setRemoteHost(job.FTPHost);
    ff.setRemoteUser(job.FTPUser);
    ff.setRemotePass(job.FTPPW);
    ff.login();
// Execute misc. extra commands
foreach (string command in job.Commands)
{
 if (log.IsDebugEnabled)
  log.Debug("JOB: " + job.ID + " --    FTP Command \"" + command + "\" sent...");

 ff.sendCommand(command);

 if (log.IsDebugEnabled)
  log.Debug("JOB: " + job.ID + " --       Response: " + ff.getLastMessage());
}

try
{
 ff.mkdir(job.FTPRemoteDir);
}
catch (IOException) { }

ff.chdir(job.FTPRemoteDir);
ff.setBinaryMode(true);

if (log.IsInfoEnabled)
 log.Info("JOB: " + job.ID + " --    FTP UPLOAD: \"" + zipfile.Name + "\" to \"" + job.FTPHost + "/" + job.FTPRemoteDir + "/\"");
ff.upload(zipfile.FullName);
if (log.IsInfoEnabled)
 log.Info("JOB: " + job.ID + " --       Completed.");

bFTPSuccess = true;
break;

}

Thanks in advance!

UPDATE: I think we all pretty much agree that the issue is going to be in the FTPLib, I'll see if we have the source for it. I found out this is an obscure bug the customer can't even reproduce consistently, so this will be a fun one pin point. I have added additional debug logging using Exception.StackTrace and Exception.ToString functions. I'll update again once the issue is solved and try to award the correct person with the correct answer, although everyone has made good suggestions. Thanks for the help!

+2  A: 

Without knowing the rest of the code that could be quite difficult to tell. The line that stands out for me is:

ff.getLastMessage()

but that is just an educated guess (if for example messages are stored in an array or simliar, and last extracts from that)

Chris
+1  A: 

I don't think it's any of those lines of code.

My first guess would be something within your foreach loop but that seems pretty solid.

Are you sure it's in the part of the code?

Can you paste the stack trace?

Robert Greiner
we haven't reproduce it yet, so I can't paste the exact stack trace. I'll see what I can do though.
cchampion
ah, well that's your problem... You could be chasing phantom code around all day without knowing specifically where to look. I think your time is much better spent reproducing the error and trying to solve it from there instead of guessing where you might have gone wrong. This is tough though, I hate it when it's difficult to reproduce bugs. Good luck.
Robert Greiner
Found out the customer can't even reproduce the error. It's only occured 3 times in the past few months LOL. Fun.
cchampion
haha, I hate that. Well good luck tracking it down if you actually can reproduce it someday.
Robert Greiner
+1  A: 
ff.getLastMessage()

What does that method do? How does it get the last message? If it's using something like messages[messages.Count - 1] or something similar, and the messages collection is empty, it'd throw that error. That'd be my best guess.

Brandon
+1  A: 

Nothing obvious. Its going to come down to the actual implementation of the FtpFactory (my vote for likely cause) and the log.

A factory might keep an array of instances of the type it creates if the creation of those types is costly. So the first time you do something that might require the use of one of those instances ("ff.login") might expose the bug.

I'd suggest you surround any attempt to use the factory with a try/catch and record in the log where it happens. Also, I'd suggest avoiding using that factory anyhow because it was written by someone with little regard for standards and practices (lower case method names?) which is a warning sign of poor workmanship.

tl;dr: Don't trust your ftp factory. It probably sucks.

Will
+1  A: 

In addition to Will's post for sake of completeness:

FtpLib.FTPFactory ff = null;
try
{
    ff = new FtpLib.FTPFactory();
}
catch(Exception ex)
{
    // Log it
}

Also, I couldn't help wondering, is the login working, that is another place to wrap it around in try/catch block

try
{
    ff.login();
}
catch(Exception ex)
{
   // Log it.
}

It could be that the login is failing, and ff is being used to carry out the commands and probably throws that exception - don't ask why? I suspect this FTP routines is written by someone who does not know what is happening or poorly written, look at the login method - all in lowercase as Will above (kudos for pointing it out) mentioned.

Hope this helps and happy bug hunting... Best regards, Tom.

tommieb75
+1  A: 

I suggest you log the Exception.StackTrace at the point where you are currently catching the out of bounds exception. Exception.ToString() is also very useful, as it gives most of the important information. This should tell you the exact place where the exception is being thrown (without having to guess). Beware however code that catches an exception and hides it--I see the code you posted has "catch (IOException) { }". Also wary off code that catches an exception and then throws a different exception, which will mask the original error.

There should not be any need to wrap in try...catch individual lines that are suspect--as long as any exception is caught at some point in the call chain and logged.

ShellShock
I agree, it appears this FTPLib relies on exceptions too much. I will take a look into that code. For now I took your advise and added the StackTrace and ToString to the log file.
cchampion
A: 

Basically every single method call is a possible candidate including the constructor and the properties. (They are basically method calls) the last two are though probably those with the lowest risk (That would be a weird implementation of either)

most plausible I'd say is ff.getLastMessage()

Rune FS