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

Use error wrapping when relevant #58

Open
AndersBennedsgaard opened this issue Aug 16, 2024 · 0 comments
Open

Use error wrapping when relevant #58

AndersBennedsgaard opened this issue Aug 16, 2024 · 0 comments
Labels
refinement Makes a minor improvement

Comments

@AndersBennedsgaard
Copy link

Hello. I am currently writing some tests for our application, and I have specifically hit the issue that pkcs12.DecodeChain can return an error that is not easy to check for, due wrapping other errors with errors.New():

return nil, nil, errors.New("pkcs12: error reading P12 data: " + err.Error())

If the returned error here (and other locations where a dynamic error message can be returned) uses fmt.Errorf to wrap the error, I would be able to use errors.Is to check it, using code such as:

cert, pass := generateCert() // helper function

_, _, _, err := pkcs12.DecodeChain(cert, pass)
if !errors.Is(err, errors.New("pkcs12: error reading P12 data:") {
    t.Errorf("expected error %v, got %v", testcase.expectedError, err)
}

Right now I have to do some annoying string prefix equality check, which is brittle and prone to error..

What I suggest is to convert errors.New("pkcs12: error reading P12 data: " + err.Error()) to fmt.Errorf("pkcs12: error reading P12 data: %w", err.Error()).
This conversion should be done anywhere else, like:

return nil, errors.New("pkcs12: error decoding PKCS#8 shrouded key bag: " + err.Error())

return nil, errors.New("pkcs12: error decrypting PKCS#8 shrouded key bag: " + err.Error())

return nil, errors.New("pkcs12: error decoding PKCS#8 shrouded key bag: " + err.Error())

return nil, errors.New("pkcs12: error parsing PKCS#8 private key: " + err.Error())

etc.

@AGWA AGWA added the refinement Makes a minor improvement label Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refinement Makes a minor improvement
Projects
None yet
Development

No branches or pull requests

2 participants