Skip to content

Commit 9750285

Browse files
olamilekan000olamilekan000
authored
fix returning of internal errors to client (#2381)
change fixes returning of system errors to the client side.
1 parent 28d217c commit 9750285

File tree

18 files changed

+227
-132
lines changed

18 files changed

+227
-132
lines changed

‎api/handlers/auth.go‎

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,20 @@
11
package handlers
22

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

8+
"github.com/go-chi/render"
9+
10+
"github.com/frain-dev/convoy/api/models"
711
"github.com/frain-dev/convoy/auth/realm/jwt"
812
"github.com/frain-dev/convoy/config"
9-
"github.com/frain-dev/convoy/datastore"
10-
1113
"github.com/frain-dev/convoy/database/postgres"
12-
"github.com/frain-dev/convoy/services"
13-
14-
"encoding/json"
15-
"github.com/frain-dev/convoy/api/models"
14+
"github.com/frain-dev/convoy/datastore"
1615
"github.com/frain-dev/convoy/internal/pkg/middleware"
16+
"github.com/frain-dev/convoy/services"
1717
"github.com/frain-dev/convoy/util"
18-
"github.com/go-chi/render"
1918
)
2019

2120
type (
@@ -44,7 +43,8 @@ func (h *Handler) InitSSO(w http.ResponseWriter, r *http.Request) {
4443

4544
resp, err := lu.Run()
4645
if err != nil {
47-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
46+
h.A.Logger.WithError(err).Errorf("SSO initialization failed: %v", err)
47+
_ = render.Render(w, r, util.NewErrorResponse("Authentication failed", http.StatusForbidden))
4848
return
4949
}
5050

@@ -60,7 +60,6 @@ func (h *Handler) RedeemRegisterSSOToken(w http.ResponseWriter, r *http.Request)
6060
}
6161

6262
func (h *Handler) redeemSSOToken(w http.ResponseWriter, r *http.Request, intent SSOAuthIntent) {
63-
6463
configuration := h.A.Cfg
6564

6665
lu := services.LoginUserSSOService{
@@ -75,7 +74,8 @@ func (h *Handler) redeemSSOToken(w http.ResponseWriter, r *http.Request, intent
7574

7675
tokenResp, err := lu.RedeemToken(r.URL.Query())
7776
if err != nil {
78-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
77+
h.A.Logger.WithError(err).Errorf("SSO token redemption failed: %v", err)
78+
_ = render.Render(w, r, util.NewErrorResponse("Authentication failed", http.StatusForbidden))
7979
return
8080
}
8181

@@ -85,21 +85,26 @@ func (h *Handler) redeemSSOToken(w http.ResponseWriter, r *http.Request, intent
8585
user, token, err = lu.RegisterSSOUser(r.Context(), h.A, tokenResp)
8686
if err != nil {
8787
if errors.Is(err, services.ErrUserAlreadyExist) {
88-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusConflict))
88+
h.A.Logger.WithError(err).Errorf("SSO user registration failed - user already exists: %v", err)
89+
_ = render.Render(w, r, util.NewErrorResponse("User already exists", http.StatusConflict))
8990
return
9091
}
91-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
92+
93+
h.A.Logger.WithError(err).Errorf("SSO user registration failed: %v", err)
94+
_ = render.Render(w, r, util.NewErrorResponse("Registration failed", http.StatusForbidden))
9295
return
9396
}
9497

9598
} else {
9699
user, token, err = lu.LoginSSOUser(r.Context(), tokenResp)
97100
if err != nil {
98101
if errors.Is(err, datastore.ErrUserNotFound) {
99-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusNotFound))
102+
h.A.Logger.WithError(err).Errorf("SSO login failed - user not found: %v", err)
103+
_ = render.Render(w, r, util.NewErrorResponse("Invalid credentials", http.StatusNotFound))
100104
return
101105
}
102-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
106+
h.A.Logger.WithError(err).Errorf("SSO login failed: %v", err)
107+
_ = render.Render(w, r, util.NewErrorResponse("Authentication failed", http.StatusForbidden))
103108
return
104109
}
105110
}
@@ -115,13 +120,15 @@ func (h *Handler) redeemSSOToken(w http.ResponseWriter, r *http.Request, intent
115120
func (h *Handler) LoginUser(w http.ResponseWriter, r *http.Request) {
116121
var newUser models.LoginUser
117122
if err := util.ReadJSON(r, &newUser); err != nil {
118-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
123+
h.A.Logger.WithError(err).Errorf("Failed to parse login request body: %v", err)
124+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
119125
return
120126
}
121127

122128
configuration, err := config.Get()
123129
if err != nil {
124-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
130+
h.A.Logger.Errorf("Failed to get configuration: %v", err)
131+
_ = render.Render(w, r, util.NewErrorResponse("Service temporarily unavailable", http.StatusInternalServerError))
125132
return
126133
}
127134

@@ -136,7 +143,21 @@ func (h *Handler) LoginUser(w http.ResponseWriter, r *http.Request) {
136143

137144
user, token, err := lu.Run(r.Context())
138145
if err != nil {
139-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
146+
h.A.Logger.WithError(err).Errorf("User login failed: %v", err)
147+
148+
var errMsg string
149+
150+
if se, ok := err.(*services.ServiceError); ok {
151+
switch se.Code {
152+
case services.ErrCodeLicenseExpired:
153+
errMsg = se.ErrMsg
154+
default:
155+
errMsg = "Invalid credentials"
156+
}
157+
}
158+
159+
_ = render.Render(w, r, util.NewErrorResponse(errMsg, http.StatusForbidden))
160+
140161
return
141162
}
142163

@@ -151,18 +172,21 @@ func (h *Handler) LoginUser(w http.ResponseWriter, r *http.Request) {
151172
func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) {
152173
var refreshToken models.Token
153174
if err := util.ReadJSON(r, &refreshToken); err != nil {
154-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
175+
h.A.Logger.WithError(err).Errorf("Failed to parse refresh token request: %v", err)
176+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
155177
return
156178
}
157179

158180
if err := refreshToken.Validate(); err != nil {
159-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
181+
h.A.Logger.WithError(err).Errorf("Refresh token validation failed: %v", err)
182+
_ = render.Render(w, r, util.NewErrorResponse("Invalid token", http.StatusBadRequest))
160183
return
161184
}
162185

163186
configuration, err := config.Get()
164187
if err != nil {
165-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
188+
h.A.Logger.Errorf("Failed to get configuration: %v", err)
189+
_ = render.Render(w, r, util.NewErrorResponse("Service temporarily unavailable", http.StatusInternalServerError))
166190
return
167191
}
168192

@@ -174,7 +198,8 @@ func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) {
174198

175199
token, err := rf.Run(r.Context())
176200
if err != nil {
177-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusUnauthorized))
201+
h.A.Logger.WithError(err).Errorf("Token refresh failed: %v", err)
202+
_ = render.Render(w, r, util.NewErrorResponse("Invalid or expired token", http.StatusUnauthorized))
178203
return
179204
}
180205

@@ -184,13 +209,15 @@ func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) {
184209
func (h *Handler) LogoutUser(w http.ResponseWriter, r *http.Request) {
185210
auth, err := middleware.GetAuthFromRequest(r)
186211
if err != nil {
187-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusUnauthorized))
212+
h.A.Logger.WithError(err).Errorf("Failed to get auth from request: %v", err)
213+
_ = render.Render(w, r, util.NewErrorResponse("Authentication required", http.StatusUnauthorized))
188214
return
189215
}
190216

191217
configuration, err := config.Get()
192218
if err != nil {
193-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
219+
h.A.Logger.Errorf("Failed to get configuration: %v", err)
220+
_ = render.Render(w, r, util.NewErrorResponse("Service temporarily unavailable", http.StatusInternalServerError))
194221
return
195222
}
196223

@@ -243,7 +270,8 @@ func (h *Handler) GoogleOAuthToken(w http.ResponseWriter, r *http.Request) {
243270

244271
user, token, err := googleOAuthService.HandleIDToken(r.Context(), request.IDToken, h.A)
245272
if err != nil {
246-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusForbidden))
273+
h.A.Logger.WithError(err).Errorf("Google OAuth authentication failed: %v", err)
274+
_ = render.Render(w, r, util.NewErrorResponse("Authentication failed", http.StatusForbidden))
247275
return
248276
}
249277

@@ -300,7 +328,8 @@ func (h *Handler) GoogleOAuthSetup(w http.ResponseWriter, r *http.Request) {
300328

301329
user, token, err := googleOAuthService.CompleteGoogleOAuthSetup(r.Context(), request.IDToken, request.BusinessName, h.A)
302330
if err != nil {
303-
_ = render.Render(w, r, util.NewErrorResponse("Failed to complete setup: "+err.Error(), http.StatusInternalServerError))
331+
h.A.Logger.Errorf("Google OAuth setup failed: %v", err)
332+
_ = render.Render(w, r, util.NewErrorResponse("Failed to complete setup", http.StatusInternalServerError))
304333
return
305334
}
306335

‎api/handlers/configuration.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,18 @@ package handlers
22

33
import (
44
"errors"
5-
"github.com/frain-dev/convoy/config"
6-
"github.com/frain-dev/convoy/pkg/log"
75
"net/http"
86

7+
"github.com/go-chi/render"
8+
99
"github.com/frain-dev/convoy"
1010
"github.com/frain-dev/convoy/api/models"
11+
"github.com/frain-dev/convoy/config"
1112
"github.com/frain-dev/convoy/database/postgres"
1213
"github.com/frain-dev/convoy/datastore"
14+
"github.com/frain-dev/convoy/pkg/log"
1315
"github.com/frain-dev/convoy/services"
1416
"github.com/frain-dev/convoy/util"
15-
"github.com/go-chi/render"
1617
)
1718

1819
func (h *Handler) GetConfiguration(w http.ResponseWriter, r *http.Request) {
@@ -46,12 +47,14 @@ func (h *Handler) GetConfiguration(w http.ResponseWriter, r *http.Request) {
4647
func (h *Handler) CreateConfiguration(w http.ResponseWriter, r *http.Request) {
4748
var newConfig models.Configuration
4849
if err := util.ReadJSON(r, &newConfig); err != nil {
49-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
50+
h.A.Logger.WithError(err).Errorf("Failed to parse configuration request: %v", err)
51+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
5052
return
5153
}
5254

5355
if err := newConfig.Validate(); err != nil {
54-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
56+
h.A.Logger.WithError(err).Errorf("Configuration validation failed: %v", err)
57+
_ = render.Render(w, r, util.NewErrorResponse("Invalid configuration provided", http.StatusBadRequest))
5558
return
5659
}
5760

@@ -77,12 +80,14 @@ func (h *Handler) CreateConfiguration(w http.ResponseWriter, r *http.Request) {
7780
func (h *Handler) UpdateConfiguration(w http.ResponseWriter, r *http.Request) {
7881
var newConfig models.Configuration
7982
if err := util.ReadJSON(r, &newConfig); err != nil {
80-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
83+
h.A.Logger.WithError(err).Errorf("Failed to parse configuration update request: %v", err)
84+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
8185
return
8286
}
8387

8488
if err := newConfig.Validate(); err != nil {
85-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
89+
h.A.Logger.WithError(err).Errorf("Configuration update validation failed: %v", err)
90+
_ = render.Render(w, r, util.NewErrorResponse("Invalid configuration provided", http.StatusBadRequest))
8691
return
8792
}
8893

‎api/handlers/endpoint.go‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,19 @@ import (
77
"net/http"
88
"time"
99

10-
"github.com/frain-dev/convoy/internal/pkg/fflag"
11-
12-
"github.com/frain-dev/convoy/pkg/circuit_breaker"
13-
"github.com/frain-dev/convoy/pkg/msgpack"
10+
"github.com/go-chi/chi/v5"
11+
"github.com/go-chi/render"
1412

1513
"github.com/frain-dev/convoy/api/models"
1614
"github.com/frain-dev/convoy/database/postgres"
1715
"github.com/frain-dev/convoy/datastore"
16+
"github.com/frain-dev/convoy/internal/pkg/fflag"
1817
"github.com/frain-dev/convoy/internal/pkg/middleware"
18+
"github.com/frain-dev/convoy/pkg/circuit_breaker"
1919
"github.com/frain-dev/convoy/pkg/log"
20+
"github.com/frain-dev/convoy/pkg/msgpack"
2021
"github.com/frain-dev/convoy/services"
2122
"github.com/frain-dev/convoy/util"
22-
"github.com/go-chi/chi/v5"
23-
"github.com/go-chi/render"
2423
)
2524

2625
// CreateEndpoint
@@ -42,27 +41,31 @@ func (h *Handler) CreateEndpoint(w http.ResponseWriter, r *http.Request) {
4241

4342
err := h.RM.VersionRequest(r, "CreateEndpoint")
4443
if err != nil {
45-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
44+
h.A.Logger.WithError(err).Errorf("Version request failed for CreateEndpoint: %v", err)
45+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request", http.StatusBadRequest))
4646
return
4747
}
4848

4949
var e models.CreateEndpoint
5050

5151
err = util.ReadJSON(r, &e)
5252
if err != nil {
53-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
53+
h.A.Logger.WithError(err).Errorf("Failed to parse endpoint creation request: %v", err)
54+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
5455
return
5556
}
5657

5758
err = e.Validate()
5859
if err != nil {
59-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
60+
h.A.Logger.WithError(err).Errorf("Endpoint creation validation failed: %v", err)
61+
_ = render.Render(w, r, util.NewErrorResponse("Invalid input provided", http.StatusBadRequest))
6062
return
6163
}
6264

6365
project, err := h.retrieveProject(r)
6466
if err != nil {
65-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
67+
h.A.Logger.WithError(err).Errorf("Failed to retrieve project: %v", err)
68+
_ = render.Render(w, r, util.NewErrorResponse("Project not found", http.StatusBadRequest))
6669
return
6770
}
6871

@@ -130,14 +133,16 @@ func (h *Handler) CreateEndpoint(w http.ResponseWriter, r *http.Request) {
130133
func (h *Handler) GetEndpoint(w http.ResponseWriter, r *http.Request) {
131134
project, err := h.retrieveProject(r)
132135
if err != nil {
133-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
136+
h.A.Logger.WithError(err).Errorf("Failed to retrieve project: %v", err)
137+
_ = render.Render(w, r, util.NewErrorResponse("Project not found", http.StatusBadRequest))
134138
return
135139
}
136140

137141
endpointID := chi.URLParam(r, "endpointID")
138142
endpoint, err := h.retrieveEndpoint(r.Context(), endpointID, project.UID)
139143
if err != nil {
140-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusNotFound))
144+
h.A.Logger.WithError(err).Errorf("Failed to retrieve endpoint: %v", err)
145+
_ = render.Render(w, r, util.NewErrorResponse("Resource not found", http.StatusNotFound))
141146
return
142147
}
143148

@@ -177,7 +182,8 @@ func (h *Handler) GetEndpoint(w http.ResponseWriter, r *http.Request) {
177182
func (h *Handler) GetEndpoints(w http.ResponseWriter, r *http.Request) {
178183
project, err := h.retrieveProject(r)
179184
if err != nil {
180-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
185+
h.A.Logger.WithError(err).Errorf("Failed to retrieve project: %v", err)
186+
_ = render.Render(w, r, util.NewErrorResponse("Project not found", http.StatusBadRequest))
181187
return
182188
}
183189

@@ -210,7 +216,7 @@ func (h *Handler) GetEndpoints(w http.ResponseWriter, r *http.Request) {
210216
endpoints, paginationData, err := postgres.NewEndpointRepo(h.A.DB).LoadEndpointsPaged(r.Context(), project.UID, data.Filter, data.Pageable)
211217
if err != nil {
212218
h.A.Logger.WithError(err).Error("failed to load endpoints")
213-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
219+
_ = render.Render(w, r, util.NewErrorResponse("Failed to load endpoints", http.StatusBadRequest))
214220
return
215221
}
216222

‎api/handlers/organisation.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,15 @@ package handlers
33
import (
44
"net/http"
55

6-
"github.com/frain-dev/convoy/pkg/log"
6+
"github.com/go-chi/render"
77

88
"github.com/frain-dev/convoy/api/models"
99
"github.com/frain-dev/convoy/api/policies"
1010
"github.com/frain-dev/convoy/database/postgres"
11+
m "github.com/frain-dev/convoy/internal/pkg/middleware"
12+
"github.com/frain-dev/convoy/pkg/log"
1113
"github.com/frain-dev/convoy/services"
1214
"github.com/frain-dev/convoy/util"
13-
"github.com/go-chi/render"
14-
15-
m "github.com/frain-dev/convoy/internal/pkg/middleware"
1615
)
1716

1817
func (h *Handler) GetOrganisation(w http.ResponseWriter, r *http.Request) {
@@ -53,7 +52,8 @@ func (h *Handler) CreateOrganisation(w http.ResponseWriter, r *http.Request) {
5352
var newOrg models.Organisation
5453
err := util.ReadJSON(r, &newOrg)
5554
if err != nil {
56-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
55+
h.A.Logger.WithError(err).Errorf("Failed to parse organisation creation request: %v", err)
56+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
5757
return
5858
}
5959

@@ -89,7 +89,8 @@ func (h *Handler) UpdateOrganisation(w http.ResponseWriter, r *http.Request) {
8989
var orgUpdate models.Organisation
9090
err := util.ReadJSON(r, &orgUpdate)
9191
if err != nil {
92-
_ = render.Render(w, r, util.NewErrorResponse(err.Error(), http.StatusBadRequest))
92+
h.A.Logger.WithError(err).Errorf("Failed to parse organisation update request: %v", err)
93+
_ = render.Render(w, r, util.NewErrorResponse("Invalid request format", http.StatusBadRequest))
9394
return
9495
}
9596

0 commit comments

Comments
 (0)