fix(cli): report output failures and retain saved request options

cmd/kurl/main.go

@@ -47,19 +47,23 @@ type cliOptions struct {
 }
 
 type savedRequest struct {
-	Method      string   `json:"method"`
-	URL         string   `json:"url"`
-	Data        string   `json:"data,omitempty"`
-	Headers     []string `json:"headers,omitempty"`
-	Timeout     string   `json:"timeout,omitempty"`
-	NoColor     bool     `json:"no_color,omitempty"`
-	HeadersOnly bool     `json:"headers_only,omitempty"`
-	BodyOnly    bool     `json:"body_only,omitempty"`
-	Raw         bool     `json:"raw,omitempty"`
-	Verbose     bool     `json:"verbose,omitempty"`
-	Timing      bool     `json:"timing,omitempty"`
-	OutputPath  string   `json:"output_path,omitempty"`
-	Env         string   `json:"env,omitempty"`
+	FilterQuery   string   `json:"filter_query,omitempty"`
+	FilterKeys    string   `json:"filter_keys,omitempty"`
+	FilterFlatten bool     `json:"filter_flatten,omitempty"`
+	HTTP3         bool     `json:"http3,omitempty"`
+	Method        string   `json:"method"`
+	URL           string   `json:"url"`
+	Data          string   `json:"data,omitempty"`
+	Headers       []string `json:"headers,omitempty"`
+	Timeout       string   `json:"timeout,omitempty"`
+	NoColor       bool     `json:"no_color,omitempty"`
+	HeadersOnly   bool     `json:"headers_only,omitempty"`
+	BodyOnly      bool     `json:"body_only,omitempty"`
+	Raw           bool     `json:"raw,omitempty"`
+	Verbose       bool     `json:"verbose,omitempty"`
+	Timing        bool     `json:"timing,omitempty"`
+	OutputPath    string   `json:"output_path,omitempty"`
+	Env           string   `json:"env,omitempty"`
 }
 
 func main() {
@@ -111,7 +115,7 @@ func runRequest(opts cliOptions) {
 		fatal(err)
 	}
 
-	if strings.HasPrefix(opts.url, "ws://") || strings.HasPrefix(opts.url, "wss://") {
+	if isWebSocketURL(opts.url) {
 		runWebSocket(opts)
 		return
 	}
@@ -133,6 +137,8 @@ func runRequest(opts cliOptions) {
 		fatal(err)
 	}
 
+	defer func() { _ = result.Response.Body.Close() }()
+
 	printerOptions := printer.Options{
 		Color:         useColor,
 		Raw:           opts.raw,
@@ -164,7 +170,9 @@ func runRequest(opts cliOptions) {
 			fatal(err)
 		}
 	}
-	bw.Flush()
+	if err := bw.Flush(); err != nil {
+		fatal(err)
+	}
 }
 
 func handleSaveCommand(args []string) {
@@ -276,9 +284,12 @@ func handleGraphQLCommand(args []string) {
 		case arg == "-v" || arg == "--verbose":
 			verbose = true
 		case !strings.HasPrefix(arg, "-"):
-			if targetURL == "" {
-				targetURL = arg
+			if targetURL != "" {
+				fatal(fmt.Errorf("unexpected argument %q", arg))
 			}
+			targetURL = arg
+		default:
+			fatal(fmt.Errorf("unknown option %q", arg))
 		}
 	}
 
@@ -310,7 +321,9 @@ func handleGraphQLCommand(args []string) {
 		bw.Flush()
 		fatal(err)
 	}
-	bw.Flush()
+	if err := bw.Flush(); err != nil {
+		fatal(err)
+	}
 }
 
 func handleSSECommand(args []string) {
@@ -357,9 +370,12 @@ func handleSSECommand(args []string) {
 		case arg == "--no-color":
 			noColor = true
 		case !strings.HasPrefix(arg, "-"):
-			if targetURL == "" {
-				targetURL = arg
+			if targetURL != "" {
+				fatal(fmt.Errorf("unexpected argument %q", arg))
 			}
+			targetURL = arg
+		default:
+			fatal(fmt.Errorf("unknown option %q", arg))
 		}
 	}
 
@@ -404,19 +420,23 @@ func saveRequestLocally(name string, opts cliOptions) error {
 	}
 
 	req := savedRequest{
-		Method:      opts.method,
-		URL:         opts.url,
-		Data:        opts.data,
-		Headers:     opts.headers,
-		Timeout:     opts.timeout.String(),
-		NoColor:     opts.noColor,
-		HeadersOnly: opts.headersOnly,
-		BodyOnly:    opts.bodyOnly,
-		Raw:         opts.raw,
-		Verbose:     opts.verbose,
-		Timing:      opts.timing,
-		OutputPath:  opts.outputPath,
-		Env:         opts.env,
+		FilterQuery:   opts.filterQuery,
+		FilterKeys:    opts.filterKeys,
+		FilterFlatten: opts.filterFlatten,
+		HTTP3:         opts.http3,
+		Method:        opts.method,
+		URL:           opts.url,
+		Data:          opts.data,
+		Headers:       opts.headers,
+		Timeout:       opts.timeout.String(),
+		NoColor:       opts.noColor,
+		HeadersOnly:   opts.headersOnly,
+		BodyOnly:      opts.bodyOnly,
+		Raw:           opts.raw,
+		Verbose:       opts.verbose,
+		Timing:        opts.timing,
+		OutputPath:    opts.outputPath,
+		Env:           opts.env,
 	}
 
 	data, err := json.MarshalIndent(req, "", "  ")
@@ -461,7 +481,10 @@ func loadRequestLocally(name string) (cliOptions, error) {
 	if req.Timeout != "" {
 		timeout, err = time.ParseDuration(req.Timeout)
 		if err != nil {
-			timeout = 30 * time.Second
+			return options, fmt.Errorf("invalid saved request timeout %q: %w", req.Timeout, err)
+		}
+		if timeout < 0 {
+			return options, fmt.Errorf("invalid saved request timeout %q: must not be negative", req.Timeout)
 		}
 	} else {
 		timeout = 30 * time.Second
@@ -480,6 +503,10 @@ func loadRequestLocally(name string) (cliOptions, error) {
 	options.timing = req.Timing
 	options.outputPath = req.OutputPath
 	options.env = req.Env
+	options.filterQuery = req.FilterQuery
+	options.filterKeys = req.FilterKeys
+	options.filterFlatten = req.FilterFlatten
+	options.http3 = req.HTTP3
 
 	return options, nil
 }

cmd/kurl/output_error_test.go

@@ -0,0 +1,59 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestRequestReportsBufferedOutputFailure(t *testing.T) {
+	if url := os.Getenv("KURL_TEST_CLOSED_OUTPUT_URL"); url != "" {
+		if err := os.Stdout.Close(); err != nil {
+			t.Fatal(err)
+		}
+		runRequest(cliOptions{url: url, method: "GET", timeout: time.Second})
+		os.Exit(0)
+	}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"ok":true}`))
+	}))
+	defer srv.Close()
+	cmd := exec.Command(os.Args[0], "-test.run=^TestRequestReportsBufferedOutputFailure$")
+	cmd.Env = append(os.Environ(), "KURL_TEST_CLOSED_OUTPUT_URL="+srv.URL)
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("request reported success despite a closed output destination")
+	}
+	if !strings.Contains(string(out), "file already closed") {
+		t.Fatalf("unexpected failure: %s", out)
+	}
+}
+
+func TestGraphQLReportsBufferedOutputFailure(t *testing.T) {
+	if url := os.Getenv("KURL_TEST_GRAPHQL_CLOSED_OUTPUT_URL"); url != "" {
+		if err := os.Stdout.Close(); err != nil {
+			t.Fatal(err)
+		}
+		handleGraphQLCommand([]string{url, "--query", "{ id }"})
+		os.Exit(0)
+	}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"data":{"id":1}}`))
+	}))
+	defer srv.Close()
+	cmd := exec.Command(os.Args[0], "-test.run=^TestGraphQLReportsBufferedOutputFailure$")
+	cmd.Env = append(os.Environ(), "KURL_TEST_GRAPHQL_CLOSED_OUTPUT_URL="+srv.URL)
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatal("GraphQL succeeded despite closed output")
+	}
+	if !strings.Contains(string(out), "file already closed") {
+		t.Fatalf("unexpected error: %s", out)
+	}
+}

cmd/kurl/response_close_test.go

@@ -0,0 +1,31 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+)
+
+func TestHeadersOnlyClosesUnreadResponse(t *testing.T) {
+	disconnected := make(chan struct{})
+	release := make(chan struct{})
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Length", "1000")
+		w.WriteHeader(http.StatusOK)
+		w.(http.Flusher).Flush()
+		select {
+		case <-r.Context().Done():
+			close(disconnected)
+		case <-release:
+		}
+	}))
+	defer server.Close()
+	defer close(release)
+	runRequest(cliOptions{url: server.URL, method: "GET", headersOnly: true, noColor: true})
+	select {
+	case <-disconnected:
+	case <-time.After(time.Second):
+		t.Fatal("headers-only request returned without closing its unread response")
+	}
+}

cmd/kurl/saved_options_test.go

@@ -0,0 +1,21 @@
+package main
+
+import (
+	"testing"
+	"time"
+)
+
+func TestSavedRequestPreservesFilteringAndProtocol(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	original := cliOptions{method: "GET", url: "https://example.com", timeout: time.Second, filterQuery: ".items", filterKeys: "id,name", filterFlatten: true, http3: true}
+	if err := saveRequestLocally("filtered", original); err != nil {
+		t.Fatal(err)
+	}
+	got, err := loadRequestLocally("filtered")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.filterQuery != original.filterQuery || got.filterKeys != original.filterKeys || !got.filterFlatten || !got.http3 {
+		t.Fatalf("saved options lost: query=%q keys=%q flatten=%v http3=%v", got.filterQuery, got.filterKeys, got.filterFlatten, got.http3)
+	}
+}

cmd/kurl/saved_timeout_test.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func TestSavedRequestRejectsInvalidTimeout(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
+	dir := filepath.Join(home, ".kurl", "requests")
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		t.Fatal(err)
+	}
+	for _, timeout := range []string{"-1s", "not-a-duration", "9999999999999999999999s"} {
+		data, err := json.Marshal(savedRequest{URL: "https://example.test", Timeout: timeout})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, "bad.json"), data, 0600); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := loadRequestLocally("bad"); err == nil {
+			t.Errorf("accepted timeout %q", timeout)
+		}
+	}
+	for _, timeout := range []time.Duration{0, time.Second} {
+		if err := saveRequestLocally("valid", cliOptions{url: "https://example.test", timeout: timeout}); err != nil {
+			t.Fatal(err)
+		}
+		got, err := loadRequestLocally("valid")
+		if err != nil || got.timeout != timeout {
+			t.Fatalf("valid timeout %v: %+v, %v", timeout, got, err)
+		}
+	}
+}

cmd/kurl/subcommand_arguments_test.go

@@ -0,0 +1,57 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"os/exec"
+	"strings"
+	"sync/atomic"
+	"testing"
+)
+
+func TestSubcommandsRejectUnexpectedArguments(t *testing.T) {
+	if os.Getenv("KURL_ARGUMENT_TEST") == "1" {
+		for i, arg := range os.Args {
+			if arg == "--" {
+				if os.Args[i+1] == "graphql" {
+					handleGraphQLCommand(os.Args[i+2:])
+				} else {
+					handleSSECommand(os.Args[i+2:])
+				}
+				return
+			}
+		}
+		t.Fatal("missing child arguments")
+	}
+	var requests atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requests.Add(1)
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"data":{}}`))
+	}))
+	defer server.Close()
+	for _, command := range []string{"graphql", "sse"} {
+		for _, unexpected := range []string{"--misspelled-option", "extra-endpoint"} {
+			t.Run(command+"/"+unexpected, func(t *testing.T) {
+				args := []string{"-test.run=^TestSubcommandsRejectUnexpectedArguments$", "--", command, server.URL}
+				if command == "graphql" {
+					args = append(args, "--query", "{ id }")
+				}
+				args = append(args, unexpected)
+				cmd := exec.Command(os.Args[0], args...)
+				cmd.Env = append(os.Environ(), "KURL_ARGUMENT_TEST=1")
+				output, err := cmd.CombinedOutput()
+				if err == nil {
+					t.Fatalf("unexpected argument succeeded: %s", output)
+				}
+				if !strings.Contains(string(output), unexpected) {
+					t.Fatalf("error omitted argument: %s", output)
+				}
+			})
+		}
+	}
+	if got := requests.Load(); got != 0 {
+		t.Errorf("invalid commands sent %d requests", got)
+	}
+}