Skip to content

Commit

Permalink
explain how -output-curl-string works in comments to avoid confusion
Browse files Browse the repository at this point in the history
  • Loading branch information
raskchanky committed Oct 2, 2024
1 parent e7e16fd commit 5b90cd8
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
6 changes: 6 additions & 0 deletions api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion api/output_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"net/http"
"strings"

retryablehttp "github.com/hashicorp/go-retryablehttp"
"github.com/hashicorp/go-retryablehttp"
)

const (
Expand All @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions command/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5b90cd8

Please sign in to comment.