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.

Sign up for GitHub

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

Jump to bottom

net/http: log ServeContent errors reading from the content during Copy? #31932

Open
zhxqgithub opened this issue May 9, 2019 · 3 comments
Open

net/http: log ServeContent errors reading from the content during Copy? #31932

zhxqgithub opened this issue May 9, 2019 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@zhxqgithub
Copy link

zhxqgithub commented May 9, 2019 •
edited

go/src/net/http/fs.go

Line 298 in f766b68

io.CopyN(w, sendContent, sendSize)

func serveContent was used by docker distribution project,I get 'unexpect eof error 'when I pull image for registry server may be related the bad network(just 20mb bandwidth).I can'not find any error in registry server log,it alwayls return response complete and 200 http status code.so i found error in io.CopyN after debug,it just copy only part of entire content due to ceph( the regitry storage backend ) rst the connection(mayby slow network).

I found same issue in 9709 ,however ,If it's not worth the cost of adding return error,can add error in ResponseWriter.like this

	if r.Method != "HEAD" {
		length, err := io.CopyN(w, sendContent, sendSize)
		if err != nil {
			errDetail := fmt.Sprintf("copy error,Just copy %d of %d", length, sendSize)
			Error(w, errDetail, StatusInternalServerError)
			return
		}
	}
@beoran
Copy link

beoran commented May 9, 2019

I'm not quite sure what you are asking, could you please clarify?

But if you need to do some more error checking, you could modify your sendContent to implement the ReaderFrom interface, and do the error checking like that.

@zhxqgithub
Copy link
Author

@beoran any example ??thanks

@bradfitz
Copy link
Contributor

bradfitz commented May 9, 2019

@zhxqgithub, we can't call http.Error as in your proposed change because we've already sent headers at this point (if we've copied at least 1 byte already). That is, CopyN doesn't slurp it all up into memory first. It streams it out. If there's a copy failure, it could be because either the read failed or the write failed. If the write failed, 99 times out of 100 (or more) it's because the client went away, and then a log statement isn't too interesting. A read error might be more interesting to log, I guess. I suppose we could write that to https://golang.org/pkg/net/http/#Server.ErrorLog

@bradfitz bradfitz changed the title io.CopyN(w, sendContent, sendSize) why swallow the error?? net/http: log ServeContent errors reading from the content during Copy? May 9, 2019
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 9, 2019
@bradfitz bradfitz added this to the Unplanned milestone May 9, 2019
@andybons andybons added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@bradfitz @beoran @andybons @zhxqgithub