Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error description to BleakDBusError in addition to error name #522

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

bojanpotocnik
Copy link
Contributor

Converting BleakDBusError to string now adds detailed description of the
error, which can be very useful for debugging, as more than 12 different
errors are reported under org.bluez.Error.Failed error name.

Instead of

bleak.exc.BleakDBusError: org.bluez.Error.Failed

one now gets

bleak.exc.BleakDBusError: org.bluez.Error.Failed (Operation failed with ATT error: 0x0e = Connection Rejected Due To Security Reasons)

Some details are less useful, as example

bleak.exc.BleakDBusError: org.bluez.Error.AuthenticationFailed

becomes

bleak.exc.BleakDBusError: org.bluez.Error.AuthenticationFailed (Authentication Failed)

But looking at more possible error codes and their details (majority
from BlueZ sources gdbus/object.c and src/gatt-client.c), it becomes
clear that additional description of error can be very useful:

  org.freedesktop.DBus.Error.InvalidArgs
  org.freedesktop.DBus.Error.UnknownProperty

    No such interface '%s'
    No such property '%s'
    Invalid signature for '%s'
    Invalid argument type: '%c'
    Property '%s' is not readable
    Property '%s' is not writable
    No arguments given
  org.bluez.Error.Failed

    `strerror(-err)`
    Connect failed
    Not connected
    Invalid UUID
    Operation failed with ATT error: 0x%02x
    Failed to initiate write
    ... and many more

@dlech
Copy link
Collaborator

dlech commented Apr 22, 2021

This is cool. I didn't know that D-Bus included extra info like this.

To keep things simple and to make sure we don't miss anything useful, I wouldn't mind always including the extra info even if it is redundant in some cases.

@dlech
Copy link
Collaborator

dlech commented Apr 22, 2021

Thinking about this a bit more, it occurred to me that it might be nice to format this similar to OSError where the machine readable part is in square braces, i.e. f"[{args[0]}] {args[1]}" which would become bleak.exc.BleakDBusError: [org.bluez.Error.Failed] Operation failed with ATT error: 0x0e = Connection Rejected Due To Security Reasons. Thoughts?

@bojanpotocnik
Copy link
Contributor Author

Good idea.

bleak.exc.BleakDBusError: [org.bluez.Error.AuthenticationFailed] Authentication Failed
bleak.exc.BleakDBusError: [org.bluez.Error.Failed] Operation failed with ATT error: 0x0e (Connection Rejected Due To Security Reasons)

But what about when there is no detail?

bleak.exc.BleakDBusError: [org.bluez.Error.AuthenticationFailed]

or

bleak.exc.BleakDBusError: org.bluez.Error.AuthenticationFailed

🤔 I guess if we are talking about machine readability, consistency shall have priority, so the first one?

@dlech
Copy link
Collaborator

dlech commented Apr 22, 2021

consistency shall have priority, so the first one?

I agree with that.

@bojanpotocnik
Copy link
Contributor Author

Done, the format is now

bleak.exc.BleakDBusError: [name] Details (further parsed details)
bleak.exc.BleakDBusError: [org.bluez.Error.Failed]
bleak.exc.BleakDBusError: [org.bluez.Error.Failed] Authentication Failed
bleak.exc.BleakDBusError: [org.bluez.Error.Failed] Operation failed with ATT error: 0x0e (Connection Rejected Due To Security Reasons)

bleak/exc.py Outdated Show resolved Hide resolved
bleak/exc.py Outdated Show resolved Hide resolved
bleak/exc.py Outdated Show resolved Hide resolved
bleak/exc.py Outdated Show resolved Hide resolved
bleak/exc.py Outdated Show resolved Hide resolved
Converting BleakDBusError to string now adds detailed description of the
error, which can be very useful for debugging, as more than 12 different
errors are reported under org.bluez.Error.Failed error name.

Instead of
  bleak.exc.BleakDBusError: org.bluez.Error.Failed
one now gets
  bleak.exc.BleakDBusError: org.bluez.Error.Failed (Operation failed
  with ATT error: 0x0e = Connection Rejected Due To Security Reasons)

Some details are less useful, as example
  bleak.exc.BleakDBusError: org.bluez.Error.AuthenticationFailed
becomes
  bleak.exc.BleakDBusError: org.bluez.Error.AuthenticationFailed
  (Authentication Failed)

But looking at more possible error codes and their details (majority
from BlueZ sources gdbus/object.c and src/gatt-client.c), it becomes
clear that additional description of error can be very useful:
  org.freedesktop.DBus.Error.InvalidArgs
  org.freedesktop.DBus.Error.UnknownProperty
    No such interface '%s'
    No such property '%s'
    Invalid signature for '%s'
    Invalid argument type: '%c'
    Property '%s' is not readable
    Property '%s' is not writable
    No arguments given
  org.bluez.Error.Failed
    `strerror(-err)`
    Connect failed
    Not connected
    Invalid UUID
    Operation failed with ATT error: 0x%02x
    Failed to initiate write
    ... and many more

Signed-off-by: Bojan Potočnik <info@bojanpotocnik.com>
Copy link
Collaborator

@dlech dlech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants