fix duplicate header sends

99designs · Nov 11, 2019 · 0ee185b · 0ee185b
1 parent 7cbd75d
commit 0ee185b
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 105 deletions.
diff --git a/graphql/handler.go b/graphql/handler.go
@@ -2,7 +2,6 @@ package graphql
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"strconv"
 	"strings"
@@ -11,8 +10,6 @@ import (
 )
 
 type (
-	Writer func(Status, *Response)
-
 	OperationMiddleware func(ctx context.Context, next OperationHandler) ResponseHandler
 	OperationHandler    func(ctx context.Context) ResponseHandler
 
@@ -89,31 +86,6 @@ type (
 
 type Status int
 
-const (
-	StatusOk Status = iota
-	StatusParseError
-	StatusValidationError
-	StatusResolverError
-)
-
-func (w Writer) Errorf(format string, args ...interface{}) {
-	w(StatusResolverError, &Response{
-		Errors: gqlerror.List{{Message: fmt.Sprintf(format, args...)}},
-	})
-}
-
-func (w Writer) Error(msg string) {
-	w(StatusResolverError, &Response{
-		Errors: gqlerror.List{{Message: msg}},
-	})
-}
-
-func (w Writer) GraphqlErr(err ...*gqlerror.Error) {
-	w(StatusResolverError, &Response{
-		Errors: err,
-	})
-}
-
 func (p *RawParams) AddUpload(upload Upload, key, path string) *gqlerror.Error {
 	if !strings.HasPrefix(path, "variables.") {
 		return gqlerror.Errorf("invalid operations paths for key %s", key)

diff --git a/graphql/handler/transport/http_form.go b/graphql/handler/transport/http_form.go
@@ -56,59 +56,49 @@ func (f MultipartForm) maxMemory() int64 {
 func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecutor) {
 	w.Header().Set("Content-Type", "application/json")
 
-	write := graphql.Writer(func(status graphql.Status, response *graphql.Response) {
-		switch status {
-		case graphql.StatusOk, graphql.StatusResolverError:
-			w.WriteHeader(http.StatusOK)
-		case graphql.StatusParseError, graphql.StatusValidationError:
-			w.WriteHeader(http.StatusUnprocessableEntity)
-		}
-
-		b, err := json.Marshal(response)
-		if err != nil {
-			panic(err)
-		}
-		w.Write(b)
-	})
-
 	var err error
 	if r.ContentLength > f.maxUploadSize() {
-		write.Errorf("failed to parse multipart form, request body too large")
+		writeJsonError(w, "failed to parse multipart form, request body too large")
 		return
 	}
 	r.Body = http.MaxBytesReader(w, r.Body, f.maxUploadSize())
 	if err = r.ParseMultipartForm(f.maxUploadSize()); err != nil {
+		w.WriteHeader(http.StatusUnprocessableEntity)
 		if strings.Contains(err.Error(), "request body too large") {
-			write.Errorf("failed to parse multipart form, request body too large")
+			writeJsonError(w, "failed to parse multipart form, request body too large")
 			return
 		}
-		write.Errorf("failed to parse multipart form")
+		writeJsonError(w, "failed to parse multipart form")
 		return
 	}
 	defer r.Body.Close()
 
 	var params graphql.RawParams
 
 	if err = jsonDecode(strings.NewReader(r.Form.Get("operations")), &params); err != nil {
-		write.Errorf("operations form field could not be decoded")
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		writeJsonError(w, "operations form field could not be decoded")
 		return
 	}
 
 	var uploadsMap = map[string][]string{}
 	if err = json.Unmarshal([]byte(r.Form.Get("map")), &uploadsMap); err != nil {
-		write.Errorf("map form field could not be decoded")
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		writeJsonError(w, "map form field could not be decoded")
 		return
 	}
 
 	var upload graphql.Upload
 	for key, paths := range uploadsMap {
 		if len(paths) == 0 {
-			write.Errorf("invalid empty operations paths list for key %s", key)
+			w.WriteHeader(http.StatusUnprocessableEntity)
+			writeJsonErrorf(w, "invalid empty operations paths list for key %s", key)
 			return
 		}
 		file, header, err := r.FormFile(key)
 		if err != nil {
-			write.Errorf("failed to get key %s from form", key)
+			w.WriteHeader(http.StatusUnprocessableEntity)
+			writeJsonErrorf(w, "failed to get key %s from form", key)
 			return
 		}
 		defer file.Close()
@@ -121,14 +111,16 @@ func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.G
 			}
 
 			if err := params.AddUpload(upload, key, paths[0]); err != nil {
-				write.GraphqlErr(err)
+				w.WriteHeader(http.StatusUnprocessableEntity)
+				writeJsonGraphqlError(w, err)
 				return
 			}
 		} else {
 			if r.ContentLength < f.maxMemory() {
 				fileBytes, err := ioutil.ReadAll(file)
 				if err != nil {
-					write.Errorf("failed to read file for key %s", key)
+					w.WriteHeader(http.StatusUnprocessableEntity)
+					writeJsonErrorf(w, "failed to read file for key %s", key)
 					return
 				}
 				for _, path := range paths {
@@ -139,14 +131,16 @@ func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.G
 					}
 
 					if err := params.AddUpload(upload, key, path); err != nil {
-						write.GraphqlErr(err)
+						w.WriteHeader(http.StatusUnprocessableEntity)
+						writeJsonGraphqlError(w, err)
 						return
 					}
 				}
 			} else {
 				tmpFile, err := ioutil.TempFile(os.TempDir(), "gqlgen-")
 				if err != nil {
-					write.Errorf("failed to create temp file for key %s", key)
+					w.WriteHeader(http.StatusUnprocessableEntity)
+					writeJsonErrorf(w, "failed to create temp file for key %s", key)
 					return
 				}
 				tmpName := tmpFile.Name()
@@ -155,21 +149,24 @@ func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.G
 				}()
 				_, err = io.Copy(tmpFile, file)
 				if err != nil {
+					w.WriteHeader(http.StatusUnprocessableEntity)
 					if err := tmpFile.Close(); err != nil {
-						write.Errorf("failed to copy to temp file and close temp file for key %s", key)
+						writeJsonErrorf(w, "failed to copy to temp file and close temp file for key %s", key)
 						return
 					}
-					write.Errorf("failed to copy to temp file for key %s", key)
+					writeJsonErrorf(w, "failed to copy to temp file for key %s", key)
 					return
 				}
 				if err := tmpFile.Close(); err != nil {
-					write.Errorf("failed to close temp file for key %s", key)
+					w.WriteHeader(http.StatusUnprocessableEntity)
+					writeJsonErrorf(w, "failed to close temp file for key %s", key)
 					return
 				}
 				for _, path := range paths {
 					pathTmpFile, err := os.Open(tmpName)
 					if err != nil {
-						write.Errorf("failed to open temp file for key %s", key)
+						w.WriteHeader(http.StatusUnprocessableEntity)
+						writeJsonErrorf(w, "failed to open temp file for key %s", key)
 						return
 					}
 					defer pathTmpFile.Close()
@@ -180,7 +177,8 @@ func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.G
 					}
 
 					if err := params.AddUpload(upload, key, path); err != nil {
-						write.GraphqlErr(err)
+						w.WriteHeader(http.StatusUnprocessableEntity)
+						writeJsonGraphqlError(w, err)
 						return
 					}
 				}
@@ -190,11 +188,11 @@ func (f MultipartForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.G
 
 	rc, gerr := exec.CreateOperationContext(r.Context(), &params)
 	if gerr != nil {
-		w.WriteHeader(http.StatusUnprocessableEntity)
 		resp := exec.DispatchError(graphql.WithOperationContext(r.Context(), rc), gerr)
-		write(graphql.StatusValidationError, resp)
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		writeJson(w, resp)
 		return
 	}
 	responses, ctx := exec.DispatchOperation(r.Context(), rc)
-	write(graphql.StatusResolverError, responses(ctx))
+	writeJson(w, responses(ctx))
 }
diff --git a/graphql/handler/transport/http_form_test.go b/graphql/handler/transport/http_form_test.go
@@ -204,7 +204,7 @@ func TestFileUpload(t *testing.T) {
 		}
 		resp := httptest.NewRecorder()
 		h.ServeHTTP(resp, req)
-		require.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
+		require.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
 		require.Equal(t, `{"errors":[{"message":"failed to parse multipart form"}],"data":null}`, resp.Body.String())
 	})
 
@@ -214,7 +214,7 @@ func TestFileUpload(t *testing.T) {
 
 		resp := httptest.NewRecorder()
 		h.ServeHTTP(resp, req)
-		require.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
+		require.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
 		require.Equal(t, `{"errors":[{"message":"operations form field could not be decoded"}],"data":null}`, resp.Body.String())
 	})
 
@@ -224,7 +224,7 @@ func TestFileUpload(t *testing.T) {
 
 		resp := httptest.NewRecorder()
 		h.ServeHTTP(resp, req)
-		require.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
+		require.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
 		require.Equal(t, `{"errors":[{"message":"map form field could not be decoded"}],"data":null}`, resp.Body.String())
 	})
 
@@ -234,7 +234,7 @@ func TestFileUpload(t *testing.T) {
 
 		resp := httptest.NewRecorder()
 		h.ServeHTTP(resp, req)
-		require.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
+		require.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
 		require.Equal(t, `{"errors":[{"message":"failed to get key 0 from form"}],"data":null}`, resp.Body.String())
 	})
 
@@ -244,7 +244,7 @@ func TestFileUpload(t *testing.T) {
 
 		resp := httptest.NewRecorder()
 		h.ServeHTTP(resp, req)
-		require.Equal(t, http.StatusOK, resp.Code, resp.Body.String())
+		require.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
 		require.Equal(t, `{"errors":[{"message":"invalid operations paths for key 0"}],"data":null}`, resp.Body.String())
 	})
 

diff --git a/graphql/handler/transport/http_get.go b/graphql/handler/transport/http_get.go
@@ -30,32 +30,18 @@ func (h GET) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecut
 		OperationName: r.URL.Query().Get("operationName"),
 	}
 
-	write := graphql.Writer(func(status graphql.Status, response *graphql.Response) {
-		switch status {
-		case graphql.StatusOk, graphql.StatusResolverError:
-			w.WriteHeader(http.StatusOK)
-		case graphql.StatusParseError, graphql.StatusValidationError:
-			w.WriteHeader(http.StatusUnprocessableEntity)
-		}
-		b, err := json.Marshal(response)
-		if err != nil {
-			panic(err)
-		}
-		w.Write(b)
-	})
-
 	if variables := r.URL.Query().Get("variables"); variables != "" {
 		if err := jsonDecode(strings.NewReader(variables), &raw.Variables); err != nil {
 			w.WriteHeader(http.StatusBadRequest)
-			write.Errorf("variables could not be decoded")
+			writeJsonError(w, "variables could not be decoded")
 			return
 		}
 	}
 
 	if extensions := r.URL.Query().Get("extensions"); extensions != "" {
 		if err := jsonDecode(strings.NewReader(extensions), &raw.Extensions); err != nil {
 			w.WriteHeader(http.StatusBadRequest)
-			write.Errorf("extensions could not be decoded")
+			writeJsonError(w, "extensions could not be decoded")
 			return
 		}
 	}
@@ -64,18 +50,18 @@ func (h GET) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecut
 	if err != nil {
 		w.WriteHeader(http.StatusUnprocessableEntity)
 		resp := exec.DispatchError(graphql.WithOperationContext(r.Context(), rc), err)
-		write(graphql.StatusValidationError, resp)
+		writeJson(w, resp)
 		return
 	}
 	op := rc.Doc.Operations.ForName(rc.OperationName)
 	if op.Operation != ast.Query {
 		w.WriteHeader(http.StatusNotAcceptable)
-		write.Errorf("GET requests only allow query operations")
+		writeJsonError(w, "GET requests only allow query operations")
 		return
 	}
 
 	responses, ctx := exec.DispatchOperation(r.Context(), rc)
-	write(graphql.StatusResolverError, responses(ctx))
+	writeJson(w, responses(ctx))
 }
 
 func jsonDecode(r io.Reader, val interface{}) error {

diff --git a/graphql/handler/transport/http_post.go b/graphql/handler/transport/http_post.go
@@ -1,7 +1,6 @@
 package transport
 
 import (
-	"encoding/json"
 	"mime"
 	"net/http"
 
@@ -30,35 +29,20 @@ func (h POST) Supports(r *http.Request) bool {
 func (h POST) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecutor) {
 	w.Header().Set("Content-Type", "application/json")
 
-	write := graphql.Writer(func(status graphql.Status, response *graphql.Response) {
-		switch status {
-		case graphql.StatusOk, graphql.StatusResolverError:
-			w.WriteHeader(http.StatusOK)
-		case graphql.StatusParseError, graphql.StatusValidationError:
-			w.WriteHeader(http.StatusUnprocessableEntity)
-		}
-
-		b, err := json.Marshal(response)
-		if err != nil {
-			panic(err)
-		}
-		w.Write(b)
-	})
-
 	var params *graphql.RawParams
 	if err := jsonDecode(r.Body, &params); err != nil {
 		w.WriteHeader(http.StatusBadRequest)
-		write.Errorf("json body could not be decoded: " + err.Error())
+		writeJsonErrorf(w, "json body could not be decoded: "+err.Error())
 		return
 	}
 
 	rc, err := exec.CreateOperationContext(r.Context(), params)
 	if err != nil {
 		w.WriteHeader(http.StatusUnprocessableEntity)
 		resp := exec.DispatchError(graphql.WithOperationContext(r.Context(), rc), err)
-		write(graphql.StatusValidationError, resp)
+		writeJson(w, resp)
 		return
 	}
 	responses, ctx := exec.DispatchOperation(r.Context(), rc)
-	write(graphql.StatusResolverError, responses(ctx))
+	writeJson(w, responses(ctx))
 }
diff --git a/graphql/handler/transport/util.go b/graphql/handler/transport/util.go
@@ -0,0 +1,30 @@
+package transport
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+
+	"github.com/99designs/gqlgen/graphql"
+	"github.com/vektah/gqlparser/gqlerror"
+)
+
+func writeJson(w io.Writer, response *graphql.Response) {
+	b, err := json.Marshal(response)
+	if err != nil {
+		panic(err)
+	}
+	w.Write(b)
+}
+
+func writeJsonError(w io.Writer, msg string) {
+	writeJson(w, &graphql.Response{Errors: gqlerror.List{{Message: msg}}})
+}
+
+func writeJsonErrorf(w io.Writer, format string, args ...interface{}) {
+	writeJson(w, &graphql.Response{Errors: gqlerror.List{{Message: fmt.Sprintf(format, args...)}}})
+}
+
+func writeJsonGraphqlError(w io.Writer, err ...*gqlerror.Error) {
+	writeJson(w, &graphql.Response{Errors: err})
+}