From 21f77f837e02050210f9e0b698d8ed7f0a9074af Mon Sep 17 00:00:00 2001
From: Owen Strain <owen.strain@oracle.com>
Date: Fri, 28 Sep 2018 17:26:54 -0700
Subject: [PATCH] Add APIErrorWrapper so that underlying errors can be logged
 (#1246)

---
 api/models/error.go            | 23 +++++++++++++++++++++++
 api/runnerpool/ch_placer.go    | 16 +++++++++++-----
 api/runnerpool/naive_placer.go | 16 +++++++++++-----
 api/server/error_response.go   |  3 +++
 4 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/api/models/error.go b/api/models/error.go
index 1620b6020..b0d1f350d 100644
--- a/api/models/error.go
+++ b/api/models/error.go
@@ -194,3 +194,26 @@ type ErrorWrapper struct {
 func (m *ErrorWrapper) Validate() error {
 	return nil
 }
+
+// APIErrorWrapper wraps an error with an APIError such that the APIError
+// governs the HTTP response but the root error remains accessible.
+type APIErrorWrapper interface {
+	APIError
+	RootError() error
+}
+
+type apiErrorWrapper struct {
+	APIError
+	root error
+}
+
+func (w apiErrorWrapper) RootError() error {
+	return w.root
+}
+
+func NewAPIErrorWrapper(apiErr APIError, rootErr error) APIErrorWrapper {
+	return &apiErrorWrapper{
+		APIError: apiErr,
+		root:     rootErr,
+	}
+}
diff --git a/api/runnerpool/ch_placer.go b/api/runnerpool/ch_placer.go
index 9d394851f..84c6c2dd2 100644
--- a/api/runnerpool/ch_placer.go
+++ b/api/runnerpool/ch_placer.go
@@ -34,12 +34,10 @@ func (p *chPlacer) PlaceCall(rp RunnerPool, ctx context.Context, call RunnerCall
 	key := call.Model().FnID
 	sum64 := siphash.Hash(0, 0x4c617279426f6174, []byte(key))
 
+	var runnerPoolErr error
 	for {
-		runners, err := rp.Runners(call)
-		if err != nil {
-			state.HandleFindRunnersFailure(err)
-			return err
-		}
+		var runners []Runner
+		runners, runnerPoolErr = rp.Runners(call)
 
 		i := int(jumpConsistentHash(sum64, int32(len(runners))))
 		for j := 0; j < len(runners) && !state.IsDone(); j++ {
@@ -59,6 +57,14 @@ func (p *chPlacer) PlaceCall(rp RunnerPool, ctx context.Context, call RunnerCall
 		}
 	}
 
+	if runnerPoolErr != nil {
+		// If we haven't been able to place the function and we got an error
+		// from the runner pool, return that error (since we don't have
+		// enough runners to handle the current load and the runner pool is
+		// having trouble).
+		state.HandleFindRunnersFailure(runnerPoolErr)
+		return runnerPoolErr
+	}
 	return models.ErrCallTimeoutServerBusy
 }
 
diff --git a/api/runnerpool/naive_placer.go b/api/runnerpool/naive_placer.go
index c1c5631bc..d8a65caf2 100644
--- a/api/runnerpool/naive_placer.go
+++ b/api/runnerpool/naive_placer.go
@@ -28,12 +28,10 @@ func (sp *naivePlacer) PlaceCall(rp RunnerPool, ctx context.Context, call Runner
 	state := NewPlacerTracker(ctx, &sp.cfg)
 	defer state.HandleDone()
 
+	var runnerPoolErr error
 	for {
-		runners, err := rp.Runners(call)
-		if err != nil {
-			state.HandleFindRunnersFailure(err)
-			return err
-		}
+		var runners []Runner
+		runners, runnerPoolErr = rp.Runners(call)
 
 		for j := 0; j < len(runners) && !state.IsDone(); j++ {
 
@@ -51,5 +49,13 @@ func (sp *naivePlacer) PlaceCall(rp RunnerPool, ctx context.Context, call Runner
 		}
 	}
 
+	if runnerPoolErr != nil {
+		// If we haven't been able to place the function and we got an error
+		// from the runner pool, return that error (since we don't have
+		// enough runners to handle the current load and the runner pool is
+		// having trouble).
+		state.HandleFindRunnersFailure(runnerPoolErr)
+		return runnerPoolErr
+	}
 	return models.ErrCallTimeoutServerBusy
 }
diff --git a/api/server/error_response.go b/api/server/error_response.go
index b18061718..6a9157145 100644
--- a/api/server/error_response.go
+++ b/api/server/error_response.go
@@ -27,6 +27,9 @@ func handleErrorResponse(c *gin.Context, err error) {
 // HandleErrorResponse used to handle response errors in the same way.
 func HandleErrorResponse(ctx context.Context, w http.ResponseWriter, err error) {
 	log := common.Logger(ctx)
+	if w, ok := err.(models.APIErrorWrapper); ok {
+		log = log.WithField("root_error", w.RootError())
+	}
 
 	if ctx.Err() == context.Canceled {
 		log.Info("client context cancelled")