Skip to content

Commit adadbcc

Browse files
sreulandsreuland
authored
exp/lighthorizon/actions: use standard Problem model on API error responses (#4542)
1 parent 7a30aef commit adadbcc

File tree

13 files changed

+421
-70
lines changed

13 files changed

+421
-70
lines changed

‎exp/lighthorizon/actions/accounts.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package actions
22

33
import (
4+
"errors"
45
"net/http"
56
"strconv"
67

@@ -11,26 +12,25 @@ import (
1112
hProtocol "github.com/stellar/go/protocols/horizon"
1213
"github.com/stellar/go/protocols/horizon/operations"
1314
"github.com/stellar/go/support/render/hal"
15+
supportProblem "github.com/stellar/go/support/render/problem"
1416
"github.com/stellar/go/toid"
1517
)
1618

1719
const (
1820
urlAccountId = "account_id"
1921
)
2022

21-
func accountRequestParams(w http.ResponseWriter, r *http.Request) (string, pagination) {
23+
func accountRequestParams(w http.ResponseWriter, r *http.Request) (string, pagination, error) {
2224
var accountId string
2325
var accountErr bool
2426

2527
if accountId, accountErr = getURLParam(r, urlAccountId); !accountErr {
26-
sendErrorResponse(w, http.StatusBadRequest, "")
27-
return "", pagination{}
28+
return "", pagination{}, errors.New("unable to find account_id in url path")
2829
}
2930

3031
paginate, err := paging(r)
3132
if err != nil {
32-
sendErrorResponse(w, http.StatusBadRequest, string(invalidPagingParameters))
33-
return "", pagination{}
33+
return "", pagination{}, err
3434
}
3535

3636
if paginate.Cursor < 1 {
@@ -41,16 +41,19 @@ func accountRequestParams(w http.ResponseWriter, r *http.Request) (string, pagin
4141
paginate.Limit = 10
4242
}
4343

44-
return accountId, paginate
44+
return accountId, paginate, nil
4545
}
4646

4747
func NewTXByAccountHandler(lightHorizon services.LightHorizon) func(http.ResponseWriter, *http.Request) {
4848
return func(w http.ResponseWriter, r *http.Request) {
4949
ctx := r.Context()
5050
var accountId string
5151
var paginate pagination
52+
var err error
5253

53-
if accountId, paginate = accountRequestParams(w, r); accountId == "" {
54+
if accountId, paginate, err = accountRequestParams(w, r); err != nil {
55+
errorMsg := supportProblem.MakeInvalidFieldProblem("account_id", err)
56+
sendErrorResponse(r.Context(), w, *errorMsg)
5457
return
5558
}
5659

@@ -65,7 +68,7 @@ func NewTXByAccountHandler(lightHorizon services.LightHorizon) func(http.Respons
6568
txns, err := lightHorizon.Transactions.GetTransactionsByAccount(ctx, paginate.Cursor, paginate.Limit, accountId)
6669
if err != nil {
6770
log.Error(err)
68-
sendErrorResponse(w, http.StatusInternalServerError, err.Error())
71+
sendErrorResponse(r.Context(), w, supportProblem.ServerError)
6972
return
7073
}
7174

@@ -74,15 +77,15 @@ func NewTXByAccountHandler(lightHorizon services.LightHorizon) func(http.Respons
7477
response, err = adapters.PopulateTransaction(r, &txn)
7578
if err != nil {
7679
log.Error(err)
77-
sendErrorResponse(w, http.StatusInternalServerError, err.Error())
80+
sendErrorResponse(r.Context(), w, supportProblem.ServerError)
7881
return
7982
}
8083

8184
page.Add(response)
8285
}
8386

8487
page.PopulateLinks()
85-
sendPageResponse(w, page)
88+
sendPageResponse(r.Context(), w, page)
8689
}
8790
}
8891

@@ -91,8 +94,11 @@ func NewOpsByAccountHandler(lightHorizon services.LightHorizon) func(http.Respon
9194
ctx := r.Context()
9295
var accountId string
9396
var paginate pagination
97+
var err error
9498

95-
if accountId, paginate = accountRequestParams(w, r); accountId == "" {
99+
if accountId, paginate, err = accountRequestParams(w, r); err != nil {
100+
errorMsg := supportProblem.MakeInvalidFieldProblem("account_id", err)
101+
sendErrorResponse(r.Context(), w, *errorMsg)
96102
return
97103
}
98104

@@ -107,7 +113,7 @@ func NewOpsByAccountHandler(lightHorizon services.LightHorizon) func(http.Respon
107113
ops, err := lightHorizon.Operations.GetOperationsByAccount(ctx, paginate.Cursor, paginate.Limit, accountId)
108114
if err != nil {
109115
log.Error(err)
110-
sendErrorResponse(w, http.StatusInternalServerError, err.Error())
116+
sendErrorResponse(r.Context(), w, supportProblem.ServerError)
111117
return
112118
}
113119

@@ -116,14 +122,14 @@ func NewOpsByAccountHandler(lightHorizon services.LightHorizon) func(http.Respon
116122
response, err = adapters.PopulateOperation(r, &op)
117123
if err != nil {
118124
log.Error(err)
119-
sendErrorResponse(w, http.StatusInternalServerError, err.Error())
125+
sendErrorResponse(r.Context(), w, supportProblem.ServerError)
120126
return
121127
}
122128

123129
page.Add(response)
124130
}
125131

126132
page.PopulateLinks()
127-
sendPageResponse(w, page)
133+
sendPageResponse(r.Context(), w, page)
128134
}
129135
}

‎exp/lighthorizon/actions/accounts_test.go

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
package actions
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"errors"
7+
"io/ioutil"
8+
"net/http"
9+
"net/http/httptest"
10+
"net/url"
11+
"testing"
12+
13+
"github.com/go-chi/chi"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/mock"
16+
"github.com/stretchr/testify/require"
17+
18+
"github.com/stellar/go/exp/lighthorizon/common"
19+
"github.com/stellar/go/exp/lighthorizon/services"
20+
"github.com/stellar/go/support/render/problem"
21+
)
22+
23+
func setupTest() {
24+
problem.RegisterHost("")
25+
}
26+
27+
func TestTxByAccountMissingParamError(t *testing.T) {
28+
setupTest()
29+
recorder := httptest.NewRecorder()
30+
request := buildHttpRequest(
31+
t,
32+
map[string]string{},
33+
map[string]string{},
34+
)
35+
36+
mockOperationService := &services.MockOperationService{}
37+
mockTransactionService := &services.MockTransactionService{}
38+
39+
lh := services.LightHorizon{
40+
Operations: mockOperationService,
41+
Transactions: mockTransactionService,
42+
}
43+
44+
handler := NewTXByAccountHandler(lh)
45+
handler(recorder, request)
46+
47+
resp := recorder.Result()
48+
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
49+
50+
raw, err := ioutil.ReadAll(resp.Body)
51+
assert.NoError(t, err)
52+
53+
var problem problem.P
54+
err = json.Unmarshal(raw, &problem)
55+
assert.NoError(t, err)
56+
assert.Equal(t, "Bad Request", problem.Title)
57+
assert.Equal(t, "bad_request", problem.Type)
58+
assert.Equal(t, "account_id", problem.Extras["invalid_field"])
59+
assert.Equal(t, "The request you sent was invalid in some way.", problem.Detail)
60+
assert.Equal(t, "unable to find account_id in url path", problem.Extras["reason"])
61+
}
62+
63+
func TestTxByAccountServerError(t *testing.T) {
64+
setupTest()
65+
recorder := httptest.NewRecorder()
66+
pathParams := make(map[string]string)
67+
pathParams["account_id"] = "G1234"
68+
request := buildHttpRequest(
69+
t,
70+
map[string]string{},
71+
pathParams,
72+
)
73+
74+
mockOperationService := &services.MockOperationService{}
75+
mockTransactionService := &services.MockTransactionService{}
76+
mockTransactionService.On("GetTransactionsByAccount", mock.Anything, mock.Anything, mock.Anything, "G1234").Return([]common.Transaction{}, errors.New("not good"))
77+
78+
lh := services.LightHorizon{
79+
Operations: mockOperationService,
80+
Transactions: mockTransactionService,
81+
}
82+
83+
handler := NewTXByAccountHandler(lh)
84+
handler(recorder, request)
85+
86+
resp := recorder.Result()
87+
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
88+
89+
raw, err := ioutil.ReadAll(resp.Body)
90+
assert.NoError(t, err)
91+
92+
var problem problem.P
93+
err = json.Unmarshal(raw, &problem)
94+
assert.NoError(t, err)
95+
assert.Equal(t, "Internal Server Error", problem.Title)
96+
assert.Equal(t, "server_error", problem.Type)
97+
}
98+
99+
func TestOpsByAccountMissingParamError(t *testing.T) {
100+
setupTest()
101+
recorder := httptest.NewRecorder()
102+
request := buildHttpRequest(
103+
t,
104+
map[string]string{},
105+
map[string]string{},
106+
)
107+
108+
mockOperationService := &services.MockOperationService{}
109+
mockTransactionService := &services.MockTransactionService{}
110+
111+
lh := services.LightHorizon{
112+
Operations: mockOperationService,
113+
Transactions: mockTransactionService,
114+
}
115+
116+
handler := NewOpsByAccountHandler(lh)
117+
handler(recorder, request)
118+
119+
resp := recorder.Result()
120+
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
121+
122+
raw, err := ioutil.ReadAll(resp.Body)
123+
assert.NoError(t, err)
124+
125+
var problem problem.P
126+
err = json.Unmarshal(raw, &problem)
127+
assert.NoError(t, err)
128+
assert.Equal(t, "Bad Request", problem.Title)
129+
assert.Equal(t, "bad_request", problem.Type)
130+
assert.Equal(t, "account_id", problem.Extras["invalid_field"])
131+
assert.Equal(t, "The request you sent was invalid in some way.", problem.Detail)
132+
assert.Equal(t, "unable to find account_id in url path", problem.Extras["reason"])
133+
}
134+
135+
func TestOpsByAccountServerError(t *testing.T) {
136+
setupTest()
137+
recorder := httptest.NewRecorder()
138+
pathParams := make(map[string]string)
139+
pathParams["account_id"] = "G1234"
140+
request := buildHttpRequest(
141+
t,
142+
map[string]string{},
143+
pathParams,
144+
)
145+
146+
mockOperationService := &services.MockOperationService{}
147+
mockTransactionService := &services.MockTransactionService{}
148+
mockOperationService.On("GetOperationsByAccount", mock.Anything, mock.Anything, mock.Anything, "G1234").Return([]common.Operation{}, errors.New("not good"))
149+
150+
lh := services.LightHorizon{
151+
Operations: mockOperationService,
152+
Transactions: mockTransactionService,
153+
}
154+
155+
handler := NewOpsByAccountHandler(lh)
156+
handler(recorder, request)
157+
158+
resp := recorder.Result()
159+
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
160+
161+
raw, err := ioutil.ReadAll(resp.Body)
162+
assert.NoError(t, err)
163+
164+
var problem problem.P
165+
err = json.Unmarshal(raw, &problem)
166+
assert.NoError(t, err)
167+
assert.Equal(t, "Internal Server Error", problem.Title)
168+
assert.Equal(t, "server_error", problem.Type)
169+
}
170+
171+
func buildHttpRequest(
172+
t *testing.T,
173+
queryParams map[string]string,
174+
routeParams map[string]string,
175+
) *http.Request {
176+
request, err := http.NewRequest("GET", "/", nil)
177+
require.NoError(t, err)
178+
179+
query := url.Values{}
180+
for key, value := range queryParams {
181+
query.Set(key, value)
182+
}
183+
request.URL.RawQuery = query.Encode()
184+
185+
chiRouteContext := chi.NewRouteContext()
186+
for key, value := range routeParams {
187+
chiRouteContext.URLParams.Add(key, value)
188+
}
189+
ctx := context.WithValue(context.Background(), chi.RouteCtxKey, chiRouteContext)
190+
return request.WithContext(ctx)
191+
}

‎exp/lighthorizon/actions/apidocs.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package actions
22

33
import (
4+
supportProblem "github.com/stellar/go/support/render/problem"
45
"net/http"
56
)
67

@@ -10,7 +11,7 @@ func ApiDocs() func(http.ResponseWriter, *http.Request) {
1011
r.URL.Host = "localhost:8080"
1112

1213
if r.Method != "GET" {
13-
sendErrorResponse(w, http.StatusMethodNotAllowed, "")
14+
sendErrorResponse(r.Context(), w, supportProblem.BadRequest)
1415
return
1516
}
1617

0 commit comments

Comments
 (0)