After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 318135 - Improvements to error handling
Improvements to error handling
Status: RESOLVED FIXED
Product: pyspi
Classification: Deprecated
Component: general
CVS HEAD
Other All
: Normal normal
: ---
Assigned To: PySPI Maintainers
PySPI Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-10-06 18:36 UTC by Dave Malcolm
Modified: 2006-07-27 23:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Unit test to force a communication error with the app under test (849 bytes, patch)
2006-07-18 13:19 UTC, Dave Malcolm
none Details | Review
Work-in-progress patch for pyspi (6.62 KB, patch)
2006-07-18 13:29 UTC, Dave Malcolm
none Details | Review
Working version of exception handling (40.34 KB, patch)
2006-07-18 16:04 UTC, Dave Malcolm
committed Details | Review
Working version of unit test for losing communication with app (873 bytes, patch)
2006-07-18 16:11 UTC, Dave Malcolm
committed Details | Review
Patch for sniff to deal with uncontactable nodes (2.12 KB, patch)
2006-07-18 16:49 UTC, Dave Malcolm
committed Details | Review

Description Dave Malcolm 2005-10-06 18:36:40 UTC
Please describe the problem:
Currently pyspi throws away AT-SPI errors
We should install an error handler callback, and use this to raise a Python
exception when errors occur in an AT-SPI call

Steps to reproduce:
1. Run a Dogtail script talking to an application
2. Kill the application, or kill at-spi-registryd



Actual results:
Watch various errors scroll by, printed to the terminal.

Expected results:
An exception should be raised in Python

Does this happen every time?


Other information:
I attempted to fix this (some stubs are in CVS, IIRC), didn't work; I think
there's the wrong level of indirection for the callback function pointer, which
is confusing Pyrex.

(NB exceptions inside Pyrex get thrown away.  We may need to save the error data
in the callback, then check after every C function call if an error state is set)
Comment 1 Zack Cerza 2005-10-06 18:48:48 UTC
Yeah, this one's annoying.
Comment 2 Dave Malcolm 2006-07-18 13:19:20 UTC
Created attachment 69110 [details] [review]
Unit test to force a communication error with the app under test

Doesn't yet pass since the exceptions aren't getting through yet.

Does generate an "IDL:omg.org/CORBA/COMM_FAILURE:1.0" message as expected (except for a case where all of dogtail gets stuck, confused over unicode conversion)
Comment 3 Dave Malcolm 2006-07-18 13:29:17 UTC
Created attachment 69112 [details] [review]
Work-in-progress patch for pyspi

Here's the furthest I've got with this.  This patch successfully registers an error handler with AT-SPI, and generates SPIException instances containing as much data as I think we can scrape, and the data appears to be correct.  It's still got lots of debug prints in it, because this stuff is hard to debug - pyrex likes to eat exceptions :-)

Unfortunately, actually _raising_ these exceptions is awkward.  We can't raise them from inside a pyrex callback, as pyrex simply throws these away (does pyrex have a workaround for this???)  

So I've tried a couple of different ways of adding exception hooks around places where exceptions could occur, storing the generated exception within a global.  Ideally we'd do this automatically somehow, since there are numerous places where problems could occur.

But so far I haven't got either approach to work.

So this is further along, but not yet working.
Comment 4 Dave Malcolm 2006-07-18 14:09:35 UTC
Judicious use of Pyrex's "except *" clause may fix this; see e.g. http://www.obviousobscurity.org/?page_id=13
Comment 5 Dave Malcolm 2006-07-18 16:04:39 UTC
Created attachment 69126 [details] [review]
Working version of exception handling

This patch:
(i) makes use of pyrex's "except *" feature to tell pyrex to add exception propagation hooks (which I believe are not in by default for performance reasons); these make exceptions work inside callbacks
(ii) introduces a SpiException class containing a scraped copy of the SPIException data, which is raised

Works for me.  The only caveat is that we now have two exception classes, the existing AtspiException class can probably be replaced using the "except" pyrex clause (or renamed, perhaps?)
Comment 6 Dave Malcolm 2006-07-18 16:11:43 UTC
Created attachment 69127 [details] [review]
Working version of unit test for losing communication with app

The initial version of the unit test didn't work; this one does
Comment 7 Dave Malcolm 2006-07-18 16:49:38 UTC
Created attachment 69129 [details] [review]
Patch for sniff to deal with uncontactable nodes

The attached patch extends sniff by dealing with SpiException errors, so that if you're browsing inside a uncontactable application (e.g. it's crashed), you get clear visible feedback of this, rather than sniff crashing.
Comment 8 Dave Malcolm 2006-07-18 16:55:17 UTC
Attachment 69126 [details] probably fixes bug 332867 as well
Comment 9 Dave Malcolm 2006-07-27 22:53:03 UTC
Committed the pyspi part of this
Comment 10 Dave Malcolm 2006-07-27 23:01:25 UTC
Committed the sniff fix.

Probably should add a unit test to ensure that exceptions are raised when an app dies
Comment 11 Dave Malcolm 2006-07-27 23:06:53 UTC
Gah - yes, this was one of the attachments.  Committed now.

This bug should now be fixed - resolving.