Modify

Opened 13 years ago

Closed 13 years ago

Last modified 5 years ago

#8834 closed defect (fixed)

TypeError: sequence item 0: expected string, int found

Reported by: stroucki@… Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: type error exception
Cc: Trac Release: 0.11

Description

224 new_password = self._random_password() 225 mgr = AccountManager(self.env) 226 try: 227 mgr._notify('password_reset', username, email, new_password) 228 except Exception, e: 229 return {'error': ','.join(e.args)} 230 mgr.set_password(username, new_password) 231 if mgr.force_passwd_change: 232 db = self.env.get_db_cnx() 233 cursor = db.cursor()

In line 229 here, if the error returning is something like (111, 'Connection refused), the statement will fail because 111 is not a string. Changing to join(map(str, e.args)) will convert all members to string and remove the error.

Attachments (0)

Change History (8)

comment:1 Changed 13 years ago by Steffen Hoffmann

Keywords: needinfo added; conversion removed

Sorry, didn't saw this coming in. Maybe email choke on the bad formatting? A little bit of formatting works wonders (just add {{{ and on the next new line #!python, both on top and another set of }}} on a line on it's own below the piece of code - similar for error messages). Result for the code above (impossible to understand otherwise):

224     new_password = self._random_password()
225     mgr = AccountManager(self.env)
226     try:
227         mgr._notify('password_reset', username, email, new_password)
228     except Exception, e:
229         return {'error': ','.join(e.args)}
230     mgr.set_password(username, new_password)
231     if mgr.force_passwd_change:
232         db = self.env.get_db_cnx()
233         cursor = db.cursor()

Another point: I still had to guess or look-up proper indentation...

I don't argue with the intention. You're trying to improve the code, that's fine. But why do you let me/us jump through unnecessary loopholes then? Easy for you to tell us, it's acct_mgr.web_ui what we're looking at here, right? I doubt, this is the only occurrence of unmapped error messages in this plugin, right? Did you check?

Considering your line numbers you're certainly not looking at the trunk branch of AccountManagerPlugin. So but what's your version? Did you ever experience a HTTP 111 error, and if YES, under what circumstances?

Your reasoning is valid and possibly valuable for the ongoing development. Thanks for taking care, taking your time to report and in advance for more feedback to elaborate towards a valid (global?) fix. Please follow-up.

comment:2 Changed 13 years ago by stroucki@…

Thanks for finding the file and formatting - it was late and I forgot. I can't see what version it was anymore, but if the code in trunk is like this the error will still happen in the same circumstance. If you let me know where I can find the version, I'll let you know.

I don't know what you mean by unmapped error message. It was an exception raised within the exception handler. I received 111, Connection refused (not an http error) when the smtp server specified wasn't actually listening for connections. The exception handler which got executed as the connection failed then excepted itself, with the above message.

Whether similar bugs exist elsewhere, maybe. I ran into this specific one and I fixed it. I wanted to let you know so you could fix it and so others could see a solution.

comment:3 Changed 13 years ago by Steffen Hoffmann

Keywords: needinfo removed

Sure, thanks for the quick reply. I've never seen a non-graceful handling of notification message sending failures, so I didn't expect that now.

I'll try to reproduce according to your hints.

Never mind about the plugin version. This is most probably valid for 0.11 and trunk branch. Since I have a full copy of the repository here, I can check this in a minute. Just begging you to provide as much inside as possible, even if it looks irrelevant to yourself.

comment:4 Changed 13 years ago by Steffen Hoffmann

Status: newassigned

A development note: There is an i18n related issue with str we expect Unicode chars here (tested), so we'll replace it with to_unicode from trac.util.text.

comment:5 Changed 13 years ago by Steffen Hoffmann

Found no other occurrence other than in the 0.11 branch to fix. BTW, line numbers were matching there.

Changes are prepared and ready for commit now. Any objections against mentioning your email in the public commit message for the repository, or what other name would you prefer for credit?

comment:6 Changed 13 years ago by stroucki@…

No objections, thank you.

comment:7 Changed 13 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(In [10259]) AccountManagerPlugin: Prevent exception in exception handler, closes #8834.

comment:8 in reply to:  6 Changed 13 years ago by Steffen Hoffmann

Replying to stroucki@andrew.cmu.edu:

No objections, thank you.

Thanks, I've been just too fast, sorry. I'm still grateful for your help.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.