Page 2 of 3

PostPosted: Sat Oct 04, 2008 12:43 am
by 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

PostPosted: Sat Oct 04, 2008 7:27 am
by James Bott
Rick,

Try something like this:

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

Regards,
James

PostPosted: Sat Oct 04, 2008 10:33 am
by Enrico Maria Giordano
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

PostPosted: Sat Oct 04, 2008 5:30 pm
by 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

PostPosted: Sun Oct 05, 2008 10:48 pm
by 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.

PostPosted: Sun Oct 05, 2008 11:11 pm
by 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

PostPosted: Mon Oct 06, 2008 1:11 am
by 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.

PostPosted: Mon Oct 06, 2008 2:41 am
by 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

PostPosted: Mon Oct 06, 2008 1:17 pm
by 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

PostPosted: Mon Oct 06, 2008 7:58 pm
by 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.

PostPosted: Mon Oct 06, 2008 11:02 pm
by 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

PostPosted: Tue Oct 07, 2008 12:03 am
by 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"

PostPosted: Tue Oct 07, 2008 12:27 am
by 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

PostPosted: Tue Oct 07, 2008 7:17 am
by 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

PostPosted: Tue Oct 07, 2008 8:39 pm
by 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