Skip to content

Commit 686c8e8

Browse files
vcheung-stripevcheung-stripe
authored
Don't retry on 4xx when authorizing listen and logs tail sessions (#1379)
* Don't retry on 4xx when authorizing listen and logs tail sessions * Fix tests
1 parent e3020d2 commit 686c8e8

File tree

6 files changed

+243
-3
lines changed

6 files changed

+243
-3
lines changed

‎pkg/logtailing/tailer.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package logtailing
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
9+
"net/http"
810
"os"
911
"time"
1012

@@ -209,6 +211,14 @@ func (t *Tailer) createSession(ctx context.Context) (*stripeauth.StripeCLISessio
209211
return
210212
}
211213

214+
if clientError, ok := stripeauth.IsAuthorizationClientError(err); ok {
215+
if clientError.StatusCode == http.StatusTooManyRequests {
216+
err = errors.New("You have too many `stripe logs tail` sessions open. Please close some and try again.")
217+
}
218+
exitCh <- struct{}{}
219+
return
220+
}
221+
212222
select {
213223
case <-ctx.Done():
214224
exitCh <- struct{}{}

‎pkg/logtailing/tailer_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
package logtailing
22

33
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
48
"testing"
59

610
"github.com/stretchr/testify/require"
11+
12+
"github.com/stripe/stripe-cli/pkg/stripe"
13+
"github.com/stripe/stripe-cli/pkg/websocket"
714
)
815

916
func TestJsonifyFiltersAll(t *testing.T) {
@@ -49,3 +56,72 @@ func TestJsonifyFiltersEmpty(t *testing.T) {
4956
require.NoError(t, err)
5057
require.Equal(t, "{}", filtersStr)
5158
}
59+
60+
func TestRun_RetryOnAuthorizationServerError(t *testing.T) {
61+
nAttempts := 0
62+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
63+
nAttempts++
64+
require.Equal(t, http.MethodPost, r.Method)
65+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
66+
w.WriteHeader(http.StatusInternalServerError)
67+
w.Write([]byte(`{"error":"internal_server_error"}`))
68+
}))
69+
defer ts.Close()
70+
71+
baseURL, _ := url.Parse(ts.URL)
72+
73+
cfg := Config{
74+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
75+
OutCh: make(chan websocket.IElement, 2),
76+
}
77+
tailer := New(&cfg)
78+
err := tailer.Run(context.Background())
79+
require.Error(t, err)
80+
require.Equal(t, 6, nAttempts)
81+
}
82+
83+
func TestRun_NoRetryOnAuthorizationClientError(t *testing.T) {
84+
nAttempts := 0
85+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
86+
nAttempts++
87+
require.Equal(t, http.MethodPost, r.Method)
88+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
89+
w.WriteHeader(http.StatusBadRequest)
90+
w.Write([]byte(`{"error":"bad_request"}`))
91+
}))
92+
defer ts.Close()
93+
94+
baseURL, _ := url.Parse(ts.URL)
95+
96+
cfg := Config{
97+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
98+
OutCh: make(chan websocket.IElement, 2),
99+
}
100+
tailer := New(&cfg)
101+
err := tailer.Run(context.Background())
102+
require.Error(t, err)
103+
require.Equal(t, 1, nAttempts)
104+
}
105+
106+
func TestRun_NoRetryOnAuthorizationClientError_TooManyRequests(t *testing.T) {
107+
nAttempts := 0
108+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
109+
nAttempts++
110+
require.Equal(t, http.MethodPost, r.Method)
111+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
112+
w.WriteHeader(http.StatusTooManyRequests)
113+
w.Write([]byte(`{"error":"too_many_requests"}`))
114+
}))
115+
defer ts.Close()
116+
117+
baseURL, _ := url.Parse(ts.URL)
118+
119+
cfg := Config{
120+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
121+
OutCh: make(chan websocket.IElement, 2),
122+
}
123+
tailer := New(&cfg)
124+
err := tailer.Run(context.Background())
125+
require.ErrorContains(t, err, "You have too many `stripe logs tail` sessions open. Please close some and try again.")
126+
require.Equal(t, 1, nAttempts)
127+
}

‎pkg/proxy/proxy.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,14 @@ func (p *Proxy) createSession(ctx context.Context) (*stripeauth.StripeCLISession
283283
return
284284
}
285285

286+
if clientError, ok := stripeauth.IsAuthorizationClientError(err); ok {
287+
if clientError.StatusCode == http.StatusTooManyRequests {
288+
err = errors.New("You have too many `stripe listen` sessions open. Please close some and try again.")
289+
}
290+
exitCh <- struct{}{}
291+
return
292+
}
293+
286294
select {
287295
case <-ctx.Done():
288296
exitCh <- struct{}{}

‎pkg/proxy/proxy_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@ package proxy
22

33
import (
44
"context"
5+
"net/http"
6+
"net/http/httptest"
57
"net/url"
68
"testing"
79

810
"github.com/stretchr/testify/require"
911

1012
"github.com/stripe/stripe-cli/pkg/requests"
13+
"github.com/stripe/stripe-cli/pkg/stripe"
1114
"github.com/stripe/stripe-cli/pkg/websocket"
1215
)
1316

@@ -210,3 +213,78 @@ func TestExtractRequestData(t *testing.T) {
210213
require.Error(t, err)
211214
})
212215
}
216+
217+
func TestRun_RetryOnAuthorizationServerError(t *testing.T) {
218+
nAttempts := 0
219+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
220+
nAttempts++
221+
require.Equal(t, http.MethodPost, r.Method)
222+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
223+
w.WriteHeader(http.StatusInternalServerError)
224+
w.Write([]byte(`{"error":"internal_server_error"}`))
225+
}))
226+
defer ts.Close()
227+
228+
baseURL, _ := url.Parse(ts.URL)
229+
230+
cfg := Config{
231+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
232+
OutCh: make(chan websocket.IElement, 2),
233+
}
234+
p, err := Init(context.Background(), &cfg)
235+
require.NoError(t, err)
236+
237+
err = p.Run(context.Background())
238+
require.Error(t, err)
239+
require.Equal(t, 6, nAttempts)
240+
}
241+
242+
func TestRun_NoRetryOnAuthorizationClientError(t *testing.T) {
243+
nAttempts := 0
244+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
245+
nAttempts++
246+
require.Equal(t, http.MethodPost, r.Method)
247+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
248+
w.WriteHeader(http.StatusBadRequest)
249+
w.Write([]byte(`{"error":"bad_request"}`))
250+
}))
251+
defer ts.Close()
252+
253+
baseURL, _ := url.Parse(ts.URL)
254+
255+
cfg := Config{
256+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
257+
OutCh: make(chan websocket.IElement, 2),
258+
}
259+
p, err := Init(context.Background(), &cfg)
260+
require.NoError(t, err)
261+
262+
err = p.Run(context.Background())
263+
require.Error(t, err)
264+
require.Equal(t, 1, nAttempts)
265+
}
266+
267+
func TestRun_NoRetryOnAuthorizationClientError_TooManyRequests(t *testing.T) {
268+
nAttempts := 0
269+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
270+
nAttempts++
271+
require.Equal(t, http.MethodPost, r.Method)
272+
require.Equal(t, "/v1/stripecli/sessions", r.URL.Path)
273+
w.WriteHeader(http.StatusTooManyRequests)
274+
w.Write([]byte(`{"error":"too_many_requests"}`))
275+
}))
276+
defer ts.Close()
277+
278+
baseURL, _ := url.Parse(ts.URL)
279+
280+
cfg := Config{
281+
Client: &stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL},
282+
OutCh: make(chan websocket.IElement, 2),
283+
}
284+
p, err := Init(context.Background(), &cfg)
285+
require.NoError(t, err)
286+
287+
err = p.Run(context.Background())
288+
require.ErrorContains(t, err, "You have too many `stripe listen` sessions open. Please close some and try again.")
289+
require.Equal(t, 1, nAttempts)
290+
}

‎pkg/stripeauth/client.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package stripeauth
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
89
"net/http"
@@ -15,6 +16,15 @@ import (
1516

1617
const stripeCLISessionPath = "/v1/stripecli/sessions"
1718

19+
type AuthorizeHttpError struct {
20+
StatusCode int
21+
Body string
22+
}
23+
24+
func (e *AuthorizeHttpError) Error() string {
25+
return fmt.Sprintf("Authorization failed, status=%d, body=%s", e.StatusCode, e.Body)
26+
}
27+
1828
//
1929
// Public types
2030
//
@@ -94,8 +104,10 @@ func (c *Client) Authorize(ctx context.Context, req CreateSessionRequest) (*Stri
94104
}
95105

96106
if resp.StatusCode != http.StatusOK {
97-
err := fmt.Errorf("Authorization failed, status=%d, body=%s", resp.StatusCode, body)
98-
return nil, err
107+
return nil, &AuthorizeHttpError{
108+
StatusCode: resp.StatusCode,
109+
Body: string(body),
110+
}
99111
}
100112

101113
var session *StripeCLISession
@@ -132,3 +144,11 @@ func NewClient(client stripe.RequestPerformer, cfg *Config) *Client {
132144
cfg: cfg,
133145
}
134146
}
147+
148+
func IsAuthorizationClientError(err error) (*AuthorizeHttpError, bool) {
149+
var clientError *AuthorizeHttpError
150+
if errors.As(err, &clientError) && clientError.StatusCode >= 400 && clientError.StatusCode < 500 {
151+
return clientError, true
152+
}
153+
return nil, false
154+
}

‎pkg/stripeauth/client_test.go‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func TestAuthorize(t *testing.T) {
4545
WebSocketFeatures: []string{"webhooks"},
4646
})
4747
require.NoError(t, err)
48-
require.NoError(t, err)
48+
49+
clientError, ok := IsAuthorizationClientError(err)
50+
require.False(t, ok)
51+
require.Nil(t, clientError)
52+
4953
require.Equal(t, "some-id", session.WebSocketID)
5054
require.Equal(t, "wss://example.com/subscribe/acct_123", session.WebSocketURL)
5155
require.Equal(t, "webhook-payloads", session.WebSocketAuthorizedFeature)
@@ -127,3 +131,47 @@ func TestAuthorizeWithURLDeviceMap(t *testing.T) {
127131
})
128132
require.NoError(t, err)
129133
}
134+
135+
func TestAuthorizeClientError(t *testing.T) {
136+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
137+
w.WriteHeader(http.StatusTooManyRequests)
138+
w.Write([]byte(`{"error":"too_many_requests"}`))
139+
}))
140+
defer ts.Close()
141+
142+
baseURL, _ := url.Parse(ts.URL)
143+
client := NewClient(&stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL}, nil)
144+
145+
_, err := client.Authorize(context.Background(), CreateSessionRequest{
146+
DeviceName: "my-device",
147+
WebSocketFeatures: []string{"webhooks"},
148+
})
149+
150+
require.Error(t, err)
151+
clientError, ok := IsAuthorizationClientError(err)
152+
require.True(t, ok)
153+
require.Equal(t, http.StatusTooManyRequests, clientError.StatusCode)
154+
require.Equal(t, `{"error":"too_many_requests"}`, clientError.Body)
155+
}
156+
157+
func TestAuthorizeServerError(t *testing.T) {
158+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
159+
w.WriteHeader(http.StatusInternalServerError)
160+
w.Write([]byte(`{"error":"internal_server_error"}`))
161+
}))
162+
defer ts.Close()
163+
164+
baseURL, _ := url.Parse(ts.URL)
165+
client := NewClient(&stripe.Client{APIKey: "sk_test_123", BaseURL: baseURL}, nil)
166+
167+
_, err := client.Authorize(context.Background(), CreateSessionRequest{
168+
DeviceName: "my-device",
169+
WebSocketFeatures: []string{"webhooks"},
170+
})
171+
172+
require.Error(t, err)
173+
174+
clientError, ok := IsAuthorizationClientError(err)
175+
require.False(t, ok)
176+
require.Nil(t, clientError)
177+
}

0 commit comments

Comments
 (0)