Trapping SMTP e-mail error

[Rick Lipkin]

Antonio

You were correct .. I failed to declare oOutMail as a variable .. ( can't believe I did that ) .. there is no error any longer and the e-mail truly failed to send .. but is there a way I can trap that error to write a log to the file that the e-mail failed ??

Getting closer !!

Thanks
Rick

[James Bott]

Rick,

Try something like this:

oOutMail:bFailure := { || LogFile( "mail.log", {oOutMail:aTo:[1] + " Failed"}), oOutMail:nStatus := ST_QUIT }

Regards,
James

[Enrico Maria Giordano]

I failed to declare oOutMail as a variable .. ( can't believe I did that )

Use /w compile switch and you will never forget it again.

EMG

[Rick Lipkin]

James

I think you have a good idea there .. however, I will not know whom in the address list that may have failed ( multiple cc ) so it will be difficult to pull out the offending array element .. but, I can use the code block with a function to write a failed indicator to my table and flag my email sent row with a failure ..

Code: Select all  Expand view

oOutMail:bFailure := { || LogFile( "mail.log", {oOutMail:aTo:[1] + " Failed"}), oOutMail:nStatus := ST_QUIT }


I will work down this road to see where it leads ..

Thanks
Rick

[reinaldocrespo]

I currently implement something similar by keeping a table that acts as a mail queue. Using an ADS trigger, a new record is added to the queue any time a given pathology is signed. One of the fields on this table holds the number of attempts. After a given number of attempts (set on an ini file) the record is no longer picked up to be sent by the queue processing routine.

Code: Select all  Expand view

            if oMail:nStatus == ST_QUIT   .or. oMail:nStatus == ST_DONE
               MailWasSent( aElem )
               ncount++
            Else
               IncAttempts( aElem )
            endif


When successful, a given field on the table is set to TRUE. If a record hols a number of attempts > than nMaxAttempts, then it is no longer picked up by the SQL script that pulls documents to be emailed. In this manner you avoid retrying bad addresses forever.

Code: Select all  Expand view

      cScript := "SELECT pathno, Send_To, Queue, Attempts FROM plmail "+;
                  "WHERE DateTime_Sent IS NULL "+;
                  "AND TIMESTAMPDIFF( SQL_TSI_HOUR, creation, now() ) >= " + Str(oApp:nLatency) + ;
                  " AND Attempts < " + str( oApp:nMaxAttempts ) + ;
                  " ORDER BY Queue"



Here are all the traps I set on tsmtp class:

Code: Select all  Expand view

// different session status
#define ST_INIT       0
#define ST_CONNECTED  1
#define ST_RESET      2
#define ST_MAILFROM   3
#define ST_RCPTTO     4
#define ST_DATA       5
#define ST_SENT       6
#define ST_QUIT       7
#define ST_DONE       8
#define ST_ERROR      9


Reinaldo.

[Rick Lipkin]

Reinaldo

I thought about your solution .. I just 'hate' tampering with FWH class code ..

I may have to do what you suggest ...

James ..

I took your suggestion and I must have some syntactical error where 7 is ST_QUIT

Your suggestion :

oOutMail:bFailure := { || LogFile( "mail.log", {oOutMail:aTo:[1] + " Failed"}), oOutMail:nStatus := ST_QUIT }

Trying to get it to work .. unfortunitly the LogFile fires everytime good or bad ??

What am I missing here ??

Rick

Code: Select all  Expand view

oOutMail:bFailure := { || LogFile( "Failed"), oOutMail:nStatus := 7 }
...

//-------------------
Static Func LogFile( cFAILED )

IF EMPTY( cFAILED )
   cFAILED := "NO"
ENDIF

MsgInFo( cFAILED )

RETURN(NIL)


Code: Select all  Expand view

[reinaldocrespo]

Rick;

I feel the same way. But sometimes you just have to. And when you do, I always try to subclass.

CLASS eMAIL FROM TSMTP
....

In this case I replaced the OnRead Method to include all the other flags.


Reinaldo.

[Rick Lipkin]

Reinaldo

I had to resort to modifying Tsmtp .. just no other way arount it .. here is what I did :

Code: Select all  Expand view

// use this public variable
         // returns from tsmtp() .T.
        lFAIL := .F.

        oWndMdi:SetMsg( "Sending Reporting noticication to "+cTO )

        WSAStartup()
        oOutMail := TSmtp():New( cIP := GetHostByName( cHOST ) )

        oOutMail:bConnecting := { || oWndMdi:SetMsg( "Connecting to "+cHOST ) }
        oOutMail:bConnected  := { || oWndMdi:SetMsg( "Connected" ) }
        oOutMail:bDone       := { || oWndMdi:SetMsg( "Message sent successfully" ) }
        oOutMail:bFailure    := { || oOutMail:nStatus := 7 }   // keep this

        oOutMail:SendMail( cFROM,;               // From
                 { cTO },;                       // To
                   cMESSAGE,;                    // Msg Text
                   cSUBJECT,;
                   {"C:\DBTMP\PROJINFO.BAT"},;   // attachment
                   aCC, ;                        // cc array
                   { }, ;                        // bc
                   .F., ;                        // no return receipt
                   NIL )                         // not html

        // wait for e-mail to be sent to get lfail value

        SysWait(1)

        IF lFAIL = .T.
           cSUBJECT := "Email Error FAILED to be Sent"
         *  MsgInfo( cSUBJECT )
        ENDIF

        SysReFresh()

        // create e-mail record //

        cEID := _GenEid(2)
        IF cEID = "BOGUS"
        ELSE
           oRsEmail:AddNew()
           oRsEmail:Fields("emaileid"):Value   := cEID
           oRsEmail:Fields("projecteid"):Value := oRsProj:Fields("projecteid"):Value
           oRsEmail:Fields("date_sent"):Value  := dtoc(DATE())+" "+time()
           oRsEmail:Fields("email_from"):Value := cFROM
           oRsEmail:Fields("email_to"):Value   := cTO //+", "+AEVAL(aCC)
           oRsEmail:Fields("subject"):Value    := SUBSTR(cSUBJECT+SPACE(50),1,49)
           oRsEmail:Fields("message"):Value    := cMESSAGE
           oRsEmail:Fields("attachments"):Value := "ProjInfo.Bat"

           IF lFAIL = .T.
              oRsEmail:Fields("address_error"):Value := "Y"
           ELSE
              oRsEmail:Fields("address_error"):Value := "N"
           ENDIF

           oRsEmail:Update()
        ENDIF

Notice that I set lFail as PUBLIC and set it to .f. before Tsmtp was initialized .. then in Tsmtp I found the 'bad address' code :

Code: Select all  Expand view

Case ::nStatus == ST_MAILFROM .or. ::nStatus == ST_RCPTTO
         If cReply == "250" .or. cReply == "251"  // Server happy with our repsonse
            If ::nTo <= Len( ::aTo )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aTo[ ::nTo ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nTo++
            Elseif ::nCC <= Len( ::aCC )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aCC[ ::nCC ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nCC++
            Elseif ::nBCC <= Len( ::aBCC )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aBCC[ ::nBCC ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nBCC++
            Else
               ::nStatus := ST_DATA
               oSocket:SendData( "DATA" + CRLF )
            Endif
         Else

            // failure here is e-mail address is bad
            // Rick added to trap

            lFAIL := .T.
            Msginfo( "Message Failed to be sent because of a Bad Address")

            ::Failure( oSocket, nWSAError, cReply )
         Endif

Then notice the SysWait(1) was very important here to reliably allow lFail to initalize and return back with it's correct value .. then I write the "address_error" field to Y if it failed N if it worked .. then when I browse the e-mail sent records I turn the row 'red' based on the value of 'address_error'.

This is not a perfect solution and the return code of Smtp is not always consistant .. and I found I could NOT put my lFail in the Failure() method .. was too inconsistant there ..

For now I have a work around .. not perfect because the SMTP error does not always return the same code..

Rick

[James Bott]

Rick,

took your suggestion and I must have some syntactical error where 7 is ST_QUIT

Your suggestion :

oOutMail:bFailure := { || LogFile( "mail.log", {oOutMail:aTo:[1] + " Failed"}), oOutMail:nStatus := ST_QUIT }

Trying to get it to work .. unfortunitly the LogFile fires everytime good or bad ??

If logfile() is getting called every time, then bFailure is being eval'd every time--it shouldn't be. This indicates that there is either a failure every time, or that there is a bug in the TSMTP class. We really should figure out which.

You also stated that you are not always getting the same error and this is futher indication of a problem or multiple problems. What are the various errors you are getting?

Assuming you do need to modify the code of the TSMTP class, you really should sublcass as Reinaldo suggested--and you should add a class var lFailed rather than using a public. If you need help doing this, just ask.

But before making modifications we really need to find out why bFailure is getting eval'd every time.

Regards,
James

[Antonio Linares]

Rick, James,

Class TSmtp Method Failure() should be modified as it receives three parameters: oSocket, nWSAError, cReply but when the bFailure codeblock is evaluated from inside it, these parameters are not supplied to the codeblock. This is the proposed change:

Code: Select all  Expand view

   If ::bFailure != nil
      Eval( ::bFailure, oSocket, nWSAError, cReply )
   Endif


The idea is to include cReply in the LogFile() call so we know under what server responses Failure() is getting invoked.

[Rick Lipkin]

Antonio

Thank you for jumping in here .. what do you see will be the return and how can I trap the failure as in this code when I make the Tsmtp change ??

Thanks
Rick Lipkin

Code: Select all  Expand view

oOutMail:bFailure := { || LogFile( "Failed"), oOutMail:nStatus := 7 }
..
//-------------------
Static Func LogFile( cFAILED )

IF EMPTY( cFAILED )
   cFAILED := "NO"
ENDIF

MsgInFo( cFAILED )

RETURN(NIL)


from this exiting code :

Code: Select all  Expand view

//----------------------------------------------------------------------------//

METHOD Failure( oSocket, nWSAError, cReply ) CLASS TSmtp

   Local aStage := { "ST_INIT", ;
                     "ST_CONNECTED", ;
                     "ST_RESET", ;
                     "ST_MAILFROM", ;
                     "ST_RCPTTO", ;
                     "ST_DATA", ;
                     "ST_SENT", ;
                     "ST_QUIT", ;
                     "ST_DONE", ;
                     "ST_ERROR", ;
                     "ST_AUTH0", ;
                     "ST_AUTH", ;
                     "ST_USER", ;
                     "ST_PASS" }

   DEFAULT oSocket := ::oSocket, nWSAError := WSAGetLastError(), cReply := ""

   If ::nStatus >= ST_INIT .and. ::nStatus <= ST_LAST
      ::cError := "Stage: " + aStage[ ::nStatus + 1 ] + CRLF
   Else
      ::cError := ""
   Endif
   ::nStatus := ST_ERROR
   ::cError += "IP Address: " + ::cIPServer + CRLF + CRLF
   AEval( ::acReply, {|cReply| ::cError += cReply + CRLF } )
   If nWSAError # 0
      ::cError += "WSA Error Code: " + AllTrim( Str( nWSAError ) )
   Endif
   If ::bFailure != nil
      Eval( ::bFailure )
   Endif
   oSocket:End()

return Self


to this :

Code: Select all  Expand view

//----------------------------------------------------------------------------//

METHOD Failure( oSocket, nWSAError, cReply ) CLASS TSmtp

   Local aStage := { "ST_INIT", ;
                     "ST_CONNECTED", ;
                     "ST_RESET", ;
                     "ST_MAILFROM", ;
                     "ST_RCPTTO", ;
                     "ST_DATA", ;
                     "ST_SENT", ;
                     "ST_QUIT", ;
                     "ST_DONE", ;
                     "ST_ERROR", ;
                     "ST_AUTH0", ;
                     "ST_AUTH", ;
                     "ST_USER", ;
                     "ST_PASS" }

   DEFAULT oSocket := ::oSocket, nWSAError := WSAGetLastError(), cReply := ""

   If ::nStatus >= ST_INIT .and. ::nStatus <= ST_LAST
      ::cError := "Stage: " + aStage[ ::nStatus + 1 ] + CRLF
   Else
      ::cError := ""
   Endif
   ::nStatus := ST_ERROR
   ::cError += "IP Address: " + ::cIPServer + CRLF + CRLF
   AEval( ::acReply, {|cReply| ::cError += cReply + CRLF } )
   If nWSAError # 0
      ::cError += "WSA Error Code: " + AllTrim( Str( nWSAError ) )
   Endif
   
   // suggested change here ... 

   If ::bFailure != nil
      Eval( ::bFailure, oSocket, nWSAError, cReply )
   Endif
   
   oSocket:End()

return Self


[Antonio Linares]

Rick,

Code: Select all  Expand view

oOutMail:bFailure := { | oSocket, nError, cReply | LogFile( "log.txt", { nError, cReply } ), oOutMail:nStatus := 7 }


Lets see what you get in "log.txt"

[Rick Lipkin]

Antonio

I got two different outputs .. sorta what I was seeing last nite when I was trying to trap this ..

I get this the first time :

10/06/2008 20:16:22: 0 550

My trap in Tsmtp caught the first error in this code :

Code: Select all  Expand view

Case ::nStatus == ST_MAILFROM .or. ::nStatus == ST_RCPTTO
         If cReply == "250" .or. cReply == "251"  // Server happy with our repsonse
            If ::nTo <= Len( ::aTo )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aTo[ ::nTo ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nTo++
            Elseif ::nCC <= Len( ::aCC )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aCC[ ::nCC ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nCC++
            Elseif ::nBCC <= Len( ::aBCC )
               oSocket:SendData( StrTran( "RCPT TO:<%>", "%", CleanEMail( ::aBCC[ ::nBCC ] ) ) + ;
                                 CRLF )
               ::nStatus := ST_RCPTTO
               ::nBCC++
            Else
               ::nStatus := ST_DATA
               oSocket:SendData( "DATA" + CRLF )
            Endif
         Else

            // failure here is e-mail address is bad
            // RIck added to trap

            lFAIL := .T.
            Msginfo( "Message Failed to be sent because of a Bad Address")

            ::Failure( oSocket, nWSAError, cReply )
         Endif

But the same error was not trapped at all when I re-ran the same code a second time ( without re-starting the program ) and the log returned below ..

10/06/2008 20:17:46: 10048

This was the same erratic problem I saw when I was working this ..

Rick

[Antonio Linares]

Rick,

> 10/06/2008 20:17:46: 10048

We should get two values there, not just one. As we are tracing nError, cReply.

Please post here the complete log.txt, thanks

[Rick Lipkin]

Antonio

I ran the same e-mail process twice and the results were appended to log.txt :

Code: Select all  Expand view

10/07/2008 16:35:41: 0   550   
10/07/2008 16:36:01: 10048

Both ( definitly ) failed .. only the top one was trapped .. I have no clue in the tsmtp code where the second one slipped thru ??

RIck