From 932ebbf31d9fe231513584258c756ddd1068f37f Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Wed, 20 Sep 2023 10:22:17 +0100
Subject: [PATCH] rc: always report an error as JSON

Before this change, the rclone rc command wouldn't actually report the
error as a JSON blob which is inconsitent with what the HTTP API does.

This change make sure we always report a JSON error, making a
synthetic one if necessary.

See: https://forum.rclone.org/t/when-using-rclone-rc-commands-somehow-return-errors-as-parsable-json/41855
Co-authored-by: Fawzib Rojas
---
 cmd/rc/rc.go | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/cmd/rc/rc.go b/cmd/rc/rc.go
index 49c3144747325..2c88d2914b1c1 100644
--- a/cmd/rc/rc.go
+++ b/cmd/rc/rc.go
@@ -168,6 +168,16 @@ func setAlternateFlag(flagName string, output *string) {
 	}
 }
 
+// Format an error and create a synthetic server return from it
+func errorf(status int, path string, format string, arg ...any) (out rc.Params, err error) {
+	err = fmt.Errorf(format, arg...)
+	out = make(rc.Params)
+	out["error"] = err.Error()
+	out["path"] = path
+	out["status"] = status
+	return out, err
+}
+
 // do a call from (path, in) to (out, err).
 //
 // if err is set, out may be a valid error return or it may be nil
@@ -176,16 +186,16 @@ func doCall(ctx context.Context, path string, in rc.Params) (out rc.Params, err
 	if loopback {
 		call := rc.Calls.Get(path)
 		if call == nil {
-			return nil, fmt.Errorf("method %q not found", path)
+			return errorf(http.StatusBadRequest, path, "loopback: method %q not found", path)
 		}
 		_, out, err := jobs.NewJob(ctx, call.Fn, in)
 		if err != nil {
-			return nil, fmt.Errorf("loopback call failed: %w", err)
+			return errorf(http.StatusInternalServerError, path, "loopback: call failed: %w", err)
 		}
 		// Reshape (serialize then deserialize) the data so it is in the form expected
 		err = rc.Reshape(&out, out)
 		if err != nil {
-			return nil, fmt.Errorf("loopback reshape failed: %w", err)
+			return errorf(http.StatusInternalServerError, path, "loopback: reshape failed: %w", err)
 		}
 		return out, nil
 	}
@@ -195,12 +205,12 @@ func doCall(ctx context.Context, path string, in rc.Params) (out rc.Params, err
 	url += path
 	data, err := json.Marshal(in)
 	if err != nil {
-		return nil, fmt.Errorf("failed to encode JSON: %w", err)
+		return errorf(http.StatusBadRequest, path, "failed to encode request: %w", err)
 	}
 
 	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(data))
 	if err != nil {
-		return nil, fmt.Errorf("failed to make request: %w", err)
+		return errorf(http.StatusInternalServerError, path, "failed to make request: %w", err)
 	}
 
 	req.Header.Set("Content-Type", "application/json")
@@ -210,28 +220,24 @@ func doCall(ctx context.Context, path string, in rc.Params) (out rc.Params, err
 
 	resp, err := client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("connection failed: %w", err)
+		return errorf(http.StatusServiceUnavailable, path, "connection failed: %w", err)
 	}
 	defer fs.CheckClose(resp.Body, &err)
 
-	if resp.StatusCode != http.StatusOK {
-		var body []byte
-		body, err = io.ReadAll(resp.Body)
-		var bodyString string
-		if err == nil {
-			bodyString = string(body)
-		} else {
-			bodyString = err.Error()
-		}
-		bodyString = strings.TrimSpace(bodyString)
-		return nil, fmt.Errorf("failed to read rc response: %s: %s", resp.Status, bodyString)
+	// Read response
+	var body []byte
+	var bodyString string
+	body, err = io.ReadAll(resp.Body)
+	bodyString = strings.TrimSpace(string(body))
+	if err != nil {
+		return errorf(resp.StatusCode, "failed to read rc response: %s: %s", resp.Status, bodyString)
 	}
 
 	// Parse output
 	out = make(rc.Params)
-	err = json.NewDecoder(resp.Body).Decode(&out)
+	err = json.NewDecoder(strings.NewReader(bodyString)).Decode(&out)
 	if err != nil {
-		return nil, fmt.Errorf("failed to decode JSON: %w", err)
+		return errorf(resp.StatusCode, path, "failed to decode response: %w: %s", err, bodyString)
 	}
 
 	// Check we got 200 OK