server/prpc: updated server according to protocol

server/prpc/error.go

 // 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"
+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")
+        }
+}