Modify

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#5538 closed defect (fixed)

global name 'body' is not defined

Reported by: anonymous Owned by: doki_pen
Priority: normal Component: AnnouncerPlugin
Severity: critical Keywords:
Cc: Trac Release: 0.11

Description

This happens if the mime_encoding in trac.ini is set to none

2009-07-21 16:19:39,663 Trac[api] ERROR: AnnouncementSystem failed.
Traceback (most recent call last):
  File "build\bdist.win32\egg\announcerplugin\api.py", line 369, in _real_send
    distributor.distribute(transport, packages[transport], evt)
  File "build\bdist.win32\egg\announcerplugin\distributors\email_distributor.py", line 204, in distribute
    formats[format], None, to, public_cc)
  File "build\bdist.win32\egg\announcerplugin\distributors\email_distributor.py", line 287, in _do_send
    dummy = body.encode('ascii')

Attachments (0)

Change History (9)

comment:1 Changed 5 years ago by anonymous

I had to fix this yesterday. Got things working by commenting out that chunk of code. I'm amazed that it works for anyone else with this issue.

--- announcerplugin/distributors/email_distributor.py   (revision 6703)
+++ announcerplugin/distributors/email_distributor.py   (working copy)
@@ -282,12 +282,12 @@
                     to_unicode(provided_headers[key])
         rootMessage['Date'] = formatdate()
         # sanity check
-        if not self._charset.body_encoding:
-            try:
-                dummy = body.encode('ascii')
-            except UnicodeDecodeError:
-                raise TracError(_("Ticket contains non-ASCII chars. " \
-                                  "Please change encoding setting"))
+        #if not self._charset.body_encoding:
+        #    try:
+        #        # dummy = body.encode('ascii')
+        #    except UnicodeDecodeError:
+        #        raise TracError(_("Ticket contains non-ASCII chars. " \
+        #                          "Please change encoding setting"))
         del rootMessage['Content-Transfer-Encoding']
         rootMessage.set_charset(self._charset)
         rootMessage['Subject'] = Header(subject, self._charset)

comment:2 Changed 5 years ago by anonymous

  • Owner changed from ixokai to doki_pen

comment:3 Changed 5 years ago by doki_pen

  • Resolution set to wontfix
  • Status changed from new to closed

A better solution would have been to set your encoding as the exception suggests.

[announcer]
mime_encoding = 'base64'

Read the description of the option:

    mime_encoding = Option('announcer', 'mime_encoding', 'base64',
        """Specifies the MIME encoding scheme for emails.
        
        Valid options are 'base64' for Base64 encoding, 'qp' for
        Quoted-Printable, and 'none' for no encoding. Note that the no encoding
        means that non-ASCII characters in text are going to cause problems
        with notifications (''since 0.10'').""")

comment:4 follow-up: Changed 5 years ago by anonymous

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I just got hit by this.

I agree that setting the mime_encoding is an easy fix. However, even so, it is still wrong for the plugin to just die with a Python NameError.

The plugin should either fall back to a basic format (like the original attempt to use ascii, which is still broken) or else it should log something educational like, "Set mime_enconding in your [announcer] setting in trac.ini"

Just having a random traceback is not an elegant way to handle this configuration error.

Finally, if you fall back to a default encoding, why not fall back to a default of base64?

comment:5 in reply to: ↑ 4 Changed 5 years ago by anonymous

Replying to anonymous:

Sorry, I forgot to mention in my last comment:

My trac.ini had a default mime_encoding value of none. I didn't put that there... but that is supposed to be a valid selection (as per the docs cited above) so in my opinion this is an actual bug. The plugin is supposed to handle "none" correctly.

I'm still trying to get HTML notifications to work here... but thanks for working on this, a lot of people really want pretty-looking notifications. Not everybody appreciates courier font as much as the Trac developers :)

comment:7 Changed 5 years ago by doki_pen

Yeah, body object doesn't exist. Not sure why this code sucks. I'll fix it.

comment:8 Changed 5 years ago by doki_pen

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [7280]) Update body reference to output.

Since body isn't defined. Fixes #5538

comment:8 Changed 5 years ago by doki_pen

(In [7280]) Update body reference to output.

Since body isn't defined. Fixes #5538

comment:9 Changed 4 years ago by doki_pen

(In [7589]) updates body reference to output.

Since body isn't defined. Fixes #5538
merge -r7280

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from doki_pen. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.