proposal: crypto/tls, crypto/x509: use %w rather than %s in fmt.Errorf

[kanywst]

Proposal Details

This is a proposal to update several fmt.Errorf calls in crypto/tls and crypto/x509 to use the %w verb instead of %s.

Rationale:
Currently, some error messages in these packages are formatted using %s. By changing them to %w, callers will be able to inspect the underlying errors using errors.Is and errors.As.

Implementation:
I have already uploaded a Gerrit CL for this change: https://go-review.googlesource.com/c/go/+/750300

Since making errors inspectable is effectively adding to the public API, I am opening this proposal to discuss whether this behavior change is desired and what kind of test/documentation updates are needed.

cc @golang/security

[mateusz834]

Can you go over the details, exactly what errors interest you for use in errors.Is/errors.As?

[mateusz834]

I think that even if we use %w, then all of these are either errors.New/fmt.Errorf, or at least these in crypto/x509 in the mentioned CL.

[gopherbot]

Change https://go.dev/cl/750300 mentions this issue: crypto/tls, crypto/x509: use %w rather than %s in fmt.Errorf

[kanywst]

@mateusz834

Good point.

I initially thought of just modernizing the error formatting from %s to %w. But yeah, as you pointed out, these underlying errors (like the ones from parseASN1String or internal hpke functions) are just unexported errors.New or fmt.Errorf values anyway.

Even if we wrap them with %w, callers can't really do anything useful with errors.Is or errors.As since those underlying errors aren't exported.

My intention was just to align with the wrapping convention, but if this doesn't actually provide any practical value to users and only complicates the internal chains, I'm happy to just abandon the CL and close this proposal. Let me know what you think!

[mateusz834]

We have bunch of %v/%s in the Go std, we never change that if there is no good reason to do so.

Closing this proposal.

[gabyhelp]

Related Issues

• proposal: crypto/x509: add sentinel errors #67338
• x/xerrors: update with new %w semantics #33143 (closed)
• proposal: Go 2: universal WrapError type #36994 (closed)
• proposal: crypto/tls: Export TLS errors / alerts #56691 (closed)
• proposal: allow `errors.New()` to take a formatting string, or add `errors.Errorf()` #49348 (closed)
• proposal: fmt: add an additional %j verb to wrap errors #58107 (closed)
• proposal: errors: add errors.Mark to support error marks #50819 (closed)

Related Code Changes

• crypto/tls, crypto/x509: use %w rather than %s in fmt.Errorf

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}