server/prpc: updated server according to protocol

server/prpc/response.go

Index: server/prpc/response.go
diff --git a/server/prpc/response.go b/server/prpc/response.go
new file mode 100644
index 0000000000000000000000000000000000000000..0d76aa9a68394914271da3e39476b5abd9e80de1
--- /dev/null
+++ b/server/prpc/response.go
@@ -0,0 +1,75 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package prpc
+
+import (
+	"fmt"
+	"net/http"
+	"strconv"
+
+	"golang.org/x/net/context"
+	"google.golang.org/grpc/codes"
+
+	"github.com/luci/luci-go/common/logging"
+	"github.com/luci/luci-go/common/prpc"
+)
+
+var newLineBytes = []byte{'\n'}
+
+// response is a pRPC server response.
+// All pRPC responses must be written using write.
+type response struct {
+	code    codes.Code // defaults to OK
+	status  int        // defaults to status derived from code.
+	header  http.Header
+	body    []byte
+	newLine bool // whether to write \n after body
+}
+
+// errResponse creates a response with an error.
+func errResponse(code codes.Code, status int, format string, a ...interface{}) *response {
+	return &response{
+		code:   code,
+		status: status,
+		header: http.Header{
+			headerContentType: []string{"text/plain"},
+		},
+		body:    []byte(fmt.Sprintf(format, a...)),
+		newLine: true,
+	}
+}
+
+// write writes r to w.
+func (r *response) write(c context.Context, w http.ResponseWriter) {
+	body := r.body
+	switch r.code {
+	case codes.Internal, codes.Unknown:
+		// res.body is error message.
+		logging.Fields{
+			"code": r.code,
+		}.Errorf(c, "%s", body)

[dnj, 2016/01/26 18:50:27]

nit: "body" is a []byte, but you're logging it as a string. This will show up as
[1, 2, 3, 4] etc. I'd either string-convert (string(body)) or log as %v if you
want the byte output.

Also include some context in the Errorf: "Masking %s error message: %s", r.code,
string(body))

[nodir, 2016/01/26 19:06:09]

%s converts bytes to string https://play.golang.org/p/m39fvwX43z
 
You are suggesting all our internal rpc errors to have this prefix. The main
point here is not masking (although we do it), but the error itself.

[dnj, 2016/01/26 19:26:38]

Oh cool, didn't know that!
Wait, this is a logged error (shows up in our logs), not a returned error. I
just think "Something went wrong" showing up in the log is less useful than,
"Masked Internal/Unknown error: Something went wrong". Especially b/c searching
for "where did this error occur in the code?" is excruciatingly difficult when
there is no text string context.

But I'll leave this up to you as a nit.

+		body = []byte("Internal Server Error")
+	}
+
+	for h, vs := range r.header {
+		w.Header()[h] = vs
+	}
+	w.Header().Set(prpc.HeaderGRPCCode, strconv.Itoa(int(r.code)))
+
+	status := r.status
+	if status == 0 {
+		status = codeStatus(r.code)
+	}
+	w.WriteHeader(status)
+
+	if _, err := w.Write(body); err != nil {
+		logging.WithError(err).Errorf(c, "Could not respond")
+		// The header is already written. There is nothing more we can do.
+		return
+	}
+	if r.newLine {
+		w.Write(newLineBytes)
+	}
+}