Issue 11694: xdrlib raises ConversionError in inconsistent way

xdrlib defines ConversionError, but very seldom uses it. For example:

    def pack_float(self, x):
        try: self.__buf.write(struct.pack('>f', x))
        except struct.error as msg:
            raise ConversionError(msg)

But it doesn't do so here:

    def pack_uint(self, x):
        self.__buf.write(struct.pack('>L', x))

Shouldn't that be more consistent?

I am happy to write a patch, that will make xdrlib raise ConversionError, as well as write proper test (I believe xdrlib tests should get some love altogether, so I would add a separate test case for this).

This seems like a bug worth fixing. The ConversionError exception has been documented, an there's an example in the docs that suggest that at least all packing fails with a ConversionError.

Patch with tests.

Bump! It's almost 3 months since I posted the patch, so I would like to remind about this bug.

Looks good to me.

I see one obvious issue with the patch: The ConversionErrors it creates are passed the struct.error or TypeError instance as a parameter. The first argument of these exceptions would be better, i.e.

try:
   ...
except struct.error as e:
   raise ConversionError(e.args[0])

Furthermore, my ear thinks that raises_conversion_error would be a better name for the decorator than raising_conversion_error.

Anyway, I think the whole ConversionError is a bit problematic, as either TypeError or ValueError would be the most appropriate exception, depending on the situation. For example:

p = Packer()
p.pack_int('foo')  # should raise a TypeError
p.pack_int(2**100) # should raise a ValueError

This would be slightly harder to implement, though, as struct.error has exactly the same problem.

Here's a refreshed patch:

- raising_conversion_error is now raise_conversion_error
- the decorator uses functools.wraps
- the ConversionError is instantiated with the first argument of the caught expression
- the reraising of ConversionError has the exception context supressed.

LGTM

New changeset 9cee201388c9 by Petri Lehtinen in branch '2.7':
Issue #11694: Raise ConversionError in xdrlib as documented
https://hg.python.org/cpython/rev/9cee201388c9

New changeset 7ef6e5f53418 by Petri Lehtinen in branch '3.4':
Issue #11694: Raise ConversionError in xdrlib as documented
https://hg.python.org/cpython/rev/7ef6e5f53418

New changeset 8e3387f566f6 by Petri Lehtinen in branch 'default':
#11694: merge with 3.4
https://hg.python.org/cpython/rev/8e3387f566f6

Applied, thanks!

stage: patch review -> resolved

messages: + msg228893
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3