Better error handling for plugins (#990)

* Better error handling for plugins

* Remove unneeded field

* add test

pkg/cmd/plugin_cmds.go

@@ -80,6 +80,10 @@ func (ptc *pluginTemplateCmd) runPluginCmd(cmd *cobra.Command, args []string) er
 		log.WithFields(log.Fields{
 			"prefix": "pluginTemplateCmd.runPluginCmd",
 		}).Debug(fmt.Sprintf("Plugin command '%s' exited with error: %s", plugin.Shortname, err))
+
+		// We can't return err because the plugin will have already printed the error message at
+		// this point, and we can't return nil because the host will exit with code 0.
+		os.Exit(1)
 	}
 
 	return nil

pkg/requests/base.go

@@ -184,8 +184,9 @@ func (rb *Base) MakeMultiPartRequest(ctx context.Context, apiKey, path string, p
 		return []byte{}, err
 	}
 
-	configure := func(req *http.Request) {
+	configure := func(req *http.Request) error {
 		req.Header.Set("Content-Type", contentType)
+		return nil
 	}
 
 	return rb.performRequest(ctx, apiKey, path, params, reqBody.String(), errOnStatus, configure)
@@ -201,7 +202,7 @@ func (rb *Base) MakeRequest(ctx context.Context, apiKey, path string, params *Re
 	return rb.performRequest(ctx, apiKey, path, params, data, errOnStatus, nil)
 }
 
-func (rb *Base) performRequest(ctx context.Context, apiKey, path string, params *RequestParameters, data string, errOnStatus bool, additionalConfigure func(req *http.Request)) ([]byte, error) {
+func (rb *Base) performRequest(ctx context.Context, apiKey, path string, params *RequestParameters, data string, errOnStatus bool, additionalConfigure func(req *http.Request) error) ([]byte, error) {
 	parsedBaseURL, err := url.Parse(rb.APIBaseURL)
 	if err != nil {
 		return []byte{}, err
@@ -213,13 +214,16 @@ func (rb *Base) performRequest(ctx context.Context, apiKey, path string, params
 		Verbose: rb.showHeaders,
 	}
 
-	configure := func(req *http.Request) {
+	configure := func(req *http.Request) error {
 		rb.setIdempotencyHeader(req, params)
 		rb.setStripeAccountHeader(req, params)
 		rb.setVersionHeader(req, params)
 		if additionalConfigure != nil {
-			additionalConfigure(req)
+			if err := additionalConfigure(req); err != nil {
+				return err
+			}
 		}
+		return nil
 	}
 
 	resp, err := client.PerformRequest(ctx, rb.Method, path, data, configure)

pkg/stripe/client.go

@@ -51,7 +51,7 @@ type Client struct {
 }
 
 // PerformRequest sends a request to Stripe and returns the response.
-func (c *Client) PerformRequest(ctx context.Context, method, path string, params string, configure func(*http.Request)) (*http.Response, error) {
+func (c *Client) PerformRequest(ctx context.Context, method, path string, params string, configure func(*http.Request) error) (*http.Response, error) {
 	url, err := url.Parse(path)
 	if err != nil {
 		return nil, err
@@ -81,7 +81,9 @@ func (c *Client) PerformRequest(ctx context.Context, method, path string, params
 	}
 
 	if configure != nil {
-		configure(req)
+		if err := configure(req); err != nil {
+			return nil, err
+		}
 	}
 
 	if c.httpClient == nil {

pkg/stripe/client_test.go

@@ -2,6 +2,7 @@ package stripe
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -135,10 +136,33 @@ func TestPerformRequest_ConfigureFunc(t *testing.T) {
 		BaseURL: baseURL,
 	}
 
-	resp, err := client.PerformRequest(context.Background(), http.MethodGet, "/get", "", func(r *http.Request) {
+	resp, err := client.PerformRequest(context.Background(), http.MethodGet, "/get", "", func(r *http.Request) error {
 		r.Header.Add("Stripe-Version", "2019-07-10")
+		return nil
 	})
 	require.NoError(t, err)
 
 	defer resp.Body.Close()
 }
+
+func TestPerformRequest_ConfigureFuncReturnsError(t *testing.T) {
+	serverCalled := false
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		serverCalled = true
+	}))
+	defer ts.Close()
+
+	baseURL, _ := url.Parse(ts.URL)
+	client := Client{
+		BaseURL: baseURL,
+	}
+
+	resp, err := client.PerformRequest(context.Background(), http.MethodGet, "/get", "", func(r *http.Request) error {
+		return errors.New("foo")
+	})
+	require.Equal(t, errors.New("foo"), err)
+	require.False(t, serverCalled)
+	if resp != nil {
+		resp.Body.Close()
+	}
+}