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
Closed

net/http: ServeContent to return bytes copied end error #9709

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

Comments

@funny-falcon
Copy link
Contributor

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
Copy link
Contributor

adg 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

adg 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
Copy link
Contributor

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

@funny-falcon
Copy link
Contributor Author

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
Copy link
Contributor

adg commented Jan 29, 2015

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

@funny-falcon
Copy link
Contributor Author

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

@bradfitz
Copy link
Contributor

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.
Projects
None yet
Development

No branches or pull requests

4 participants