Skip to content

Commit

Permalink
chore: Fix error response with no body (#365)
Browse files Browse the repository at this point in the history
Unauthenticated client handles error response without body
  • Loading branch information
dbolson authored Jul 3, 2024
1 parent ad88cfa commit 36dc28f
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 51 deletions.
69 changes: 69 additions & 0 deletions internal/login/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package login

import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"

"github.com/launchdarkly/ldcli/internal/errors"
)

type UnauthenticatedClient interface {
MakeRequest(
method string,
path string,
data []byte,
) ([]byte, error)
}

type Client struct {
cliVersion string
}

func NewClient(cliVersion string) Client {
return Client{
cliVersion: cliVersion,
}
}

func (c Client) MakeRequest(
method string,
path string,
data []byte,
) ([]byte, error) {
client := http.Client{}
req, _ := http.NewRequest(method, path, bytes.NewReader(data))
req.Header.Add("Content-Type", "application/json")
req.Header.Set("User-Agent", fmt.Sprintf("launchdarkly-cli/v%s", c.cliVersion))
res, err := client.Do(req)
if err != nil {
return nil, err
}

body, err := io.ReadAll(res.Body)
if err != nil {
return nil, err
}
defer res.Body.Close()

if res.StatusCode < http.StatusBadRequest {
return body, nil
}

if len(body) > 0 {
return body, errors.NewError(string(body))
}

switch res.StatusCode {
case http.StatusMethodNotAllowed:
resp, _ := json.Marshal(map[string]string{
"code": "method_not_allowed",
"message": "method not allowed",
})
return body, errors.NewError(string(resp))
default:
return body, errors.NewError(fmt.Sprintf("could not complete the request: %d", res.StatusCode))
}
}
57 changes: 57 additions & 0 deletions internal/login/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package login_test

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/launchdarkly/ldcli/internal/login"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMakeRequest(t *testing.T) {
t.Run("with a successful response", func(t *testing.T) {
server := makeServer(t, http.StatusOK, `{"message": "success"}`)
defer server.Close()
c := login.NewClient("test-version")

response, err := c.MakeRequest("POST", server.URL, []byte(`{}`))

require.NoError(t, err)
assert.JSONEq(t, `{"message": "success"}`, string(response))
})

t.Run("with an error with a response body", func(t *testing.T) {
server := makeServer(t, http.StatusBadRequest, `{"message": "an error"}`)
defer server.Close()
c := login.NewClient("test-version")

_, err := c.MakeRequest("POST", server.URL, []byte(`{}`))

assert.EqualError(t, err, `{"message": "an error"}`)
})

t.Run("with a method not allowed status code and no body", func(t *testing.T) {
server := makeServer(t, http.StatusMethodNotAllowed, "")
defer server.Close()
c := login.NewClient("test-version")
expectedResponse := `{
"code": "method_not_allowed",
"message": "method not allowed"
}`

_, err := c.MakeRequest("POST", server.URL, []byte(`{}`))

assert.JSONEq(t, expectedResponse, err.Error())
})
}

func makeServer(t *testing.T, statusResponse int, responseBody string) *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "application/json", r.Header.Get("Content-Type"))
assert.Equal(t, "launchdarkly-cli/vtest-version", r.Header.Get("User-Agent"))
w.WriteHeader(statusResponse)
_, _ = w.Write([]byte(responseBody))
}))
}
50 changes: 0 additions & 50 deletions internal/login/login.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
package login

import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"
"os"
"time"

Expand All @@ -29,53 +26,6 @@ type DeviceAuthorizationToken struct {
AccessToken string `json:"accessToken"`
}

type UnauthenticatedClient interface {
MakeRequest(
method string,
path string,
data []byte,
) ([]byte, error)
}

type Client struct {
cliVersion string
}

func NewClient(cliVersion string) Client {
return Client{
cliVersion: cliVersion,
}
}

func (c Client) MakeRequest(
method string,
path string,
data []byte,
) ([]byte, error) {
client := http.Client{}

req, _ := http.NewRequest(method, path, bytes.NewReader(data))
req.Header.Add("Content-type", "application/json")
req.Header.Set("User-Agent", fmt.Sprintf("launchdarkly-cli/v%s", c.cliVersion))

res, err := client.Do(req)
if err != nil {
return nil, err
}

body, err := io.ReadAll(res.Body)
if err != nil {
return nil, err
}
defer res.Body.Close()

if res.StatusCode >= 400 {
return body, errors.NewError(string(body))
}

return body, nil
}

// FetchDeviceAuthorization makes a request to create a device authorization that will later be
// used to set a local access token if the user grants access.
func FetchDeviceAuthorization(
Expand Down
2 changes: 1 addition & 1 deletion internal/resources/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (c ResourcesClient) MakeRequest(accessToken, method, path, contentType stri

req, _ := http.NewRequest(method, path, bytes.NewReader(data))
req.Header.Add("Authorization", accessToken)
req.Header.Add("Content-type", contentType)
req.Header.Add("Content-Type", contentType)
req.Header.Set("User-Agent", fmt.Sprintf("launchdarkly-cli/v%s", c.cliVersion))
if isBeta {
req.Header.Set("LD-API-Version", "beta")
Expand Down

0 comments on commit 36dc28f

Please sign in to comment.