Opened 5 years ago

Closed 5 years ago

TypeError: sequence item 0: expected string, int found

Reported by: Owned by: stroucki@… hasienda normal AccountManagerPlugin normal type error exception 0.11

Description

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.

comment:1 Changed 5 years ago by hasienda

• 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:
228     except Exception, e:
229         return {'error': ','.join(e.args)}
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 5 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 5 years ago by hasienda

• 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 5 years ago by hasienda

• Status changed from new to assigned

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 5 years ago by hasienda

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 follow-up: ↓ 8 Changed 5 years ago by stroucki@…

No objections, thank you.

comment:7 Changed 5 years ago by hasienda

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

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

comment:8 in reply to: ↑ 6 Changed 5 years ago by hasienda

No objections, thank you.

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