I rencently had trouble finding a bug in an email processing script because of an error that pass silently in the email module.
The get_payload() method in the email.message module always return something when the decode argument is set to True.
This behaviour is well documented, but is their a reason to catch the decoding errors ?
Why not let them pop-up to the calling code ?

renaud

Renaud: providing a test script would help us here.

Daniel: i can't remember the exact scenario (i filled this bug 2 years
ago !)
after having a look back at email.message.Message.get_payload, i
remember the problem: the decoding errors are silented by the method and
you have no way to know if the decoding has been successful or not.
to find my malformed email i had to patch the module like that
(basically just removing the try/except blocks):

<patch>
--- message.py.orig	2009-04-03 23:46:47.000000000 +0200
+++ message.py	2009-04-03 23:48:27.000000000 +0200
@@ -192,19 +192,11 @@
             if cte == 'quoted-printable':
                 return utils._qdecode(payload)
             elif cte == 'base64':
-                try:
-                    return utils._bdecode(payload)
-                except binascii.Error:
-                    # Incorrect padding
-                    return payload
+                return utils._bdecode(payload)
             elif cte in ('x-uuencode', 'uuencode', 'uue', 'x-uue'):
                 sfp = StringIO()
-                try:
-                    uu.decode(StringIO(payload+'\n'), sfp, quiet=True)
-                    payload = sfp.getvalue()
-                except uu.Error:
-                    # Some decoding problem
-                    return payload
+                uu.decode(StringIO(payload+'\n'), sfp, quiet=True)
+                return sfp.getvalue()
         # Everything else, including encodings with 8bit or 7bit are
returned
         # unchanged.
         return payload
</patch>

once again, the behaviour is documented, so it's not really a bug.
but it caused me a lot of trouble (and it does not conforms very well to
"Errors should never pass silently.":)

but i guess applying such a patch could potentially break number of
client code so... is it worth the change?

Hmm, ISTM that a change that breaks existing code that relies on
documented behavior has a negligible chance of being accepted.

However, I agree that the current behavior isn't developer-friendly. I
think adding an alternative behavior to get_payload (add a new
parameter?), allowing callers to log errors and/or displaying warnings
could have a better chance of being accepted, while still providing the
control you want.

Barry?

good idea, why not something like sketched in the attached patch?
it does not break any existing code, while providing a way for new users
to have a chance to get the decoding errors.
of course, the doc should be updated accordingly, and tests should be added.

Looks good to me, adding tests and docs could be a nice Bug Day task.

Renaud,
Here's your patch with a test case against trunk.

looks very good to me.
thanks daniel for your work
renaud

To me a bug is a bug is a bug, but I'm sure there are different opinions so feel free to swap this back to feature request if you think that fits.  I'll try and have a crack at this tomorrow.

No, it's not a bug if it is working as documented.  (Well, it might be a design bug, but fixes to those are called "feature requests" :)

We're planning on addressing this is email6, by the way.

I'm OK with trying to get this into 3.2, but I'll be giving higher priority to bugs.

I've patched py3k test_email.py without patching message.py and the test passes on Windows Vista.  Either the patch is wrong, I've had finger problems or I'm misunderstanding the intent, can someone please advise.

I've double checked this and still all tests pass before patching message.py.  Could somebody else please pick this one up, thanks.

I've taken another look at this, and the email module is pretty consistent about just passing through data if it can't interpret it according to standards.  I think it would lead to a cluttered API if we add support for being strict and raising errors piecemeal.  So I'm deferring this to 3.3 and the email6 work, where we plan to have a comprehensive 'strict' mode.