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

Context from Connection #3

Closed
tooolbox opened this issue Jul 7, 2020 · 1 comment · Fixed by #4
Closed

Context from Connection #3

tooolbox opened this issue Jul 7, 2020 · 1 comment · Fixed by #4

Comments

@tooolbox
Copy link
Contributor

tooolbox commented Jul 7, 2020

It's normal if you're writing an HTTP server to take the context.Context from the http.Request and use that when you make various calls to formulate the response. In that way if the client closes the connection, your RPC call that is 3 microservices deep doing resource-intensive work can cancel because nobody is listening to the answer anymore.

Although I appreciate that someone has taken the time to add context.Context support to this package, it doesn't seem like I can hook it up to the context from an HTTP request. For example, I'd like to do this:

import (
    "net/http"
    "bytes"

    "github.com/keegancsmith/rpc"
)

rpcServer := rpc.NewServer()
rpcServer.Register(myApi)
srv := &http.Server{
    Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        buf := bytes.NewBuffer()
        codec := makeCodec(r.Body, buf)
        err := rpcServer.ServeRequest(r.Context(), codec) // not currently in existence
        if err != nil {
            http.Error(...)
            return
        }
        buf.WriteTo(w)
        // ...
    }),
}
srv.ListenAndServe(":3000")

Anyway, that's pseudo-code, but you get the point. I need to be able to do rpcServer.ServeCodec(ctx, codec) essentially. I can figure out hacking that in, but I'm wondering if any of the maintainers have thoughts on this.

@tooolbox
Copy link
Contributor Author

tooolbox commented Jul 7, 2020

Pretty sure I see how it can be done. PR coming shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant