Skip to content

Use error wrapping when relevant #58

@AndersBennedsgaard

Description

@AndersBennedsgaard

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refinementMakes a minor improvement

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions