Urgent fixes needed - exception handler in _get in gwibber/microblog/twitter.py

[Ed S]

Binary package hint: gwibber

Hi,

This is with reference to http://bazaar.launchpad.net/~gwibber-committers/gwibber/trunk/annotate/head:/gwibber/microblog/twitter.py (I read revision 743). I am using gwibber 2.30.0.1-0ubuntu1 on Lucid, with only a single Twitter account.

The exception handler in _get() in twitter.py (try/except block at lines 131-138) seems to be receiving unforeseen/unexpected exceptions. The code in the except block causes an exception of its own, masking the original error and making bug reports with gwibber-service logs non-useful for debugging.

Errors that seem to end up in this handler:

Error 1. Twitter authentication failure (original purpose of the handler).

Error 2. Unsupported locale setting - see https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/533017/comments/24 and comment 25

Error 3. Network-related problems (DNS failure, failure to connect to Twitter/retrieve JSON) - see https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/530195/comments/34 for a pastebin log; the later part of the log at https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/530195/comments/21

When each of these errors occurs, the "data" variable has a different data type:

Error 1 (auth failure): dictionary with single item whose key is "errors" (not "error" - see https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/533017/comments/26 )

Error 2 (locale problem): a _list_ containing multiple dictionaries (timeline or other valid data received from Twitter).

Error 3 (network/DNS): str.

Effects of current code:

For Errors 2 and 3, the _if data.has_key("error")_ check results in an AttributeError exception since the "data" variable is the wrong data type. This results in the original error being lost, and a useless stack trace printed to debug log.

For Error 1, the auth failure is not detected and the GwibberProtocolError is not raised, because the key checked for is "error" and not "errors". I haven't read other (calling) code to find out how this is handled, but I confirm that any "wrong credentials" error message or notification to user is not showing up.

Ideas/suggestions:

Network/DNS problems are going to happen. A small notification (an icon or tooltip) to warn the user that no (new) messages are displayed because of network problems will help improve usability. For that, the corresponding error has to be trapped here, and a useful exception raised that can be handled by GUI code.

The auth failure can be handled by changing the sought key from "error" to "errors", and perhaps introducing another check before _if data.has_key("error")_ -- something like _if type(data) == types.DictionaryType_. Also, the check for "authenticate" in data["errors"] probably needs to be fixed, since it is a dictionary being returned now, not a string - see https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/533017/comments/26

The locale problem may take a while to work out (sorry, no ideas about that) but in the interim, may I please ask that the except line assign the exception to a variable, as in https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/533017/comments/24, so that at least the encountered exception can be printed in the debug logs. This can be the fallback "final case" if the checks for auth failure and network problems exceptions are not triggered. It will also be useful if any other unforeseen exceptions manifest themselves here.

I hope one of the developers will please pay attention to this area soon, because multiple bug reports with apparently different root causes are being submitted with the AttributeError stack trace, and this is bound to be confusing to people trying to help other users with bugs, or trying to triage bugs, especially if they are not familiar with the code.

Thank you for your urgent consideration of this issue,
Ed.

[Ed S (edgar-b-dsouza)]

https://bugs.launchpad.net/ubuntu/+source/gwibber/+bug/530195/comments/72 looks like it addresses at least some of the issues mentioned in this bug. Please consider the patch for testing and inclusion into trunk.

Thanks,
Ed.

[Omer Akram (om26er)]

gwibber 2.30.1 and 2.31.2 were released.

[Omer Akram (om26er)]

gwibber 2.30.1 is in lucid-proposed

[Omer Akram (om26er)]

gwibber 2.30.1 is now in Lucid updates.