Merge pull request #3824 from smarterclayton/allow_panic_and_error_reporting

Allow panics and unhandled errors to be sent elsewhere

pkg/apiserver/apiserver.go

@@ -31,6 +31,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
 
 	"github.com/emicklei/go-restful"
@@ -372,6 +373,7 @@ func errorJSON(err error, codec runtime.Codec, w http.ResponseWriter) {
 
 // errorJSONFatal renders an error to the response, and if codec fails will render plaintext
 func errorJSONFatal(err error, codec runtime.Codec, w http.ResponseWriter) {
+	util.HandleError(fmt.Errorf("apiserver was unable to write a JSON response: %v", err))
 	status := errToAPIStatus(err)
 	output, err := codec.Encode(status)
 	if err != nil {
@@ -386,7 +388,6 @@ func errorJSONFatal(err error, codec runtime.Codec, w http.ResponseWriter) {
 
 // writeRawJSON writes a non-API object in JSON.
 func writeRawJSON(statusCode int, object interface{}, w http.ResponseWriter) {
-	// PR #2243: Pretty-print JSON by default.
 	output, err := json.MarshalIndent(object, "", "  ")
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)

pkg/apiserver/errors.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
-	"github.com/golang/glog"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 // statusError is an object that can be converted into an api.Status
@@ -49,7 +49,7 @@ func errToAPIStatus(err error) *api.Status {
 		// by REST storage - these typically indicate programmer
 		// error by not using pkg/api/errors, or unexpected failure
 		// cases.
-		glog.V(1).Infof("An unchecked error was received: %v", err)
+		util.HandleError(fmt.Errorf("apiserver received an error that is not an api.Status: %v", err))
 		return &api.Status{
 			Status:  api.StatusFailure,
 			Code:    status,

pkg/controller/replication_controller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controller
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -65,15 +66,15 @@ func (r RealPodControl) createReplica(namespace string, controller api.Replicati
 		},
 	}
 	if err := api.Scheme.Convert(&controller.Spec.Template.Spec, &pod.Spec); err != nil {
-		glog.Errorf("Unable to convert pod template: %v", err)
+		util.HandleError(fmt.Errorf("unable to convert pod template: %v", err))
 		return
 	}
 	if labels.Set(pod.Labels).AsSelector().Empty() {
-		glog.Errorf("Unable to create pod replica, no labels")
+		util.HandleError(fmt.Errorf("unable to create pod replica, no labels"))
 		return
 	}
 	if _, err := r.kubeClient.Pods(namespace).Create(pod); err != nil {
-		glog.Errorf("Unable to create pod replica: %v", err)
+		util.HandleError(fmt.Errorf("unable to create pod replica: %v", err))
 	}
 }
 
@@ -108,7 +109,7 @@ func (rm *ReplicationManager) watchControllers(resourceVersion *string) {
 		*resourceVersion,
 	)
 	if err != nil {
-		glog.Errorf("Unexpected failure to watch: %v", err)
+		util.HandleError(fmt.Errorf("unable to watch: %v", err))
 		time.Sleep(5 * time.Second)
 		return
 	}
@@ -125,13 +126,13 @@ func (rm *ReplicationManager) watchControllers(resourceVersion *string) {
 				return
 			}
 			if event.Type == watch.Error {
-				glog.Errorf("error from watch during sync: %v", errors.FromObject(event.Object))
+				util.HandleError(fmt.Errorf("error from watch during sync: %v", errors.FromObject(event.Object)))
 				continue
 			}
 			glog.V(4).Infof("Got watch: %#v", event)
 			rc, ok := event.Object.(*api.ReplicationController)
 			if !ok {
-				glog.Errorf("unexpected object: %#v", event.Object)
+				util.HandleError(fmt.Errorf("unexpected object: %#v", event.Object))
 				continue
 			}
 			// If we get disconnected, start where we left off.
@@ -140,7 +141,7 @@ func (rm *ReplicationManager) watchControllers(resourceVersion *string) {
 			// it in the desired state.
 			glog.V(4).Infof("About to sync from watch: %v", rc.Name)
 			if err := rm.syncHandler(*rc); err != nil {
-				glog.Errorf("unexpected sync. error: %v", err)
+				util.HandleError(fmt.Errorf("unexpected sync error: %v", err))
 			}
 		}
 	}
@@ -199,7 +200,7 @@ func (rm *ReplicationManager) synchronize() {
 	var controllers []api.ReplicationController
 	list, err := rm.kubeClient.ReplicationControllers(api.NamespaceAll).List(labels.Everything())
 	if err != nil {
-		glog.Errorf("Synchronization error: %v (%#v)", err, err)
+		util.HandleError(fmt.Errorf("synchronization error: %v", err))
 		return
 	}
 	controllers = list.Items
@@ -211,7 +212,7 @@ func (rm *ReplicationManager) synchronize() {
 			glog.V(4).Infof("periodic sync of %v", controllers[ix].Name)
 			err := rm.syncHandler(controllers[ix])
 			if err != nil {
-				glog.Errorf("Error synchronizing: %v", err)
+				util.HandleError(fmt.Errorf("error synchronizing: %v", err))
 			}
 		}(ix)
 	}

pkg/util/util.go

@@ -34,14 +34,23 @@ import (
 // For testing, bypass HandleCrash.
 var ReallyCrash bool
 
+// PanicHandlers is a list of functions which will be invoked when a panic happens.
+var PanicHandlers = []func(interface{}){logPanic}
+
 // HandleCrash simply catches a crash and logs an error. Meant to be called via defer.
 func HandleCrash() {
 	if ReallyCrash {
 		return
 	}
+	if r := recover(); r != nil {
+		for _, fn := range PanicHandlers {
+			fn(r)
+		}
+	}
+}
 
-	r := recover()
+// logPanic logs the caller tree when a panic occurs.
-	if r != nil {
+func logPanic(r interface{}) {
 	callers := ""
 	for i := 0; true; i++ {
 		_, file, line, ok := runtime.Caller(i)
@@ -52,6 +61,24 @@ func HandleCrash() {
 	}
 	glog.Infof("Recovered from panic: %#v (%v)\n%v", r, r, callers)
 }
+
+// ErrorHandlers is a list of functions which will be invoked when an unreturnable
+// error occurs.
+var ErrorHandlers = []func(error){logError}
+
+// HandlerError is a method to invoke when a non-user facing piece of code cannot
+// return an error and needs to indicate it has been ignored. Invoking this method
+// is preferable to logging the error - the default behavior is to log but the
+// errors may be sent to a remote server for analysis.
+func HandleError(err error) {
+	for _, fn := range ErrorHandlers {
+		fn(err)
+	}
+}
+
+// logError prints an error with the call stack of the location it was reported
+func logError(err error) {
+	glog.ErrorDepth(2, err)
 }
 
 // Forever loops forever running f every period.  Catches any panics, and keeps going.

pkg/util/util_test.go

@@ -18,6 +18,7 @@ package util
 
 import (
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -59,6 +60,40 @@ func TestHandleCrash(t *testing.T) {
 	}
 }
 
+func TestCustomHandleCrash(t *testing.T) {
+	old := PanicHandlers
+	defer func() { PanicHandlers = old }()
+	var result interface{}
+	PanicHandlers = []func(interface{}){
+		func(r interface{}) {
+			result = r
+		},
+	}
+	func() {
+		defer HandleCrash()
+		panic("test")
+	}()
+	if result != "test" {
+		t.Errorf("did not receive custom handler")
+	}
+}
+
+func TestCustomHandleError(t *testing.T) {
+	old := ErrorHandlers
+	defer func() { ErrorHandlers = old }()
+	var result error
+	ErrorHandlers = []func(error){
+		func(err error) {
+			result = err
+		},
+	}
+	err := fmt.Errorf("test")
+	HandleError(err)
+	if result != err {
+		t.Errorf("did not receive custom handler")
+	}
+}
+
 func TestNewIntOrStringFromInt(t *testing.T) {
 	i := NewIntOrStringFromInt(93)
 	if i.Kind != IntstrInt || i.IntVal != 93 {