cc @ianlancetaylor as likely Proposal

This seems related to net/url.(*URL).Redacted. And, for that matter, net/http already tries to strip the password in a returned URL (https://go.googlesource.com/go/+/refs/heads/master/src/net/http/client.go#1003).

This seems like a reasonable idea to me if you want to write a proposal. Thanks.

@ianlancetaylor thank you for the quick reply; I can definitely work to reformat this issue into more of a proposal format. I've zero experience with that so I'll figure out how to do it right.

To your comment, yeah it's definitely similar. In this case it was a query parameter that wouldn't be caught by that method, and so I was thinking it might be better to have a way to turn this functionality off versus trying to complicate the stdlib with some sort of programmer-supplied filter specification.

I'll be sure to include that context in the reformat I do, to give others a concrete opportunity to challenge that assumption. Thanks again!

@ianlancetaylor alrighty, I've reformatted the issue to be more proposal-like. If this format doesn't fit the bill give me a shout.

CC @bradfitz @neild

One thing I considered when writing my proposal was to not have the bool on the net/url.Error be exported, and to instead move the error creation to a constructor function so that net/http could still specify it. That felt like a much larger change that I had a hard time justifying, but I could see it as being a way to avoid exposing what is effectively meant to be an internal API.

This seems reasonable to me.

You can do something like this to redact the URLs without requiring any changes to net/http (playground link - won't run on the playground because of #44822):

func RedactErrorURL(err error) {
	var ue *url.Error
	if errors.As(err, &ue) {
		ue.URL = "<redacted>"
	}
}

That works even when you don't have control over the Client struct (which is pretty common in my experience) and can be used wherever is appropriate. You could use it directly after a http request, or after a wrapper function is done, or all the way down at where the error is actually being logged.

@tmthrgd you are correct that is the solution you can pursue today. In the context section I mention the challenges of that approach:

The first is that you’d run the risk of forgetting to add that protection somewhere, or a code change / addition in the future could omit it. In addition if you’re mutating the error value to remove the URL, you’ve made it so that callers cannot inspect that detail of the error if they wanted to use it to decide to handle the error in a specific way.

@tmthrgd you are correct that is the solution you can pursue today. In the context section I mention the challenges of that approach:

The first is that you’d run the risk of forgetting to add that protection somewhere, or a code change / addition in the future could omit it. In addition if you’re mutating the error value to remove the URL, you’ve made it so that callers cannot inspect that detail of the error if they wanted to use it to decide to handle the error in a specific way.

But surely that’s no different at all to how easy it would be to forget to set the OnErrorOmitURL field? Also you have to be aware of any calls to http.Get or similar, any code using a custom http.Client that you might have missed or not easily control, and any code that performs http requests as part of a library.

What about net/http.Client.MutateErr func(error) error? So the user can specify the policy and we don't need a dozen policy bools in the future for all the possible variants people might want? (keep domain, keep URL path, only redact auth, only redact params, only redact certain params, ....)

Notably, http.Client is already a bag of policy funcs/interfaces, so it's somewhat fitting.

@bradfitz Considering the use case of wanting to retain the URL in the net/url.Error struct so that consumers can inspect it, but not have the URL be printed out when the .Error() method is invoked, I'm having a hard time visualizing how that would work with your suggestion. Or are you suggesting that we don't support that, and make the errors unusable by the caller?

I guess a second question comes to mind, would we want that function to take an error or a *url.Error? If we made it an error, we'd probably need to be real careful about how we change error handling inside of the net/http package going forward since folks would need to use type assertions to sanitize their URLs. Discoverability of all potential error types would likely need to be documented.

@bradfitz wanted to see if you'd had any spare cycles to noodle on the above.

If we're willing to permit net/http.Client.MutateErr to return a non-*url.Error error, then a redaction function could return an error that wraps the original *url.Error.

type redactedURLError url.Error

func (e *redactedURLError) Unwrap() error { return (*url.Error)(e) }
func (e *redactedURLError) Error() string { return fmt.Sprintf("%s: %s", e.Op, e.Err) }

func RedactURLError(err error) error {
  if e, ok := err.(*url.Error); ok {
    return (*redactedURLError)(e)
  }
  return e
}

client.MutateError = RedactURLError

This permits inspecting the original URL (by recovering the url.Error with errors.As), while redacting it from the error text.