From 721c0f1255ea873c75ad4bdee3b358f001c880fd Mon Sep 17 00:00:00 2001
From: Denis Makogon <lildee1991@gmail.com>
Date: Fri, 28 Jul 2017 12:52:51 +0300
Subject: [PATCH] Improving erro handling while trying to reserve tasks at
 async runner

 Each time when MQ becomes unreachable HTTP GET /tasks returned HTTP 500
 and code was not handling this case except expecting networking errors.
 After that it tried to unmarshal empty response body that caused another sort of an error.

 This patch triggers error based on http response code, explicitly checking if response code
 is something unexpected (not HTTP 200 OK).

 Response status code for /tasks for changed from 202 Accepted to 200 OK according to swagger doc.
---
 api/runner/async_runner.go | 21 ++++++++++++++-------
 api/server/server.go       |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/api/runner/async_runner.go b/api/runner/async_runner.go
index 0cf0d1f0b..8dcde6c14 100644
--- a/api/runner/async_runner.go
+++ b/api/runner/async_runner.go
@@ -14,30 +14,37 @@ import (
 	"sync"
 	"time"
 
+	"fmt"
 	"github.com/Sirupsen/logrus"
-	"github.com/opentracing/opentracing-go"
 	"github.com/fnproject/fn/api/models"
 	"github.com/fnproject/fn/api/runner/common"
 	"github.com/fnproject/fn/api/runner/task"
+	"github.com/opentracing/opentracing-go"
 )
 
 func getTask(ctx context.Context, url string) (*models.Task, error) {
-	// TODO shove this ctx into the request?
+	ctx, log := common.LoggerWithFields(ctx, logrus.Fields{"runner": "async"})
 	span, _ := opentracing.StartSpanFromContext(ctx, "get_task")
 	defer span.Finish()
 
-	// TODO uh, make a better http client :facepalm:
-	resp, err := http.Get(url)
-	if err != nil {
-		return nil, err
-	}
+	req, _ := http.NewRequest("GET", url, nil)
+	resp, err := http.DefaultClient.Do(req.WithContext(ctx))
 	defer func() {
 		io.Copy(ioutil.Discard, resp.Body)
 		resp.Body.Close()
 	}()
+	if err != nil {
+		return nil, err
+	}
+	if resp.StatusCode != http.StatusOK {
+		return nil, errors.New(fmt.Sprintf("Unable to get task. Reason %v", resp.Status))
+	}
 
 	var task models.Task
 	err = json.NewDecoder(resp.Body).Decode(&task)
+	if err != nil {
+		log.WithError(err).Error("Unable to decode task from response object")
+	}
 	if err != nil {
 		return nil, err
 	}
diff --git a/api/server/server.go b/api/server/server.go
index 5b10e86dc..57827b42e 100644
--- a/api/server/server.go
+++ b/api/server/server.go
@@ -267,7 +267,7 @@ func (s *Server) handleTaskRequest(c *gin.Context) {
 			handleErrorResponse(c, err)
 			return
 		}
-		c.JSON(http.StatusAccepted, task)
+		c.JSON(http.StatusOK, task)
 	case "DELETE":
 		body, err := ioutil.ReadAll(c.Request.Body)
 		if err != nil {