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

signer API: review exceptions #468

Open
jku opened this issue Nov 29, 2022 · 3 comments
Open

signer API: review exceptions #468

jku opened this issue Nov 29, 2022 · 3 comments
Labels
discussion Issues that require discussion

Comments

@jku
Copy link
Collaborator

jku commented Nov 29, 2022

This is after #456:

Signer API exceptions may not be 100% coherent: it's an API that's grown over time. It's also not as critical that the exceptions are well designed as e.g. python-tuf verification API (as signing happens in much more controlled environments: new keys may be added but that never happens as a surprise to user). Still, would be good to review the exception usage.

One especially tricky case is Signer implementation specific exceptions -- like a KMS signer raising on network error. Currently we just document this as a possibility.

@jku jku mentioned this issue Dec 2, 2022
@jku
Copy link
Collaborator Author

jku commented Dec 29, 2022

There is, AFAICT, no really nice solution to this. We will not be able to handle all exceptions that e.g. google-cloud-kms might raise in a generic and useful way. So we can either tell callers to handle all errors or we can handle all the errors ourselves, and raise a generic SignerException...

The latter would normally be preferable (it's what we do in python-tuf serialization for example) but I can also imagine cases where caller wants to handle some specific errors: in that case wrapping the actual error just makes things more difficult for the caller.

So I'm leaning towards documenting Signers like

callers should prepare for sign() and import_() to raise Signer implementation specific errors that may be undocumented:

it's not amazing but I can live with that

@lukpueh
Copy link
Member

lukpueh commented Jan 9, 2023

it's not amazing but I can live with that

Agreed, also because of what you said above about sign and import happening in a more controlled environment.

This was referenced May 31, 2023
@lukpueh
Copy link
Member

lukpueh commented Jun 2, 2023

Let's also review if the exceptions we do know are properly documented and tested.

@lukpueh lukpueh added the discussion Issues that require discussion label Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issues that require discussion
Projects
None yet
Development

No branches or pull requests

2 participants