Add errno attribute to JackError exception

[SpotlightKid]

Makes it possible to determine the exact cause of a JackError exception by looking at the errno attribute of exception instances, instead of having to rely on parsing the exception's error message.

For instance, if creating a jack.Client fails, since no Client instance is created, the <client>.status property is not available. With this change, one could use the errno to check the status:

try:
    client = jack.Client('my-client')
except JackError as e:
   status = jack.Status(e.errno))
   if status.server_failed:
      ...
   elif status.version_error:
      ...

For other instances of this exception, errno is set to the error code returned by the JACK library function, whose call was not successful.

try:
    client.outports[0].write_midi_event(time, event)
except JackError as e:
    if e.errno == errno.ENOBUFS:
         ...

I chose to call the attribute errno, because some built-in exceptions use this name too, but it could be easily changed.

Signed-off-by: Christopher Arndt chris@chrisarndt.de

[mgeier]

Thanks for this PR!

I agree that it would be good to store the numeric error value in the exception.

But what about adding this to Exception.args instead of creating a new attribute?

https://docs.python.org/3/library/exceptions.html#BaseException.args

That's how I've done it in the sounddevice module: https://python-sounddevice.readthedocs.io/en/0.3.13/api.html#sounddevice.PortAudioError

Here it would be even simpler: args would be either a 1-tuple with the error message or a 2-tuple with error message and error code.

This way, you wouldn't have to implement __init__(), but instead you'd have to implement __str__().

In case of the "initializing" error, you could store the actual jack.Status instead of the numeric code. This way it would be possible to differentiate jack.Status codes from the other JACK error codes.

What do you think about this?

[SpotlightKid]

My reasons for adding an extra attribute whose value is passed by a keyword args instead of just adding to args by passing additional positional arguments were twofold:

a) To keep the string representation of JackError instances unchanged, in case existing code depends on it. If you add to args, the string representation would be the string representation of that tuple. That could be solved by overwriting the __str__ method too, though.

b) To keep exception handling code simple. You just need to check the value of the errno attribute. If you have a variable length args tuple, exception handling code needs to check for the length of the args tuple first, to determine if an error code is present, before it can check it. That's also the reason I decided to just put the status code of the client_open call into the errno attribute, instead of a Status instance, so exception handling code doesn't have to check for the errno attribute value type. We could add the Status instance to a separate status attributr, or, which is preferable IMO, raise a more specific exception sub-class here, called e.g. JackOpenError and add the Status instance to this.

[mgeier]

ad a) yes, this only makes sense with the __str__() method.

To keep the string representation of JackError instances unchanged, in case existing code depends on it.

I don't mind changing the string representation. Nobody should depend on it.

ad b) OK, that totally makes sense. Except that storing two different "kinds" of numbers in the same attribute would likely lead to confusion.

raise a more specific exception sub-class here

Yes, I think that would be the right solution in this case.

I think the JackError sub-classes could still use the args thing and their final string representation could be computed in their __str__() method.

[SpotlightKid]

I think the JackError sub-classes could still use the args

I'm not sure I understand. How would exception handling code look like then?

I'm currently a bit busy, so it will probably be a few days before I can address this.

[mgeier]

How would exception handling code look like then?

try:
    ...
except jack.SomeJackErrorWithErrno as e:
    msg, errno = e.args
    ...

But if you have a better idea, I'm of course open to it.

I'm currently a bit busy

No problem at all, take your time!

[SpotlightKid]

But then we'd have to always make sure that args is a two-item tuple, which then differs from the built-in exceptions. So why not make errno its own attribute?

[mgeier]

But then we'd have to always make sure that args is a two-item tuple

Yes, no problem, we just have to always call the constructor with two arguments.

which then differs from the built-in exceptions

Well, most built-in exceptions only have args and no further attributes.

But you are right, some of them have attributes, e.g.:

https://docs.python.org/3/library/exceptions.html#OSError https://docs.python.org/3/library/exceptions.html#SystemExit https://docs.python.org/3/library/exceptions.html#UnicodeError

So I think you are right and we should also use attributes.

I was hoping to avoid implementing constructors, but I guess it's not the end of the world.

So why not make errno its own attribute?

You are right, we should.

But only for the real "errno", the other type of exit code should have a different name (probably code?).

[mgeier]

Would you like to split the error into sub-classes? Or do you think a single exception class would be better?

[SpotlightKid]

Ok, I've split JackError into two separate exceptions. If you want a separate exception class for JackError with the errno attribute, so that JackOpenError doesn't share that attribute, let me know. But I think it only make things unnecessarily complex.

I've also added some documentation about where exceptions are raised to the docstrings, but I can put that in a separate PR, if you want.

Or just modify the PR yourself as you see fit.

[mgeier]

Thanks for the update!

I had imagined it working slightly differently, though.

Wouldn't it be better to save the raw error information in __init__() and only create the full error message in __str__()?

Right now, the two things seem to be conflated in https://github.com/spatialaudio/jackclient-python/pull/76/files#diff-f93af5423cded612b8e872f289e6cf2eR185-R186 and https://github.com/spatialaudio/jackclient-python/pull/76/files#diff-f93af5423cded612b8e872f289e6cf2eR2998

[mgeier]

I've added some of the things mentioned above in #79.

@SpotlightKid What do you think about this approach?

[SpotlightKid]

@mgeier Sorry for taking so long to answer. New job, less time for FLOSS.

Looks very good to me. Would buy. :)

[mgeier]

No problem, I'm not in a hurry.

Thanks for the feedback, I very much appreciate this. I'll merge #79 then.