try/exception - error block

[Steven D'Aprano]

You also posted this question to the tutor at python.org mailing list, which is
where I gave an answer. This is a much better place, so I'll re-post the
most important parts of my answer here. If you read nothing else, scroll
down to the end and read the last part of my comment.


bruce wrote:

> I have a long running process, it generates calls to a separate py
> app. The py app appears to generate errors, as indicated in the
> /var/log/messages file for the abrtd daemon.. The errors are
> intermittent.

Well, what do the errors say?


[...]
> if __name__ == "__main__":
> # main app
>
>   try:
[deleting lots of commented out code]
>     if(len(sys.argv)<2):
>       print "error\n"
>       sys.exit()

You really should raise an exception on errors, but if you insist on
doing things this way, you should print to stderr, not stdout, and you
should exit with a non-zero status:

    print >>sys.stdout, "descriptive error messages are better\n"
    sys.exit(101)

>     a=sys.argv[1]
>     aaa=a

A minor stylistic thing: you can write this as:

    a = aaa = sys.argv[1]

but of course you really ought to use descriptive variable names rather
than cryptic "a" and "aaa" and "z" and other meaningless names.


[deleting more fossil code]
>     z=simplejson.loads(urllib.unquote_plus(a))
>     print z
>     url=str(z['currentURL'])
>     level=str(z['level'])
>     cname=str(z['parseContentFileName'])

>     cmd='echo ${yolo_clientParseInputDir}/'
>     proc=subprocess.Popen(cmd, shell=True,stdout=subprocess.PIPE)
>     cpath=proc.communicate()[0].strip()

Hmmm. Are you trying to read the value of an environment variable using
subprocess? If so, then try this instead:

    cpath = os.getenv('yolo_clientParseInputDir')

If not, then sorry for the noise. Perhaps you could explain what your
call to echo in the shell is meant to do?

>     cname=cpath+cname

>     cmd='test -e '+cname+' && echo 1'
>     proc=subprocess.Popen(cmd, shell=True,stdout=subprocess.PIPE)
>     c1=proc.communicate()[0].strip()

And here I think you're trying to test whether a file exists?

    os.path.exists(cname)

>     if(not c1):
>       #got an error - process it, return
>       print "error in parse"

Um, surely not? Surely the error is that the file doesn't exist, not
that it is a parsing error?

>     with open(cname,"r") as myfile:
>       content=myfile.read()
>       myfile.close()

If you use the "with open" form, there is no need to manually close the
file, it will be automatically closed for you.

    with open(cname,"r") as myfile:
        content = myfile.read()

is all you need (assuming you have permission to open the file, and that
it still exists).

[lots more fossils deleted]
>     ret={} # null it out to start
>     if (level=='rState'):
>       ret=getParseStates(content)
>     elif (level=='stateCollegeList'):
>       ret=getParseStateCollegeList(url,content)
>     elif (level=='collegeFaculty1'):
>       ret=getParseCollegeFacultyList1(url,content)
>     elif (level=='collegeFaculty2'):
>       ret=getParseCollegeFacultyList2(content)

I'm not really sure if any of that code is relevant to the problem
you're having.

>     a={}
>     status=False
>     if(ret['status']==True):
>       s=ascii_strip(ret['data'])
>       if(((s.find("</html")>-1) or (s.find("</HTML")>-1)) and
>           ((s.find("<html")>-1) or (s.find("<HTML")>-1)) and
>            level=='classSectionDay'):
>         status=True
>       a['Status']=True
>       a['recCount']=ret['count']
>       a['data']=ret['data']
>       a['nextLevel']=''
>       a['timestamp']=''
>       a['macAddress']=''
>     elif(ret['status']==False):
>       a['Status']=False
>       a['recCount']=0
>       a['data']=''
>       a['nextLevel']=''
>       a['timestamp']=''
>       a['macAddress']=''
>     res=urllib.quote(simplejson.dumps(a))

Your code will be much, much, much more readable if you use a reasonable
indent between levels. Four spaces rather than two, or a tab. I'm
finding it quite difficult to keep track of the levels when they are so
close together.

>     name=subprocess.Popen('uuidgen -t', shell=True,stdout=subprocess.PIPE)
>     name=name.communicate()[0].strip()
>     name=name.replace("-","_")
>     name2=tmpParseDir+"/rr_"+name+".dat"
>     ofile1=open(name2,"w+")
>     ofile1.write(res)
>     ofile1.close()
>     print name2

So does this file get written to?

Does name2 get printed?


>     if status==False:
>       sname=tmpParseDir+"/serr_"+name+".dat"
>       ofile1=open(sname,"w+")
>       ofile1.write(aaa)
>       ofile1.close()

How about this one? Does it get written to?

>     sys.exit()

Since you exit here, the rest of the code in the block is never
executed:

>     print "term = "+str(termVal)+"\n"
>     print "url = "+url+"\n"
>     getTerm(url,college,termVal)
>     print "exit"
>     sys.exit()

That's all dead code. I hope it isn't important.

>   except Exception, e:
>     print e
>     print "pycolFac1 - error!! \n";

Does that get printed?

>     name=subprocess.Popen('uuidgen -t', shell=True,stdout=subprocess.PIPE)
>     name=name.communicate()[0].strip()
>     name=name.replace("-","_")
>     name2="/home/ihubuser/parseErrTest/pp_"+name+".dat"
>     ofile1=open(name2,"w+")
>     ofile1.write(e)
>     ofile1.write(aaa)
>     ofile1.close()
>     sys.exit()

That's awfully ambitious code for an except clause that you're not even
sure is working. Simplify, simplify, simplify.

    except Exception, e:
        with open("/tmp/myerror.txt", "w") as f:
            f.write("%r" % e)
        sys.exit(102)


Now you should be able to see the error written to the file, which you
should have write privileges to unless you're doing something very
unusual.




-- 
Steven