monadicstack/frodo

Use of negroni in readme is incorrect

Closed this issue · 2 comments

Negroni's logging handler expects its own wrapped response writer. So to make the example in the readme work as expected, it looks more like this:

logMiddleware := negroni.NewLogger()
rpc.WithMiddleware(
    func(w http.ResponseWriter, req *http.Request, next http.HandlerFunc) {
        logMiddleware.ServeHTTP(negroni.NewResponseWriter(w), req, next)
    },
))

Otherwise, you'll see an error like "interface conversion: *http.response is not negroni.ResponseWriter: missing method Before" in your response bodies.

Ah crap. Didn't realize their library had such a tight dependency on its own response writer. Your snippet does work, but it feels rather cumbersome to have users do that just to pull a middleware off the shelf.

Rather than updating the readme, I can make the frodo middleware internals use a compatible response writer. This way you can still have the simplicity of the current example but... you know... it would work :)

I thought I was already using a custom response writer for another purpose where I needed to inspect the status code, but I can't seem to find that anywhere (I must have backed that out). I'll need to do some noodling to determine if the custom response writer belongs in Frodo or if it should live in http://github.com/monadicstack/respond which handles the gory details of HTTP response wrangling.

Sounds good!