1

I have this simple script that I use to send myself email with a status on a server. It runs, and it gives me no errors, but I do not receive any email, and there is no email in the sent folder at Google. But if I copy and paste every line by line into python in a shell, it does send email and works. There are no errors anywhere. I even get accepted status from Google.

UPDATE: I might have cut to much code out of the sample, and the i variable was accidentally taken out. I have added it again. When I copy paste each line into python cmd line, the script works. When I just run the script, it reports no errors, but does not send the email.

import smtplib
i = 0
try:
    text = "This is remarkable"
    fromaddr = "<gmail address>"
    toaddr = "<email address>"
    msg = """\
            From: <gmail address>
            To: <email address>
            Subject: Message number %i

            %s
    """ % (i, text)

    server = smtplib.SMTP("smtp.gmail.com:587")
    server.set_debuglevel(1)
    server.ehlo()
    server.starttls()
    server.login("<email>", "<password>")

    ret = server.sendmail(fromaddr, toaddr, msg)
    print "returned : ", ret
    print "message sent"
    i += 1
except:
    print "Now some crazy **** happened"
Trausti Thor
  • 3,722
  • 31
  • 41
  • 1
    here i is not defined therefore there is an error. Give an initial value to i, then the script runs fine – Vipul Mar 08 '14 at 17:10
  • 1
    This is why you shouldn't wrap everything in a `try`... http://blog.codekills.net/2011/09/29/the-evils-of--except--/ – jonrsharpe Mar 08 '14 at 17:17
  • 1
    FWIW: Despite that fact that you've accepted an answer below -- I wanted to mention that the current code in your question (with the appropriate email addresses and password substituted) worked for me running it as a script. There were some formatting issues, but I received it and no exceptions were raised. – martineau Mar 08 '14 at 20:31
  • BTW: I fixed the formatting issues in the received message by using `msg = textwrap.dedent("""\ ... """) % (i, text)`. – martineau Mar 08 '14 at 20:44
  • 1
    To avoid numerous gotchas with the email message encoding, you could use MIMEText, Header classes from `email` package. Here's a [complete code example](http://stackoverflow.com/a/20787826/4279) – jfs Mar 09 '14 at 02:24

3 Answers3

2

i is not defined. You would see a NameError if you hadn't wrapped try and a bare except around everything. You should have as little as possible in the try block, at least. Also you can catch the specific error as e. This will print the error also so that you can understand what is the problem.

So the code should be:

import smtplib

i = 1
text = "This is remarkable"
fromaddr = "<gmail address>"
toaddr = "<email address>"
msg = """\
        From: <gmail address>
        To: <email address>
        Subject: Message number %i

        %s""" % (i, text)

try: 
    server = smtplib.SMTP("smtp.gmail.com:587")
    server.set_debuglevel(1)
    server.ehlo()
    server.starttls()
    server.login("<email>", "<password>")
    ret = server.sendmail(fromaddr, toaddr, msg)
except Exception as e:
    print 'some error occured'
    print e
else:
    print "returned : ", ret
    print "message sent"
    i += 1
jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
Vipul
  • 4,038
  • 9
  • 32
  • 56
  • There should be less in `try`! If the trivial string formatting were outside it the error would have been caught immediately. The last few lines (`print`, increment `i`) could be moved into an `else`, too. – jonrsharpe Mar 08 '14 at 17:32
  • yes ! you are right.. can you please edit the code for better exception handling ! – Vipul Mar 08 '14 at 17:38
  • Catching _all_ exceptions with a `except Exception:` is generally a poor programming practice, especially when it (or another in its place) isn't (re)raised and the script is allowed to continue execution. – martineau Mar 08 '14 at 18:50
  • The try was just a replacement for a loop I had, for a pub sub subscription. The changes above work – Trausti Thor Mar 08 '14 at 20:02
1

Your counter variable i is undefined. A try-except is not a loop. You should be more conservative with what you put inside a try block. Only wrap statements that you think could throw an exception that you need to catch. Certainly your msg string should be built outside the exception handler, since it shouldn't fail and if it does your code is wrong. The calls to the mail server could be wrapped:

server = smtplib.SMTP(host="smtp.gmail.com", port="587")
server.set_debuglevel(1)
try:
    server.ehlo()
    server.starttls()
    server.login("<email>", "<password>")
    server.sendmail(fromaddr, toaddr, msg)
    print "message sent"

except smtplib.SMTPException as e:
    print "Something went wrong: %s" %str(e)

finally:
    server.close()

In this way you've reduced the amount of code in the try-except block.

When catching exceptions take care not to catch more than you expect. We can define what type of exceptions we expect and then take appropriate action. Here we simply print it. The finally: block will be executed even if an unknown exception is thrown, one that we don't catch. Typically we close a server connection or a file handler here, since an exception thrown in the try block could leave the server connection open. Creating the server instance outside of the try block is necessary here otherwise we are trying to access a variable that would be out of scope if an exception is thrown.

The smtplib.SMTP methods all throw different variants of smtplib.SMTPException so you should look at the definition of the methods and then only catch the right exception type. They all inherit from smtplib.SMTPException so we can catch that type if we are lazy.

Ronny Andersson
  • 1,558
  • 13
  • 12
  • That's an improvement -- but you're still catching all exceptions with the `except Exceptions as e:` and then allowing the program to continue. I would suggest at least reraising it with a plain `raise` statement after the `print` statement and letting any higher-level control flow deal with it appropriately (including allowing the Python interpreter to terminate the script if it's not). – martineau Mar 08 '14 at 19:07
  • Yes. The OP did not check the exception at all, this is the reason for the simple print statement. Doing this properly would mean that we should check what type of exceptions the `smtplib.SMTP` class is expected to throw and then catch them. Finally we _could_ end in the "catch all" that we have here. I agree that catching everything and then just continuing is bad practice, unless that is what we want. – Ronny Andersson Mar 08 '14 at 19:23
0

I would suspect the very sensitive SPAM filtering systems which Google has, as you are able to get the alerts when you run the code line by line without any issues. May be introducing a delay by using time.sleep at some places could give the SPAM filter a feel that the disposition of an email is more human than automated.

Adding to that I would also suggest using an alternate SMTP server by any other provider too and I am sure that you would have already check your spam folder....

Venu Murthy
  • 2,064
  • 1
  • 15
  • 13