Merge pull request #163 from ipfs/fix/http-context

http: use the request context

Author: Stebalien

cli/run_test.go

@@ -28,10 +28,6 @@ type env struct {
 	ch chan struct{}
 }
 
-func (e env) Context() context.Context {
-	return context.Background()
-}
-
 func TestRunWaits(t *testing.T) {
 	flag := make(chan struct{}, 1)
 

examples/adder/remote/server/main.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"context"
 	nethttp "net/http"
 
 	"github.com/ipfs/go-ipfs-cmds/examples/adder"
@@ -11,10 +10,6 @@ import (
 
 type env struct{}
 
-func (env) Context() context.Context {
-	return context.TODO()
-}
-
 func main() {
 	h := http.NewHandler(env{}, adder.RootCmd, http.NewServerConfig())
 

executor.go

@@ -8,12 +8,8 @@ type Executor interface {
 	Execute(req *Request, re ResponseEmitter, env Environment) error
 }
 
-// Environment is the environment passed to commands. The only required method
-// is Context.
-type Environment interface {
-	// Context returns the environment's context.
-	Context() context.Context
-}
+// Environment is the environment passed to commands.
+type Environment interface{}
 
 // MakeEnvironment takes a context and the request to construct the environment
 // that is passed to the command's Run function.

executor_test.go

@@ -38,10 +38,6 @@ type wc struct {
 
 type env int
 
-func (e *env) Context() context.Context {
-	return context.Background()
-}
-
 func TestExecutor(t *testing.T) {
 	env := env(42)
 	req, err := NewRequest(context.Background(), []string{"test"}, nil, nil, nil, root)

http/handler.go

@@ -2,8 +2,6 @@ package http
 
 import (
 	"context"
-	"crypto/rand"
-	"encoding/base32"
 	"errors"
 	"net/http"
 	"runtime/debug"
@@ -97,12 +95,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	ctx := h.env.Context()
-	if ctx == nil {
-		log.Error("no root context found, using background")
-		ctx = context.Background()
-	}
-
 	if !allowOrigin(r, h.cfg) || !allowReferer(r, h.cfg) {
 		w.WriteHeader(http.StatusForbidden)
 		w.Write([]byte("403 - Forbidden"))
@@ -128,7 +120,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		r.Body = bw
 	}
 
-	req, err := parseRequest(ctx, r, h.root)
+	req, err := parseRequest(r, h.root)
 	if err != nil {
 		if err == ErrNotFound {
 			w.WriteHeader(http.StatusNotFound)
@@ -152,18 +144,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 	defer cancel()
 
-	req.Context = logging.ContextWithLoggable(req.Context, uuidLoggable())
-	if cn, ok := w.(http.CloseNotifier); ok {
-		clientGone := cn.CloseNotify()
-		go func() {
-			select {
-			case <-clientGone:
-			case <-req.Context.Done():
-			}
-			cancel()
-		}()
-	}
-
 	re, err := NewResponseEmitter(w, r.Method, req, withRequestBodyEOFChan(bodyEOFChan))
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
@@ -186,15 +166,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	h.root.Call(req, re, h.env)
 }
 
-func uuidLoggable() logging.Loggable {
-	ids := make([]byte, 16)
-	rand.Read(ids)
-
-	return logging.Metadata{
-		"requestId": base32.HexEncoding.EncodeToString(ids),
-	}
-}
-
 func sanitizedErrStr(err error) string {
 	s := err.Error()
 	s = strings.Split(s, "\n")[0]

http/handler_test.go

@@ -2,7 +2,6 @@ package http
 
 import (
 	"bytes"
-	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -26,15 +25,10 @@ type VersionOutput struct {
 
 type testEnv struct {
 	version, commit, repoVersion string
-	rootCtx                      context.Context
 	t                            *testing.T
 	wait                         chan struct{}
 }
 
-func (env testEnv) Context() context.Context {
-	return env.rootCtx
-}
-
 func getCommit(env cmds.Environment) (string, bool) {
 	tEnv, ok := env.(testEnv)
 	return tEnv.commit, ok
@@ -307,7 +301,6 @@ func getTestServer(t *testing.T, origins []string) (cmds.Environment, *httptest.
 		version:     "0.1.2",
 		commit:      "c0mm17", // yes, I know there's no 'm' in hex.
 		repoVersion: "4",
-		rootCtx:     context.Background(),
 		t:           t,
 		wait:        make(chan struct{}),
 	}

http/parse.go

@@ -1,9 +1,10 @@
 package http
 
 import (
-	"context"
+	"encoding/base32"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"mime"
 	"net/http"
 	"strconv"
@@ -12,10 +13,11 @@ import (
 	"github.com/ipfs/go-ipfs-cmds"
 
 	"github.com/ipfs/go-ipfs-files"
+	logging "github.com/ipfs/go-log"
 )
 
 // parseRequest parses the data in a http.Request and returns a command Request object
-func parseRequest(ctx context.Context, r *http.Request, root *cmds.Command) (*cmds.Request, error) {
+func parseRequest(r *http.Request, root *cmds.Command) (*cmds.Request, error) {
 	if r.URL.Path[0] == '/' {
 		r.URL.Path = r.URL.Path[1:]
 	}
@@ -134,6 +136,7 @@ func parseRequest(ctx context.Context, r *http.Request, root *cmds.Command) (*cm
 		return nil, fmt.Errorf("File argument '%s' is required", requiredFile)
 	}
 
+	ctx := logging.ContextWithLoggable(r.Context(), uuidLoggable())
 	req, err := cmds.NewRequest(ctx, pth, opts, args, f, root)
 	if err != nil {
 		return nil, err
@@ -229,3 +232,12 @@ func parseResponse(httpRes *http.Response, req *cmds.Request) (cmds.Response, er
 
 	return res, nil
 }
+
+func uuidLoggable() logging.Loggable {
+	ids := make([]byte, 16)
+	rand.Read(ids)
+
+	return logging.Metadata{
+		"requestId": base32.HexEncoding.EncodeToString(ids),
+	}
+}

http/parse_test.go

@@ -33,7 +33,7 @@ func TestParse(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	req, err := parseRequest(nil, r, root)
+	req, err := parseRequest(r, root)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -47,7 +47,7 @@ func TestParse(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	req, err = parseRequest(nil, r, root)
+	req, err = parseRequest(r, root)
 	if err != ErrNotFound {
 		t.Errorf("expected ErrNotFound, got: %v", err)
 	}
@@ -78,7 +78,7 @@ func (tc parseReqTestCase) test(t *testing.T) {
 	}
 	httpReq.URL.RawQuery = vs.Encode()
 
-	req, err := parseRequest(nil, httpReq, cmdRoot)
+	req, err := parseRequest(httpReq, cmdRoot)
 	if !errEq(err, tc.err) {
 		t.Fatalf("expected error to be %v, but got %v", tc.err, err)
 	}