Proposal. Backend.start should return a boolean value

[heuer]

The Backend.start() method does not only start the backend but checks the result, destroys it in case of an error and raises a RuntimeException in case of an error.

IMO it would be much cleaner if start() returns a boolean and stops itself from cleaning up.

Compositors could easily check the result from Backend.start() and react in their way in case of an error.

I've found more candidates where a boolean or None value would make more sense (Renderer.autocreate(), Renderer.init_display(), Allocator.autocreate()) and pywlroots should not raise a RuntimeException.

If there is a general consensus about reducing the exceptions and replacing them with boolean values or None, I could submit a PR.

Or if there is no general agreement, at least the deviations from wlroots (i.e. calling destory() and a runtime error) should be better documented

[heuer]

See also #135

[flacjacket]

Hi @heuer, thanks for the feedback here! The rationale I had for raising an exception here rather than using a return code is channeling Raymond Hettinger's "Beyond PEP 8" PyCon talk, in that we have more naturally suited tools in Python for reporting and handling errors than the underlying C library than return codes. Ensuring return codes are always checked in all clients is more error prone, and if there is clean-up that needs to be done on failure we can encapsulate that here for every use case rather than needing to have that repeated everywhere. Certainly if there are things that we are doing in the attempted clean-up that wouldn't necessarily be the thing that we want to always have happen we can go about changing that.

That said, I'm sure there are cases where return codes would come in handy. In the cases you mention, what would be the benefit of having the return code rather than the exception?

[heuer]

Thanks for your reply. Acc. to my understanding all applications have to catch the exception since a wayland.Display was already created which have to be destroyed as well:

try:
    server.backend.start()
except RuntimeError as ex:
    # backend was destroyed already, more cleanup necessary
    server.display.destroy()
    raise ex

If Backend.start() just returns a boolean value, it would lead to more, at least in my opinion, elegant code which communicates the intention better.

if not server.backend.start():
    server.backend.destroy()
    server.display.destroy()
    raise RuntimeException("Problem starting backend")

And it would be closer to the wlroots API. At least the fact that the backend tries to destroy itself in case of an error was very surprising for me.