search

DavidVentura / Keepass

Ubuntu touch keepass app
Other
2 stars 5 forks source link

try to parse error messages for a more userfriendly output #11

Open Danfro opened 3 years ago

Danfro commented 3 years ago

This PR is more to give an idea for discussion. It is likely not perfect or you might have some better ideas.

But if the password is typed wrong, the error message is displayed. This error message is not very userfriendly since it does not indicate to the user what is wrong.

I added a simple function to parse the error message and replace the strings to display depending on the error message.

What do you think of this? Surely you will know more messages. They could be listed there too and be replaced by corresponding strings.

DavidVentura commented 3 years ago

Interesting; what database version are you using? I use version 3 and when mistyping my password I get KDBX error: Incorrect key specified which is a lot better

I think the error should come in a nicer format from the keepass lib, but it still won't be localized so I think this approach makes sense

Danfro commented 3 years ago

I agree, proper error messaging would better be done in the keepass lib.

I am using KeePassXC snap v2.6.6 on Ubuntu. That uses libgcrypt 1.8.1 and current database version used is KDBX 3.1.

But while searching for this, I found that I could update it to use KDBX4.0. I might give that a try later.

Danfro commented 3 years ago

Just did a quick test. With KDBX4.0 I do get the same error message that you get. So obviously messages need to be specified depending on database version.

Danfro commented 3 years ago

I will remove the WIP label, since we agree on this approach. Feel free to add more error messages if you encounter them.

DavidVentura commented 3 years ago

hey! i didn't forget about this PR, but i think the message parsing should change from KDBX error: Database integrity error: Cryptography error: BlockMode { e: BlockModeError } to the error types in here

however, i still don't know what's the best way to map these, as the phrasing could change on different keepass-rs versions.

maybe i can map error-type to error-code (ie ERR1234) and have the errors keyed by error code on the i18n?

Danfro commented 3 years ago

Surely parsing the error message would not need to check for the full string. Parsing could be reduced to the relevant (different) part.

Unless I have missed one, there are three main types of error:

Since I would not expect all errors to occur, it might be too much to parse for all detailed error types.

Maybe it is sufficient to give a translated message for every one of the three types like: "an error occured while encrypting the database, please check your password" "an error occured while reading the database, please make sure it is a valid keepass database of format KDBX3 or KDBX4" "an display error occured, please nuke your phone since I have no idea what happened"

The message should give an idication of the most likely source for the error e.g. wrong password.

Then either print the full error message underneath or only to the logfile. Maybe leave a note to open an issue on github if the problem persists including the error message/logfile.