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

create PyKeePassException #385

Open
Evidlo opened this issue Mar 11, 2024 · 3 comments
Open

create PyKeePassException #385

Evidlo opened this issue Mar 11, 2024 · 3 comments

Comments

@Evidlo
Copy link
Member

Evidlo commented Mar 11, 2024

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?

@A6GibKm
Copy link
Contributor

A6GibKm commented Mar 11, 2024

I did a grep for except : in Secrets and the results I got (not including except GLib.Error ...) are

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.

@A6GibKm
Copy link
Contributor

A6GibKm commented Mar 21, 2024

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.

@firecat53
Copy link
Contributor

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.

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

No branches or pull requests

3 participants