explain how -output-curl-string works in comments to avoid confusion (#…

…28576)

api/client.go

@@ -1467,6 +1467,12 @@ START:
 	}
 
 	if outputCurlString {
+		// Note that although we're building this up here and returning it as an error object, the Error()
+		// interface method on it only gets called in a context where the actual string returned from that
+		// method is irrelevant, because it gets swallowed by an error buffer that's never output to the user.
+		// That's on purpose, not a bug, because in this case, OutputStringError is not really an _error_, per se.
+		// It's just a way of aborting the control flow so that requests don't actually execute, and instead,
+		// we can detect what's happened back in the CLI machinery and show the actual curl string to the user.
 		LastOutputStringError = &OutputStringError{
 			Request:       req,
 			TLSSkipVerify: c.config.HttpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify,

api/output_string.go

@@ -8,7 +8,7 @@ import (
 	"net/http"
 	"strings"
 
-	retryablehttp "github.com/hashicorp/go-retryablehttp"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 const (
@@ -25,6 +25,10 @@ type OutputStringError struct {
 	finalCurlString            string
 }
 
+// Error is here so that we can return this struct as an error from client.rawRequestWithContext(). Note that
+// the ErrOutputStringRequest constant is never actually used and is completely irrelevant to how this all functions.
+// We could've just as easily returned an empty string. What matters is the machinery that happens before then where
+// the curl string is built. So yes, this is confusing, but yes, this is also on purpose, and it is not incorrect.
 func (d *OutputStringError) Error() string {
 	if d.finalCurlString == "" {
 		cs, err := d.buildCurlString()

command/main.go

@@ -186,6 +186,8 @@ func RunCustom(args []string, runOpts *RunOptions) int {
 		runOpts.Stderr = colorable.NewNonColorable(runOpts.Stderr)
 	}
 
+	// This bytes.Buffer override of the uiErrWriter is why we don't see errors printed to the screen
+	// when running commands with e.g. -output-curl-string
 	uiErrWriter := runOpts.Stderr
 	if outputCurlString || outputPolicy {
 		uiErrWriter = &bytes.Buffer{}
@@ -318,6 +320,9 @@ func generateCurlString(exitCode int, runOpts *RunOptions, preParsingErrBuf *byt
 		return 1
 	}
 
+	// When we actually return from client.rawRequestWithContext(), this value should be set to the OutputStringError
+	// that contains the data/context required to output the actual string, so it's doubtful this chunk of code will
+	// ever run, but I'm guessing it's a defense in depth thing.
 	if api.LastOutputStringError == nil {
 		if exitCode == 127 {
 			// Usage, just pass it through