zopefoundation / Acquisition

Acquisition is a mechanism that allows objects to obtain attributes from the containment hierarchy they're in.
Other
12 stars 12 forks source link

Additional debugging information for Acquisition error #61

Open darryldixon opened 2 years ago

darryldixon commented 2 years ago

_Acquisition.c has specific code for recognising when an Acquisition-wrapped item is being fed in to the Python Pickler, and it throws TypeError: "Can't pickle objects in acquisition wrappers." when this occurs.

The error is very opaque and difficult to track down what, actually, is being erroneously Acquisition wrapped and fed in to the Pickler. A sample (not Production-quality) improvement for this would be something like:

--- Acquisition-4.7/setup.py    2020-10-07 22:44:28.000000000 +1300
+++ Acquisition-4.7a/setup.py   2022-04-06 18:29:55.235552924 +1200
@@ -36,7 +36,7 @@
                   include_dirs=['include', 'src']),
     ]

-version = '4.7'
+version = '4.7a'

 setup(
     name='Acquisition',
diff -ru Acquisition-4.7/src/Acquisition/_Acquisition.c Acquisition-4.7a/src/Acquisition/_Acquisition.c
--- Acquisition-4.7/src/Acquisition/_Acquisition.c  2020-10-07 22:44:28.000000000 +1300
+++ Acquisition-4.7a/src/Acquisition/_Acquisition.c 2022-04-07 00:14:47.942013701 +1200
@@ -1477,8 +1477,16 @@
 PyObject *
 Wrappers_are_not_picklable(PyObject *wrapper, PyObject *args)
 {
+    PyObject *obj;
+    PyObject *repr;
+    /* C progammers disease... */
+    char msg[1024];
+    /* Unwrap wrapper completely -> obj. */
+    obj = get_base(wrapper);
+    repr = PyObject_Repr((PyObject *)obj);
+    snprintf(msg, 1024, "Can't pickle objects in acquisition wrappers - %s", (char *)PyBytes_AS_STRING(repr));
     PyErr_SetString(PyExc_TypeError,
-                    "Can't pickle objects in acquisition wrappers.");
+                    msg);
     return NULL;
 }
icemac commented 2 years ago

@d-maurer What do you think about this proposed change?

d-maurer commented 2 years ago

Michael Howitz wrote at 2022-4-12 23:10 -0700: @.*** What do you think about this proposed change?

I, too, had in the past met the error several times and remember that the analysis was not easy. Therefore, it can be good to provide more information for this error type.

Regarding the proposed details: I would not unwrap but directly apply PyObject_Str|Repr to the wrapper. The wrapper's str|repr typically provides valuable information beyond that of the base object (it usually includes the containment path). I am not sure whether I typically see repr(wrapper) or str(wrapper); that's why I use str|repr above; I would use what has a chance to provide information about the containment path.