create PyKeePassException
There are lots of cases right now where PyKeePass allows internal exceptions to bubble up to the end-user. We should consider catching some of these where it makes sense and raising a PyKeePassException that is more descriptive.
e.g. A user tries to dereference a field when the reference doesn't exist
>>> kp.entries[0].username = '{REF:U@I:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA}'
>>> kp.deref(kp.entries[0].username)
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
Cell In[23], line 1
----> 1 kp.deref(kp.entries[0].username)
File ~/resources/pykeepass/pykeepass/pykeepass.py:703, in PyKeePass.deref(self, value)
701 search_value = uuid.UUID(search_value)
702 ref_entry = self.find_entries(first=True, **{search_in: search_value})
--> 703 value = value.replace(ref, getattr(ref_entry, wanted_field))
704 return self.deref(value)
AttributeError: 'NoneType' object has no attribute 'username'
I'd like to collate a list of these cases here. Maybe @A6GibKm @FalkAlexander @firecat53 have some examples?
I did a grep for except : in Secrets and the results I got (not including except GLib.Error ...) are
-
PyKeePass(...) -
.save() -
.atime,.mtime,.ctime, and.expiry_time,see https://gitlab.gnome.org/World/secrets/-/issues/439 -
entry.custom_properties.items(), see https://gitlab.gnome.org/World/secrets/-/issues/413
For me the most important thing would be to document what calls can fail. The secondary one would be to have descriptive errors in the library and only throw those.
For example, .save() can clearly fail due to IO and it would be nice to do except PyKeePassException.IO or something of the sort.
I just stumbled into https://gitlab.gnome.org/World/secrets/-/issues/525. While its definitively my fault to not try to catch a broad exception (see related MR) it shows a bit the pitfalls of manually trying to handle exceptions that might come from outside the library.
I'm not coming up with anything obvious other than the deref and IO examples you two listed already.
I also agree that getting too aggressive about catching too many exceptions could cause some debugging challenges when getting bug reports from people.
Do you think that the way @br-olf handles the deref issue in #386 (returning None if the referenced entry is missing) is reasonable or should it return an exception to be caught? I can handle either way (his fix is pretty simple to catch in my client)...just not sure if there's a "right" way in this case.