net/http: imprecise description of the return values of HTTP serve functions

[yaojingguo]

What version of Go are you using (go version)?

go version go1.10.3 darwin/amd64

In net.http package, there are following fucntions/methods related to serve HTTP:

1. http.Serve
2. http.ListenAndServe
3. http.ListenAndServeTLS
4. http.Server.Serve
5. http.Server.ListenAndServe
6. http.Server.ListenAndServeTLS

Except the first one, all docs for those functiosn/methods say something like always returns a non-nil error. But if such a function/method is terminated by a signal which is a common way to terminate a HTTP server, it will not return. For example, consider the following go code http.go:

package main

import (
	"log"
	"net/http"
)

func main() {
	var srv http.Server
	srv.Addr = ":12345"

	if err := srv.ListenAndServe(); err != http.ErrServerClosed {
		log.Printf("http error: %s", err)
	}
}

A SIGINT signal will terminate the program and http.ListenAndServe will not return.

➜  local go run http.go
^Csignal: interrupt

Only if srv is teriminated by Server.Shutdown, srv.ListenAndServe will return.

I think that the descripion always returns a non-nil error should mention this behaviour. Otherwise, people will tend to write the following code:

	err = (&http.Server{Handler: rc.transport.Handler()}).Serve(ln)
	select {
	case <-rc.httpstopc:
	default:
		log.Fatalf("raftexample: Failed to serve rafthttp (%v)", err)
	}

They are hoping that Serve will return. But it will not since Shutdown is not used in a way descripted in Shutdown's example code.

[meirf]

I think "always" is a description of the "non-nil" quality of the error as opposed to a description of the "return" of the error. In other words, any error that is returned is guaranteed to be non-nil. It is probably not meant to guarantee that the function returns - I don't know if we can guarantee that any function in the standard library returns.

But perhaps there's a way to remove this ambiguity in a similarly concise fashion.

[mvdan]

Perhaps the sentences could be reworded, like:

The returned error will always be non-nil.

[agnivade]

Yep, it just says that the returned error will always be non-nil. It does not say anything about the scenarios in which the function will return. Seems clear enough to me, but maybe we can be more explicit.

EDIT: @mvdan - our responses crossed :), I like your suggestion.

/cc @bradfitz

[gopherbot]

Change https://golang.org/cl/122465 mentions this issue: net/http: tweak "always returns" godoc wording

[mvdan]

I've sent a small CL above with the proposed change.

More broadly, I've filed #26268 to look into all the other cases of "always returns X" in the standard library.

[mvdan]

Closing, just like #26268. See Russ's point there. We didn't reach consensus, and it doesn't look like we found a wording that was considerably better.