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

proposal: net/http: toHTTPError should be public #38375

Closed
purpleidea opened this issue Apr 11, 2020 · 7 comments
Closed

proposal: net/http: toHTTPError should be public #38375

purpleidea opened this issue Apr 11, 2020 · 7 comments
Labels
Projects
Milestone

Comments

@purpleidea
Copy link

@purpleidea purpleidea commented Apr 11, 2020

The toHTTPError method in net/http is useful! Any reason it isn't public? Can we make it public? I'd have written the patch if it wasn't for the CLA.

Thanks!

@purpleidea
Copy link
Author

@purpleidea purpleidea commented Apr 11, 2020

And as an aside, I would have expected the function to look like this instead:

func toHTTPError(err error) (msg string, httpStatus int) {
	if os.IsNotExist(err) {
		return http.StatusText(http.StatusNotFound), http.StatusNotFound
	}
	if os.IsPermission(err) {
		return http.StatusText(http.StatusForbidden), http.StatusForbidden
	}
	return http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError
}

Any reason it has an extra copy of the status text instead of re-using the useful helper that we already have?

@andybons andybons changed the title toHTTPError should be public proposal: net/http: toHTTPError should be public Apr 13, 2020
@gopherbot gopherbot added this to the Proposal milestone Apr 13, 2020
@gopherbot gopherbot added the Proposal label Apr 13, 2020
@rsc rsc added this to Incoming in Proposals Apr 15, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Apr 15, 2020

This doesn't seem important enough to expose in the standard library.
Maybe it should go into x/net/http/httpguts?
(If it did go into httpguts it should probably use errors.Is.)

@rsc rsc moved this from Incoming to Active in Proposals Apr 15, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Apr 22, 2020

Another problem with putting this in httpguts is that different servers are likely to want different customizations. The current situation, where people write the code they need, even if they start by copying those few lines, may be preferable to having a function in httpguts that will itself be the subject of more proposals for extensions.

There doesn't seem to be much enthusiasm for adding this or not. Perhaps we should leave things as is.

@purpleidea
Copy link
Author

@purpleidea purpleidea commented Apr 22, 2020

@rsc
Copy link
Contributor

@rsc rsc commented Apr 29, 2020

It's a useful helper for people like me who just want something along
these lines, and would rather use a vaguely common behaviour rather
than re-writing their own version constantly.

If you find yourself writing the same code over and over again, it's always a good idea to put it in your own library, published or not. On balance it doesn't seem like it should be published in the Go libraries, though. (Not all useful code has to be in the Go libraries.)

@rsc
Copy link
Contributor

@rsc rsc commented May 6, 2020

Based on the discussion above, especially how easy it is to put in a third-party package, this seems like a likely decline.

@rsc rsc moved this from Active to Likely Decline in Proposals May 6, 2020
@rsc
Copy link
Contributor

@rsc rsc commented May 20, 2020

No change in consensus, so declined.

@rsc rsc closed this May 20, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.