Trapping SMTP e-mail error

Postby Rick Lipkin » Sat Oct 04, 2008 12:43 am

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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby James Bott » Sat Oct 04, 2008 7:27 am

Rick,

Try something like this:

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

Regards,
James
User avatar
James Bott
 
Posts: 4840
Joined: Fri Nov 18, 2005 4:52 pm
Location: San Diego, California, USA

Postby Enrico Maria Giordano » Sat Oct 04, 2008 10:33 am

Rick Lipkin wrote: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
User avatar
Enrico Maria Giordano
 
Posts: 8725
Joined: Thu Oct 06, 2005 8:17 pm
Location: Roma - Italia

Postby Rick Lipkin » Sat Oct 04, 2008 5:30 pm

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  RUN
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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby reinaldocrespo » Sun Oct 05, 2008 10:48 pm

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  RUN
            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  RUN
      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  RUN
// 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.
User avatar
reinaldocrespo
 
Posts: 979
Joined: Thu Nov 17, 2005 5:49 pm
Location: Fort Lauderdale, FL

Postby Rick Lipkin » Sun Oct 05, 2008 11:11 pm

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  RUN
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  RUN
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby reinaldocrespo » Mon Oct 06, 2008 1:11 am

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.
User avatar
reinaldocrespo
 
Posts: 979
Joined: Thu Nov 17, 2005 5:49 pm
Location: Fort Lauderdale, FL

Postby Rick Lipkin » Mon Oct 06, 2008 2:41 am

Reinaldo

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

Code: Select all  Expand view  RUN
// 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  RUN
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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby James Bott » Mon Oct 06, 2008 1:17 pm

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
User avatar
James Bott
 
Posts: 4840
Joined: Fri Nov 18, 2005 4:52 pm
Location: San Diego, California, USA

Postby Antonio Linares » Mon Oct 06, 2008 7:58 pm

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  RUN
   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.
regards, saludos

Antonio Linares
www.fivetechsoft.com
User avatar
Antonio Linares
Site Admin
 
Posts: 42203
Joined: Thu Oct 06, 2005 5:47 pm
Location: Spain

Postby Rick Lipkin » Mon Oct 06, 2008 11:02 pm

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  RUN
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  RUN
//----------------------------------------------------------------------------//

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  RUN
//----------------------------------------------------------------------------//

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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby Antonio Linares » Tue Oct 07, 2008 12:03 am

Rick,

Code: Select all  Expand view  RUN
oOutMail:bFailure := { | oSocket, nError, cReply | LogFile( "log.txt", { nError, cReply } ), oOutMail:nStatus := 7 }

Lets see what you get in "log.txt"
regards, saludos

Antonio Linares
www.fivetechsoft.com
User avatar
Antonio Linares
Site Admin
 
Posts: 42203
Joined: Thu Oct 06, 2005 5:47 pm
Location: Spain

Postby Rick Lipkin » Tue Oct 07, 2008 12:27 am

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  RUN
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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

Postby Antonio Linares » Tue Oct 07, 2008 7:17 am

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
regards, saludos

Antonio Linares
www.fivetechsoft.com
User avatar
Antonio Linares
Site Admin
 
Posts: 42203
Joined: Thu Oct 06, 2005 5:47 pm
Location: Spain

Postby Rick Lipkin » Tue Oct 07, 2008 8:39 pm

Antonio

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

Code: Select all  Expand view  RUN
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
User avatar
Rick Lipkin
 
Posts: 2666
Joined: Fri Oct 07, 2005 1:50 pm
Location: Columbia, South Carolina USA

PreviousNext

Return to FiveWin for Harbour/xHarbour

Who is online

Users browsing this forum: Google [Bot] and 11 guests