ServeHTTP panic does not close stream

[rnapier]

If a response ServeHTTP panics during RequestResponse, the stream is not closed, causing the requester to hang. The equivalent http method does close the stream:

If ServeHTTP panics, the server (the caller of ServeHTTP) assumes that the effect of the panic was isolated to the active request. It recovers the panic, logs a stack trace to the server error log, and hangs up the connection.

I've tried adding a s.shutdown() to the recovery function in ResponseStream.Run(), but that doesn't seem to help. s.conn.Close() is too much (we don't want to kill the whole connection).

proxy_server.go

package main

import (
    "bytes"
    "fmt"
    "io"
    "net/http"

    "github.com/SlyMarbo/spdy"
)

func handle(err error) {
    if err != nil {
        panic(err)
    }
}

func fetch(conn spdy.Conn, path string) {
    fmt.Printf("Fetching %v\n", path)
    url := "http://" + conn.Conn().RemoteAddr().String() + path

    req, err := http.NewRequest("GET", url, nil)
    if err != nil {
        println("NEWREQ: " + err.Error())
        return
    }

    fmt.Printf("REQRES: %v\n", path)
    res, err := conn.RequestResponse(req, nil, 1)
    if err != nil {
        println("REQRES: " + err.Error())
        return
    }

    fmt.Printf("COPY: %v\n", path)
    buf := new(bytes.Buffer)
    _, err = io.Copy(buf, res.Body)
    if err != nil {
        println("COPY: " + err.Error())
        return
    }

    res.Body.Close()

    fmt.Printf("RECV: %v (%d) %v\n", path, res.StatusCode, buf.String())
}

func handleProxy(conn spdy.Conn) {
    fetch(conn, "/crash")
    fetch(conn, "/") // Never runs

    wait := make(chan interface{})
    <-wait
}

func main() {
    // spdy.EnableDebugOutput()
    handler := spdy.ProxyConnHandlerFunc(handleProxy)
    http.Handle("/", spdy.ProxyConnections(handler))
    handle(http.ListenAndServeTLS(":8080", "cert.pem", "key.pem", nil))
}

proxy_client.go

package main

import (
    "crypto/tls"
    "fmt"
    "net/http"

    "github.com/SlyMarbo/spdy"
)

func handle(err error) {
    if err != nil {
        panic(err)
    }
}

func serveHTTP(w http.ResponseWriter, r *http.Request) {
    fmt.Printf("Serving %v\n", r.URL.Path)
    if r.URL.Path == "/crash" {
        panic("CRASH")
    }
    w.Write([]byte("Testing, testing, 1, 2, 3."))
}

func main() {
    // spdy.EnableDebugOutput()
    http.HandleFunc("/", serveHTTP)
    handle(spdy.ConnectAndServe("http://localhost:8080/", &tls.Config{InsecureSkipVerify: true}, nil))

    wait := make(chan interface{})
    <-wait
}

[rnapier]

More than just a close is required. We actually need to send something back (like a SYN_REPLY). This recover seems to work (but it's wrong, and way too duplicative; I just cut and pasted the rest of Run). I haven't really decided what the right result here is. Probably SYN_REPLY with a 500 result (rather than RST_STREAM with an internal error reason). The protocol implementation didn't have an error; the server did, so that's probably best expressed as a 500.

    defer func() {
        if v := recover(); v != nil {
            if s != nil && s.state != nil && !s.state.Closed() {
                const size = 64 << 10
                buf := make([]byte, size)
                buf = buf[:runtime.Stack(buf, false)]
                log.Printf("Encountered stream error:: %v\n%s", v, buf)

                // Make sure any queued data has been sent.
                if err := s.flow.Wait(); err != nil {
                    log.Println(err)
                }

                // Close the stream with a SYN_REPLY if
                // none has been sent, or an empty DATA
                // frame, if a SYN_REPLY has been sent
                // already.
                // If the stream is already closed at
                // this end, then nothing happens.
                if !s.unidirectional {
                    if s.state.OpenHere() && !s.wroteHeader {
                        s.header.Set(":status", "200")
                        s.header.Set(":version", "HTTP/1.1")

                        // Create the response SYN_REPLY.
                        synReply := new(frames.SYN_REPLY)
                        synReply.Flags = common.FLAG_FIN
                        synReply.StreamID = s.streamID
                        synReply.Header = make(http.Header)

                        for name, values := range s.header {
                            for _, value := range values {
                                synReply.Header.Add(name, value)
                            }
                            s.header.Del(name)
                        }

                        s.output <- synReply
                    } else if s.state.OpenHere() {
                        // Create the DATA.
                        data := new(frames.DATA)
                        data.StreamID = s.streamID
                        data.Flags = common.FLAG_FIN
                        data.Data = []byte{}

                        s.output <- data
                    }
                }

                // Clean up state.
                s.state.CloseHere()

                if s.state.Closed() {
                    s.Close()
                }
            }
        }
    }()