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

Better errors #4

Open
steamraven opened this issue Nov 2, 2016 · 2 comments
Open

Better errors #4

steamraven opened this issue Nov 2, 2016 · 2 comments

Comments

@steamraven
Copy link
Contributor

Asserts may not be the best to catch length mismatches.
Maybe:
Convert length errors to ValueErrors
_raise_on_error raise a more specific error (like SodiumError?)
Possibly a special error for open and verify failures

@ereOn
Copy link
Owner

ereOn commented Nov 3, 2016

@steamraven To be honest, I did not give much of a thought on this when I did it and I'm still not sure what is better.

I may be influenced by the C++ world where I started but I've always used assertions to signal programmer errors. Now I understand that even the Python standard library raises ValueError or TypeError on programmer errors so it might just not be a good idea to do otherwise.

Do you think going with ValueError is more Pythonic ? If so, I have no problem changing that.

@steamraven
Copy link
Contributor Author

I think there are several types of errors.

Input validation errors: these are from incorrect length. I think these should be ValueError

Libsodium errors: since most libsodium functions are fairly simple an error result would mean something very broken. Probably not recoverable. I think systemerror or runtimeerror or a custom exception would be appropriate

Libsodium Resource errors: there are a few libsodium functions that rely on external resources: secure memory functions and random generators. I think we can fold them into the above section

Open/verify errors: these could easily happen in a correct program. However, it is a symptom of something wrong. I think either return None/False or raise a different custom exception

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

2 participants