server/prpc: updated server according to protocol

server/prpc/error.go

Index: server/prpc/error.go
diff --git a/server/prpc/error.go b/server/prpc/error.go
index 950ef99f65c3787f36c451cf1653057d8e3c54a7..6df3c72c05f91983cfcd1ba769e0afac82ff9d83 100644
--- a/server/prpc/error.go
+++ b/server/prpc/error.go
@@ -4,45 +4,43 @@
 
 package prpc
 
-import "fmt"
+import (
+	"fmt"
+)
 
-// httpError is an error with HTTP status.
-// Many internal functions return *httpError instead of error
-// to guarantee expected HTTP status via type-system.
-// Supported by ErrorStatus and ErrorDesc.
-type httpError struct {
+// protocolError is returned if a pRPC request is malformed.
+type protocolError struct {
 	err    error
-	status int
+	status int // HTTP status to use in response.
 }
 
-func (e *httpError) Error() string {
-	return fmt.Sprintf("HTTP %d: %s", e.status, e.err)
+func (e *protocolError) Error() string {
+	return fmt.Sprintf("pRPC: %s", e.err)
 }
 
 // withStatus wraps an error with an HTTP status.
 // If err is nil, returns nil.
-func withStatus(err error, status int) *httpError {
-	if _, ok := err.(*httpError); ok {
+func withStatus(err error, status int) *protocolError {
+	if _, ok := err.(*protocolError); ok {
 		panicf("httpError in httpError")
 	}
 	if err == nil {
 		return nil
 	}
-	return &httpError{err, status}
+	return &protocolError{err, status}
 }
 
-// errorf creates a new error with an HTTP status.
-func errorf(status int, format string, a ...interface{}) *httpError {
+// errorf creates a new protocol error.
+func errorf(status int, format string, a ...interface{}) *protocolError {
 	return withStatus(fmt.Errorf(format, a...), status)
 }
 
-// Errorf creates a new error with an HTTP status.
-//
-// See also grpc.Errorf that accepts a gRPC code.
-func Errorf(status int, format string, a ...interface{}) error {
-	return errorf(status, format, a...)
-}
-
 func panicf(format string, a ...interface{}) {
 	panic(fmt.Errorf(format, a...))
 }
+
+func assert(condition bool) {

[dnj (Google), 2016/01/26 16:13:56]

Can we make an effort to get rid of all instances of this, either with softer
code (gRPC/protocol error) or by removing unnecessary assertions?

Panics in core server code are an easy DoS path and, as far as I can see, add no
real value here.

[nodir, 2016/01/26 18:01:07]

I don't panic unless there is an obvious bug in server-side code, but I don't
strongly object either.

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

Yeah it's a tough line to draw. A panic will completely reset a VM instance,
though, so it's a fairly high price to pay. If there's a non-panic alternative
(e.g., log and return Internal) it's IMO a better thing to deploy.

+	if !condition {
+		panicf("assertion failed")
+	}
+}