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: ServeContent to return bytes copied end error #9709

Closed
funny-falcon opened this issue Jan 28, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@funny-falcon
Copy link
Contributor

commented Jan 28, 2015

It will be good if http.ServeContent will return number of bytes copied and error.
It should not break API compatibility, cause it doesn't return anything at the moment.

@adg

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2015

But it would break compatibility for anyone that's using http.ServeContent as a function value.

To do what you want, you could wrap the ResponseWriter with one that records errors etc. I know this isn't ideal.

@funny-falcon

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2015

No, I can't wrap cause I still could not access bytes copied (and it is primary reason for issue).

At the moment, I see that it looks like I ought to copy whole ServeContent implementation to achieve goal.

@funny-falcon

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2015

may be there is possibility to make checkETag and checkLastModified public?
or make serveContent to be a public function with different name, like DoServeContent(...)(int64, error)?

@mikioh mikioh changed the title net/http ServeContent to return bytes copied end error net/http: ServeContent to return bytes copied end error Jan 28, 2015

@adg

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2015

You can wrap both the io.ReadSeeker and the http.ResponseWriter with something that counts the bytes copied:

package main

import (
        "io"
        "log"
        "net/http"
        "strings"
        "time"
)

func main() {
        http.HandleFunc("/", handler)
        log.Fatal(http.ListenAndServe("localhost:8080", nil))
}

func handler(rw http.ResponseWriter, r *http.Request) {
        rs := strings.NewReader("content")
        crs := &countingReadSeeker{ReadSeeker: rs}
        crw := &countingResponseWriter{ResponseWriter: rw}
        http.ServeContent(crw, r, "file.txt", time.Now(), crs)
        log.Printf("read %v bytes, wrote %v bytes", crs.count, crw.count)
}

type countingReadSeeker struct {
        io.ReadSeeker
        count int64
}

func (r *countingReadSeeker) Read(b []byte) (n int, err error) {
        n, err = r.ReadSeeker.Read(b)
        r.count += int64(n)
        return
}

type countingResponseWriter struct {
        http.ResponseWriter
        count int64
}
@bradfitz

This comment has been minimized.

Copy link
Member

commented Jan 28, 2015

Yes, we can't change the signature of things due to the Go 1 API compatibility promise.

@bradfitz bradfitz closed this Jan 28, 2015

@funny-falcon

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2015

But you can expose new function: just make private function 'serveContent' to be a public one with non colliding name, and make it return bytes and error.
For example, it could be named as "ServeContentImpl", "ServeContentInternal" or other.

@adg

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2015

ServeContent hasn't changed in years. If you want to access its internals, just make a copy.

@funny-falcon

This comment has been minimized.

Copy link
Contributor Author

commented Jan 29, 2015

And now I do not offer to change ServeContent, i offer to make serveContent public with changed name and return type.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jan 29, 2015

Your minor convenience isn't worth the cost of all other Go programmer's increased cognitive load required by having more stuff in the net/http package to read and understand the difference between.

Sorry, we're not adding this. There are ways to do this already. Unless a large number/percentage of people needed this, it's not worth the cost of adding it.

@golang golang locked and limited conversation to collaborators Jun 25, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.