Try/Catch- how to stop process 1

[DCSage]

I have a small application that uploads, copies and then logs the actions to a text file. I have my functions enclosed in try/catch statements, but the application seems to still move, copy the files and write to the log even if the upload was unsuccessful. Could this be happening because of my catch statements? How can I makle sure that the application stops (writes to the log that it is stopping) if the upload was unsuccessful?

        static void Main(string[] args)
        {
            string GroupEmail = "myemail@abc.com";
            sendEmail confirmMail = new sendEmail();
          
            Program prg = new Program();

            try
            {
                prg.UploadEdi();
            }
            catch(Exception uplErr)
            {
                confirmMail.send("<font face='arial' size='2'>Error Uploading File(s)</font><br>" + uplErr.ToString(), GroupEmail, "Unsuccessful File Upload to Server", "0");
            }

            try
            {
                prg.WriteToStream();
            }
            catch (Exception StreamErr)
            {
                confirmMail.send("<font face='arial' size='2'>Error Sending File</font><br>" + StreamErr.ToString(), GroupEmail, "Unsuccessful Log", "0");
            }
            try
            {
                prg.MoveEdi();
            }
            catch (Exception movErr)
            {
                confirmMail.send("Unsuccessful file move to destination folder" + movErr.ToString(), GroupEmail, "Unsuccessful Move", "0");
            }
          }

 

[jmeckley]

catching an exception allows you to handle the error and continue processing. if you change the code to

try
{
   prg.UploadEdi();
   prg.WriteToStream();
   prg.MoveEdi();
}
catch(Exception e)
{
   confirmMail.send(uplErr.ToString(), GroupEmail, "EDI Error", "0");
}

the program will email the error and exit as soon as an exception is thrown. however this still has issues.

1. if the error occurs when WriteToStream the upload files will not be cleaned up.
2. if the error occurs when MoveEdi the files are written to the stream but never moved.
3. what happens if an error occurs while sending the error email?

I also can't help but think there are issues with the code that reads/writes files. If these objects are not properly disposed of you can run into locking and resource management errors very quickly.

You can implement you're own System Transaction Coordinators
which help ensure ACID operations. in which case you're code could looks like this

try
{
   using(var tx = new TransactionScope())
   {
      var prg = new Program();
      prg.UploadEdi();
      prg.WriteToStream();
      prg.MoveEdi();

      tx.Complete();
   }
}
catch(Exception e)
{
   Log.Error(e);
}

Nothing happens until Complete is called. And complete does does a double pass to ensure the operation will be successful. 1st pass checks each coordinator to see if it is ready. if all coordinators are ready the 2nd pass will preform the action.

as a last resort (or quick a dirty) solution. you can nest the try/catch block.

try
{
   prg.UploadEdi();
   try
   {
      prg.WriteToStream();
      try
      {
         prg.MoveEdi();
      }
      catch
      {
         ... send email notice
      }
   }
   catch
   {
      ... send email notice
   }
}
catch
{
   ... send email notice
}

but that is really just procedural code hacked together with OOP.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[DCSage]

thank you so much. I didn't think about transactions for this code. You mentioned the following (which helps me review my code more):

1. if the error occurs when WriteToStream the upload files will not be cleaned up.
2. if the error occurs when MoveEdi the files are written to the stream but never moved.
3. what happens if an error occurs while sending the error email?


I have separate classes for each one. For the WriteToStream, I am clsing the stream after the file uploads. Should I include try/catch statements or even transactions within that class to make sure the file are cleaned up?

for the MoveEdit files, if the error occurs here, is it best to include a try/catch statment in the actual class? the smae goes for the send email.

    class uploadFile
    {
        /// <summary>
        /// upLoadFile class sets the parameters enabling file uploads
        /// </summary>
        /// <param name="ftpServer"></param>
        /// <param name="userName"></param>
        /// <param name="password"></param>
        /// <param name="uplfile"></param>
        public void Upload(string ftpServer, string userName, string password, string uplfile)
        {

            //establish the parameters

            FtpWebRequest request = (FtpWebRequest)FtpWebRequest.Create(ftpServer + "/" + Path.GetFileName(uplfile));

            request.Method = WebRequestMethods.Ftp.UploadFile;
            request.Credentials = new NetworkCredential(userName, password);
            request.UsePassive = true;
            request.UseBinary = true;
            request.KeepAlive = false;

            //load the file
            FileStream stream = File.OpenRead(uplfile);
            byte[] buffer = new byte[stream.Length];
            stream.Read(buffer, 0, buffer.Length);
            stream.Close();
            
            //upload the file

            Stream reqStream = request.GetRequestStream();
            reqStream.Write(buffer, 0, buffer.Length);
            reqStream.Close();
        }

    }

 

[jmeckley]

managing try/catch is somewhat of an art. There is reference material on the web about exception management.

it's not so much that you need to catch the exception to clean up remote resources (like file streams). Rather you need to properly dispose of them. for objects which implement IDisposable you can use the 'using' keyword

using(var stream = File.Open(...))
{
   stream.Write(...);
}

which is syntax sugar for

Stream stream = null;
try
{
   stream = File.Open(...);
   ...
}
finally
{
   if(stream != null)
   {
      stream.Dispose();
   }
}

Dispose will automatically close a stream.

In my experience I find try/catch is most appropriate at the infrastructure level. once I'm coding for the UI and or the business domain I may throw custom exceptions, but I don't usually catch exceptions. If I were to catch an exception in the domain logic, it would only be something I would expect, which would be a custom exception I defined. Even then I would catch the exception, reset the state of the system (for example rollback the transaction) and then throw the exception again. example

using(var tx = session.BeginTransaction())
{
   try
   {
      //do some work
      tx.Commit();
   }
   catch
   {
      tx.Rollback();
      throw;
   }
}

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[DCSage]

Thank You!!!!!!


The System.Transactions worked perfectly. I like the simplicity of it.

Thank you so much.

 

[jmeckley]

could you share the code, at least the part related to transactions? it would be helpful for others.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259