Skip to content

Commit 0ab7e7a

Browse files
MakMufticMakMuftic
committed
code cleanup and add unit tests for retry function
1 parent c25455a commit 0ab7e7a

File tree

2 files changed

+128
-25
lines changed

2 files changed

+128
-25
lines changed

‎protocol/lighter/api.go‎

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ type LighterError struct {
5050
Message string `json:"message"`
5151
}
5252

53-
func (e *LighterError) Error() error {
54-
return fmt.Errorf("lighter error: code %d, message: %s", e.Code, e.Message)
53+
func (e *LighterError) Error() string {
54+
return fmt.Sprintf("lighter error: code %d, message: %s", e.Code, e.Message)
5555
}
5656

5757
func (tx *LighterTx) UnmarshalJSON(data []byte) error {
@@ -60,18 +60,14 @@ func (tx *LighterTx) UnmarshalJSON(data []byte) error {
6060
return err
6161
}
6262

63-
if tx.Code != 200 {
64-
return fmt.Errorf("lighter error: code %d, message: %s", tx.Code, tx.Info)
65-
}
66-
6763
if tx.Type == TxTypeL2Transfer {
6864
var t *Transfer
6965
if err := json.Unmarshal([]byte(tx.Info), &t); err != nil {
7066
return fmt.Errorf("failed to unmarshal transfer: %w", err)
7167
}
7268
tx.Transfer = t
7369
} else {
74-
return fmt.Errorf("invalid transaction type: %d", tx.Type)
70+
return fmt.Errorf("unsupported transaction type: %d", tx.Type)
7571
}
7672

7773
return nil
@@ -86,17 +82,17 @@ func NewLighterAPI() *LighterAPI {
8682
retryClient.RetryMax = TX_NOT_FOUND_RETRIES - 1 // RetryMax is a number of retries after an initial attempt
8783
retryClient.RetryWaitMin = TX_NOT_FOUND_RETRY_WAIT
8884
retryClient.RetryWaitMax = TX_NOT_FOUND_RETRY_WAIT
89-
retryClient.CheckRetry = lighterCheckRetry
85+
retryClient.CheckRetry = LighterCheckRetry
9086
retryClient.Logger = log.Logger
9187

9288
return &LighterAPI{
9389
HTTPClient: retryClient.StandardClient(),
9490
}
9591
}
9692

97-
// lighterCheckRetry checks if we should retry based on Lighter API error codes
98-
func lighterCheckRetry(ctx context.Context, resp *http.Response, err error) (bool, error) {
99-
// Don't retry on context cancellation or client errors
93+
// LighterCheckRetry checks if we should retry the request.
94+
// Retries when: error code is TX_NOT_FOUND_ERROR_CODE (21500) or response has code 200 but missing/empty info.
95+
func LighterCheckRetry(ctx context.Context, resp *http.Response, err error) (bool, error) {
10096
if ctx.Err() != nil {
10197
return false, ctx.Err()
10298
}
@@ -105,7 +101,6 @@ func lighterCheckRetry(ctx context.Context, resp *http.Response, err error) (boo
105101
return false, err
106102
}
107103

108-
// Only retry on 200 OK with transaction not found error
109104
if resp.StatusCode == http.StatusOK {
110105
body, err := io.ReadAll(resp.Body)
111106
if err != nil {
@@ -114,14 +109,22 @@ func lighterCheckRetry(ctx context.Context, resp *http.Response, err error) (boo
114109
resp.Body.Close()
115110
resp.Body = io.NopCloser(bytes.NewReader(body))
116111

117-
s := new(LighterTx)
118-
if err := json.Unmarshal(body, s); err != nil {
119-
e := new(LighterError)
120-
if err := json.Unmarshal(body, e); err != nil {
121-
return false, fmt.Errorf("failed to unmarshal response body: %s, with error: %w", string(body), err)
122-
}
123-
if e.Code == TX_NOT_FOUND_ERROR_CODE || e.Code == TX_FOUND_STATUS_CODE {
124-
return true, nil
112+
// First try to unmarshal as LighterError
113+
e := new(LighterError)
114+
if err := json.Unmarshal(body, e); err == nil && e.Code == TX_NOT_FOUND_ERROR_CODE {
115+
// Retry on TX_NOT_FOUND_ERROR_CODE
116+
return true, nil
117+
}
118+
119+
// Check if it's a LighterTx with code 200 but missing info (before custom UnmarshalJSON runs)
120+
var raw map[string]interface{}
121+
if err := json.Unmarshal(body, &raw); err == nil {
122+
if code, ok := raw["code"].(float64); ok && code == TX_FOUND_STATUS_CODE {
123+
info, hasInfo := raw["info"].(string)
124+
// Retry if info is missing or empty
125+
if !hasInfo || info == "" {
126+
return true, nil
127+
}
125128
}
126129
}
127130
}
@@ -149,11 +152,7 @@ func (a *LighterAPI) GetTx(hash string) (*LighterTx, error) {
149152

150153
s := new(LighterTx)
151154
if err := json.Unmarshal(body, s); err != nil {
152-
e := new(LighterError)
153-
if err := json.Unmarshal(body, e); err != nil {
154-
return nil, fmt.Errorf("failed to unmarshal response body: %s, with error: %w", string(body), err)
155-
}
156-
return nil, e.Error()
155+
return nil, err
157156
}
158157

159158
return s, nil

‎protocol/lighter/api_test.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package lighter_test
33

44
import (
55
"bytes"
6+
"context"
67
"encoding/json"
78
"errors"
89
"fmt"
@@ -113,3 +114,106 @@ func Test_LighterAPI_GetTx(t *testing.T) {
113114
})
114115
}
115116
}
117+
118+
func Test_LighterCheckRetry(t *testing.T) {
119+
tests := []struct {
120+
name string
121+
statusCode int
122+
responseBody string
123+
inputError error
124+
ctxCanceled bool
125+
wantRetry bool
126+
wantErr bool
127+
}{
128+
{
129+
name: "retry on TX_NOT_FOUND_ERROR_CODE",
130+
statusCode: http.StatusOK,
131+
responseBody: `{"code": 21500, "message": "transaction not found"}`,
132+
wantRetry: true,
133+
wantErr: false,
134+
},
135+
{
136+
name: "retry on code 200 with missing info",
137+
statusCode: http.StatusOK,
138+
responseBody: `{"code": 200, "hash": "0xabc", "type": 12}`,
139+
wantRetry: true,
140+
wantErr: false,
141+
},
142+
{
143+
name: "retry on code 200 with empty info",
144+
statusCode: http.StatusOK,
145+
responseBody: `{"code": 200, "hash": "0xabc", "type": 12, "info": ""}`,
146+
wantRetry: true,
147+
wantErr: false,
148+
},
149+
{
150+
name: "no retry on code 200 with valid info",
151+
statusCode: http.StatusOK,
152+
responseBody: `{"code": 200, "hash": "0xabc", "type": 12, "info": "{\"USDCAmount\": 1000}"}`,
153+
wantRetry: false,
154+
wantErr: false,
155+
},
156+
{
157+
name: "no retry on different error code",
158+
statusCode: http.StatusOK,
159+
responseBody: `{"code": 500, "message": "internal server error"}`,
160+
wantRetry: false,
161+
wantErr: false,
162+
},
163+
{
164+
name: "no retry on non-200 status",
165+
statusCode: http.StatusNotFound,
166+
responseBody: `{"error": "not found"}`,
167+
wantRetry: false,
168+
wantErr: false,
169+
},
170+
{
171+
name: "no retry on input error",
172+
inputError: errors.New("connection error"),
173+
wantRetry: false,
174+
wantErr: true,
175+
},
176+
{
177+
name: "error on context cancellation",
178+
ctxCanceled: true,
179+
wantRetry: false,
180+
wantErr: true,
181+
},
182+
}
183+
184+
for _, tc := range tests {
185+
t.Run(tc.name, func(t *testing.T) {
186+
ctx := context.Background()
187+
if tc.ctxCanceled {
188+
var cancel context.CancelFunc
189+
ctx, cancel = context.WithCancel(ctx)
190+
cancel()
191+
}
192+
193+
var resp *http.Response
194+
if tc.statusCode != 0 {
195+
resp = &http.Response{
196+
StatusCode: tc.statusCode,
197+
Body: io.NopCloser(bytes.NewReader([]byte(tc.responseBody))),
198+
Header: make(http.Header),
199+
}
200+
}
201+
202+
shouldRetry, err := lighter.LighterCheckRetry(ctx, resp, tc.inputError)
203+
204+
if tc.wantErr {
205+
if err == nil {
206+
t.Errorf("expected error, got nil")
207+
}
208+
} else {
209+
if err != nil {
210+
t.Errorf("unexpected error: %v", err)
211+
}
212+
}
213+
214+
if shouldRetry != tc.wantRetry {
215+
t.Errorf("expected retry=%v, got retry=%v", tc.wantRetry, shouldRetry)
216+
}
217+
})
218+
}
219+
}

0 commit comments

Comments
 (0)