From 3247e69cf56016cb361cbfdab6e746a15c11e469 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Matczuk?= <michal@scylladb.com>
Date: Wed, 21 Aug 2019 14:50:18 +0200
Subject: [PATCH] fs/rc/jobs: ExecuteJob propagate the error returned by
 function

Without this patch the resulting error is first converted to string and then recreated.
This makes it impossible to use the defined error types to figure out the cause of the error,
and may result in invalid HTTP status codes.

This patch adds a test TestExecuteJobErrorPropagation to validate that the errors are
properly propagated.
---
 fs/rc/jobs/job.go      | 13 ++++++++-----
 fs/rc/jobs/job_test.go | 11 +++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/rc/jobs/job.go b/fs/rc/jobs/job.go
index 186127d0f..30c80cf91 100644
--- a/fs/rc/jobs/job.go
+++ b/fs/rc/jobs/job.go
@@ -29,6 +29,11 @@ type Job struct {
 	Duration  float64   `json:"duration"`
 	Output    rc.Params `json:"output"`
 	Stop      func()    `json:"-"`
+
+	// realErr is the Error before printing it as a string, it's used to return
+	// the real error to the upper application layers while still printing the
+	// string error message.
+	realErr error
 }
 
 // Jobs describes a collection of running tasks
@@ -122,9 +127,11 @@ func (job *Job) finish(out rc.Params, err error) {
 	job.Output = out
 	job.Duration = job.EndTime.Sub(job.StartTime).Seconds()
 	if err != nil {
+		job.realErr = err
 		job.Error = err.Error()
 		job.Success = false
 	} else {
+		job.realErr = nil
 		job.Error = ""
 		job.Success = true
 	}
@@ -221,11 +228,7 @@ func StartAsyncJob(fn rc.Func, in rc.Params) (rc.Params, error) {
 func ExecuteJob(ctx context.Context, fn rc.Func, in rc.Params) (rc.Params, int64, error) {
 	job, ctx := running.NewSyncJob(ctx, in)
 	job.run(ctx, fn, in)
-	var err error
-	if !job.Success {
-		err = errors.New(job.Error)
-	}
-	return job.Output, job.ID, err
+	return job.Output, job.ID, job.realErr
 }
 
 func init() {
diff --git a/fs/rc/jobs/job_test.go b/fs/rc/jobs/job_test.go
index 3125c6cf4..dbd82e773 100644
--- a/fs/rc/jobs/job_test.go
+++ b/fs/rc/jobs/job_test.go
@@ -213,6 +213,17 @@ func TestExecuteJob(t *testing.T) {
 	assert.Equal(t, int64(1), id)
 }
 
+func TestExecuteJobErrorPropagation(t *testing.T) {
+	jobID = 0
+
+	testErr := errors.New("test error")
+	errorFn := func(ctx context.Context, in rc.Params) (out rc.Params, err error) {
+		return nil, testErr
+	}
+	_, _, err := ExecuteJob(context.Background(), errorFn, rc.Params{})
+	assert.Equal(t, testErr, err)
+}
+
 func TestRcJobStatus(t *testing.T) {
 	jobID = 0
 	_, err := StartAsyncJob(longFn, rc.Params{})