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

net/http: describe where multiple response.WriteHeader calls occur #18761

Closed
dsnet opened this issue Jan 23, 2017 · 8 comments
Closed

net/http: describe where multiple response.WriteHeader calls occur #18761

dsnet opened this issue Jan 23, 2017 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Jan 23, 2017

I see this in an HTTP server log:

server.go:2315: http: multiple response.WriteHeader calls

Written by the http package here.

It would be nice if it described file+line of the first non-http package function that caused WriteHeader to be called so that it's easier to find the offending code.

\cc @bradfitz

@dsnet dsnet added this to the Unplanned milestone Jan 23, 2017
@siadat
Copy link
Contributor

siadat commented Jan 27, 2017

I can work on this if it is okay.

Regarding the implementation, would it be okay to use runtime.Callers here, and find the first function whose name does not start with "net/http."?

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Unplanned Feb 1, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Feb 1, 2017

@siadat, sounds like it could work.

As long as you only do that work when we're already spamming logs, and not for all WriteHeader calls.

@gopherbot
Copy link

CL https://golang.org/cl/36096 mentions this issue.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jun 30, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 30, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@odeke-em
Copy link
Member

Hello there @siadat, Go1.11 is active right now and @danp pinged you on the CL, please let us know if you'll be carrying on with the CL, if you are busy, no worries, we can work on it.

@siadat
Copy link
Contributor

siadat commented Mar 11, 2018

Hi @odeke-em, thanks for the reminder, I will have a look at it!

@gopherbot gopherbot modified the milestones: Go1.11, Unplanned May 23, 2018
@gopherbot
Copy link

Change https://golang.org/cl/130997 mentions this issue: net/http: print caller which causes multiple header writes

@nightlyone
Copy link
Contributor

I debugged quite a few of these in PHP in a previous life nearly a decade ago.

From that experience I would like to mention that you usually don't know whether the first or the second one is the caller that should have been written the header. So I would suggest reporting both: The first writer and the second one.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 3, 2018

@nightlyone, that means making all users pay the cost of recording that first write's stack frame, even though the majority will only ever have one write.

@golang golang locked and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants