fix: address browser curl review findings

Made-with: Cursor

Author: rgarcia

cmd/browsers.go

@@ -3301,17 +3301,43 @@ type BrowsersCurlInput struct {
 	Silent     bool
 }
 
-func parseCurlHeaders(raw []string) map[string]string {
+type silentCurlError struct {
+	err error
+}
+
+func (e silentCurlError) Error() string {
+	return e.err.Error()
+}
+
+func (e silentCurlError) Unwrap() error {
+	return e.err
+}
+
+func (e silentCurlError) Silent() bool {
+	return true
+}
+
+func curlError(in BrowsersCurlInput, err error) error {
+	if err == nil {
+		return nil
+	}
+	if in.Silent {
+		return silentCurlError{err: err}
+	}
+	return err
+}
+
+func parseCurlHeaders(raw []string) http.Header {
 	if len(raw) == 0 {
 		return nil
 	}
-	headers := make(map[string]string)
+	headers := make(http.Header)
 	for _, h := range raw {
 		k, v, ok := strings.Cut(h, ":")
 		if !ok {
 			continue
 		}
-		headers[strings.TrimSpace(k)] = strings.TrimSpace(v)
+		headers.Add(strings.TrimSpace(k), strings.TrimSpace(v))
 	}
 	return headers
 }
@@ -3328,7 +3354,7 @@ func readCurlBody(in BrowsersCurlInput) (string, error) {
 	return string(data), nil
 }
 
-func hasCurlHeader(headers map[string]string, name string) bool {
+func hasCurlHeader(headers http.Header, name string) bool {
 	for key := range headers {
 		if strings.EqualFold(key, name) {
 			return true
@@ -3375,18 +3401,18 @@ func openCurlOutputFile(path string) (io.Writer, func() error, error) {
 func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 	body, err := readCurlBody(in)
 	if err != nil {
-		return err
+		return curlError(in, err)
 	}
 
 	method := in.Method
 	if method == "" {
 		method = "GET"
-		if in.Head {
-			method = "HEAD"
-		}
 		if body != "" {
 			method = "POST"
 		}
+		if in.Head {
+			method = "HEAD"
+		}
 	}
 	include := in.Include || in.Head
 
@@ -3397,24 +3423,26 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 
 	// Seed the SDK's browser route cache before constructing the raw curl client.
 	if _, err := b.browsers.Get(ctx, in.Identifier, kernel.BrowserGetParams{}); err != nil {
-		return util.CleanedUpSdkError{Err: err}
+		return curlError(in, util.CleanedUpSdkError{Err: err})
 	}
 
 	httpClient, err := b.browsers.HTTPClient(in.Identifier)
 	if err != nil {
-		return util.CleanedUpSdkError{Err: err}
+		return curlError(in, util.CleanedUpSdkError{Err: err})
 	}
 	if in.MaxTime > 0 {
 		httpClient.Timeout = in.MaxTime
 	}
 
 	req, err := http.NewRequestWithContext(ctx, method, in.URL, bodyReader)
 	if err != nil {
-		return fmt.Errorf("creating request: %w", err)
+		return curlError(in, fmt.Errorf("creating request: %w", err))
 	}
 	headers := parseCurlHeaders(in.Headers)
-	for key, value := range headers {
-		req.Header.Set(key, value)
+	for key, values := range headers {
+		for _, value := range values {
+			req.Header.Add(key, value)
+		}
 	}
 	if body != "" && !hasCurlHeader(headers, "Content-Type") {
 		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
@@ -3423,7 +3451,7 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 	start := time.Now()
 	resp, err := httpClient.Do(req)
 	if err != nil {
-		return fmt.Errorf("request failed: %w", err)
+		return curlError(in, fmt.Errorf("request failed: %w", err))
 	}
 	defer resp.Body.Close()
 
@@ -3432,7 +3460,7 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 	if in.OutputFile != "" {
 		f, err := os.Create(in.OutputFile)
 		if err != nil {
-			return fmt.Errorf("creating output file: %w", err)
+			return curlError(in, fmt.Errorf("creating output file: %w", err))
 		}
 		outputFile = f
 		defer outputFile.Close()
@@ -3442,7 +3470,7 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 	if in.DumpHeader != "" {
 		headerWriter, closeHeaderWriter, err := openCurlOutputFile(in.DumpHeader)
 		if err != nil {
-			return fmt.Errorf("creating dump header file: %w", err)
+			return curlError(in, fmt.Errorf("creating dump header file: %w", err))
 		}
 		defer closeHeaderWriter()
 		writeCurlResponseHeaders(headerWriter, resp)
@@ -3455,7 +3483,7 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 				timeTotal:  time.Since(start),
 			}))
 		}
-		return fmt.Errorf("HTTP error: %s", resp.Status)
+		return curlError(in, fmt.Errorf("HTTP error: %s", resp.Status))
 	}
 
 	if include {
@@ -3467,9 +3495,9 @@ func (b BrowsersCmd) Curl(ctx context.Context, in BrowsersCurlInput) error {
 		sizeDownload, err = io.Copy(writer, resp.Body)
 		if err != nil {
 			if in.OutputFile != "" {
-				return fmt.Errorf("writing output file: %w", err)
+				return curlError(in, fmt.Errorf("writing output file: %w", err))
 			}
-			return err
+			return curlError(in, err)
 		}
 	}
 

cmd/browsers_test.go

@@ -125,7 +125,7 @@ func (f *FakeBrowsersService) LoadExtensions(ctx context.Context, id string, bod
 func TestBrowsersCurlRawUsesBrowserHTTPClient(t *testing.T) {
 	var (
 		gotMethod      string
-		gotHeader      string
+		gotHeaders     []string
 		gotContentType string
 		gotBody        string
 	)
@@ -135,7 +135,7 @@ func TestBrowsersCurlRawUsesBrowserHTTPClient(t *testing.T) {
 		require.NoError(t, err)
 
 		gotMethod = r.Method
-		gotHeader = r.Header.Get("X-Test")
+		gotHeaders = r.Header.Values("X-Test")
 		gotContentType = r.Header.Get("Content-Type")
 		gotBody = string(body)
 
@@ -163,7 +163,7 @@ func TestBrowsersCurlRawUsesBrowserHTTPClient(t *testing.T) {
 	err := b.Curl(context.Background(), BrowsersCurlInput{
 		Identifier: "brw_123",
 		URL:        srv.URL + "/target",
-		Headers:    []string{"X-Test: yes"},
+		Headers:    []string{"X-Test: yes", "X-Test: also-yes"},
 		Data:       "hello",
 		OutputFile: outputFile,
 		Silent:     true,
@@ -174,7 +174,7 @@ func TestBrowsersCurlRawUsesBrowserHTTPClient(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, getCalled)
 	assert.Equal(t, http.MethodPost, gotMethod)
-	assert.Equal(t, "yes", gotHeader)
+	assert.Equal(t, []string{"yes", "also-yes"}, gotHeaders)
 	assert.Equal(t, "application/x-www-form-urlencoded", gotContentType)
 	assert.Equal(t, "hello", gotBody)
 	assert.Equal(t, "proxied", string(data))
@@ -239,6 +239,7 @@ func TestBrowsersCurlHeadWritesHeadersOnly(t *testing.T) {
 	err := b.Curl(context.Background(), BrowsersCurlInput{
 		Identifier: "brw_123",
 		URL:        srv.URL + "/target",
+		Data:       "hello",
 		OutputFile: outputFile,
 		Head:       true,
 	})
@@ -252,6 +253,29 @@ func TestBrowsersCurlHeadWritesHeadersOnly(t *testing.T) {
 	assert.NotContains(t, string(data), "proxied")
 }
 
+func TestBrowsersCurlSilentWrapsErrors(t *testing.T) {
+	fake := &FakeBrowsersService{
+		GetFunc: func(ctx context.Context, id string, query kernel.BrowserGetParams, opts ...option.RequestOption) (*kernel.BrowserGetResponse, error) {
+			return &kernel.BrowserGetResponse{}, nil
+		},
+		HTTPClientFunc: func(id string, opts ...option.RequestOption) (*http.Client, error) {
+			return http.DefaultClient, nil
+		},
+	}
+
+	b := BrowsersCmd{browsers: fake}
+	err := b.Curl(context.Background(), BrowsersCurlInput{
+		Identifier: "brw_123",
+		URL:        "://not-a-url",
+		Silent:     true,
+	})
+	require.Error(t, err)
+
+	var silent interface{ Silent() bool }
+	require.ErrorAs(t, err, &silent)
+	assert.True(t, silent.Silent())
+}
+
 func TestBrowsersCurlDumpHeaderAndWriteOut(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("X-Test", "yes")

cmd/root.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -184,6 +185,10 @@ func Execute(m Metadata) {
 		fang.WithCommit(metadata.Commit),
 		fang.WithErrorHandler(func(w io.Writer, styles fang.Styles, err error) {
 			err = util.CleanedUpSdkError{Err: err}
+			var silent interface{ Silent() bool }
+			if errors.As(err, &silent) && silent.Silent() {
+				return
+			}
 			// remove margins so that it matches other pterm.error "style"
 			// we should add them back later as it looks cleaner
 			errorTextStyle := styles.ErrorText.UnsetMargins()