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

return a tls.CertificateVerificationError when verifying the certificate fails #3730

Closed
marten-seemann opened this issue Mar 23, 2023 · 3 comments · Fixed by #4015
Closed

Comments

@marten-seemann
Copy link
Member

This error was added in Go 1.20.

@marten-seemann
Copy link
Member Author

The question is, how does a reasonable error API look like.
Currently, the TransportError contains an ErrorMessage string:

type TransportError struct {
Remote bool
FrameType uint64
ErrorCode TransportErrorCode
ErrorMessage string
}

We could replace this with an Err error, which would allow seamless unwrapping of the error (and users to use standard Go error inspection functions like errors.As). For crypto errors, we'd just set whatever error we get from the TLS stack. For errors that we generate, we'd use fmt.Errorf.

However, this doesn't really make a lot of sense for errors that we receive from the peer. There, we only have the error code and the error message, and wrapping those in a Go error doesn't feel right. I'm wondering if there's a better error abstraction.

@MarcoPolo
Copy link
Collaborator

What about a way for an application to define the mapping/func from (errorCode, errorMessage) -> error. Then the application can create distinguished errors for certain error codes and handle them cleanly in their application logic. The fallback case can be a new Go error.

@marten-seemann
Copy link
Member Author

I'm not sure how that would work? The more idiomatic way would be to allow the application to do error assertions / unwrap the underlying error, which is what #4015 does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants