On 2021-09-30 11:23 a.m., Chris Angelico wrote:
For example this: (real code)
def get_property_values(self, prop): try: factory = self.get_supported_properties()[prop] except KeyError as exc: raise PropertyError from exc iterator = factory(self._obj) try: first = next(iterator) except StopIteration: return (x for x in ()) except abdl.exceptions.ValidationError as exc: raise LookupError from exc except LookupError as exc: raise RuntimeError from exc # don't accidentally swallow bugs in the iterator return itertools.chain([first], iterator)
There's a lot of exception transformation happening here, and I don't understand, without context, the full purpose of it all. But the fact that you're raising LookupError directly seems a tad odd (it's a parent class for IndexError and KeyError), and it seems like a custom exception type would solve all of this. I *think* what's happening here is that you transform ValidationError into LookupError, but only if it happens on the very first yielded result?? Then how about:
class PropertyNotFoundError(LookupError): pass
def get_property_values(self, prop): try: factory = self.get_supported_properties()[prop] except KeyError as exc: raise PropertyError from exc iterator = factory(self._obj) try: yield next(iterator) except StopIteration: pass except abdl.exceptions.ValidationError as exc: raise PropertyNotFoundError from exc yield from iterator
If the factory raises some other LookupError, then that's not a PropertyNotFoundError. And if it *does* raise PropertyNotFoundError, then that is clearly deliberate, and it's a signal that the property is, in fact, not found.
Your code, as currently written (and including the rewrite), is *still* capable of leaking a faulty exception - just as long as the factory returns one good value first. So I'm not sure what you actually gain by this transformation.
Okay let us stop you right there. This generator is not a function. Generators don't do anything until iterated. There's a HUGE semantic difference there. You're making assumptions that you could easily invalidate yourself if you paid attention to the original code. Don't do that. try: iterator = foo.get_property_values(prop) except PropertyError, etc: pass # handle it for thing in iterator: pass # exceptions in the iteration actually get propagated because they mean something went wrong and the program should terminate. in the real code, this includes spurious ValidationError etc, because those are *never* supposed to happen.