Skip to content

Commit 8493dcd

Browse files
committed
fix: coderrabit suggestions
1 parent a9c2ac4 commit 8493dcd

File tree

7 files changed

+162
-74
lines changed

7 files changed

+162
-74
lines changed

call-profile/main.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
3030
var user utils.User
3131
err = dsnap.DataTo(&user)
3232
if err != nil {
33-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "UserData Type Error: "+fmt.Sprintln(err), sessionId)
33+
utils.LogProfileSkipped(d.Client, d.Ctx, "UserData Type Error: "+fmt.Sprintln(err), userId, sessionId)
3434
return events.APIGatewayProxyResponse{
3535
Body: "Profile Skipped No User Data",
3636
StatusCode: 200,
@@ -40,7 +40,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
4040
discordId := user.DiscordID
4141

4242
if user.ProfileURL == "" {
43-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "Profile URL not available", sessionId)
43+
utils.LogProfileSkipped(d.Client, d.Ctx, "Profile URL not available", userId, sessionId)
4444
utils.SetProfileStatusBlocked(d.Client, d.Ctx, userId, "Profile URL not available", sessionId, discordId)
4545
return events.APIGatewayProxyResponse{
4646
Body: "Profile Skipped No Profile URL",
@@ -50,7 +50,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
5050

5151
_, chaincodeExists := data["chaincode"]
5252
if !chaincodeExists {
53-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "Chaincode Not Found", sessionId)
53+
utils.LogProfileSkipped(d.Client, d.Ctx, "Chaincode Not Found", userId, sessionId)
5454
utils.SetProfileStatusBlocked(d.Client, d.Ctx, userId, "Chaincode Not Found", sessionId, discordId)
5555
return events.APIGatewayProxyResponse{
5656
Body: "Profile Skipped Chaincode Not Found",
@@ -59,7 +59,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
5959
}
6060

6161
if user.Chaincode == "" {
62-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "Profile Service Blocked or Chaincode is empty", sessionId)
62+
utils.LogProfileSkipped(d.Client, d.Ctx, "Profile Service Blocked or Chaincode is empty", userId, sessionId)
6363
utils.SetProfileStatusBlocked(d.Client, d.Ctx, userId, "Profile Service Blocked or Chaincode is empty", sessionId, discordId)
6464
return events.APIGatewayProxyResponse{
6565
Body: "Profile Skipped Profile Service Blocked",
@@ -73,7 +73,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
7373
var userData utils.Diff
7474
err = dsnap.DataTo(&userData)
7575
if err != nil {
76-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "UserData Type Error: "+fmt.Sprintln(err), sessionId)
76+
utils.LogProfileSkipped(d.Client, d.Ctx, "UserData Type Error: "+fmt.Sprintln(err), userId, sessionId)
7777
return events.APIGatewayProxyResponse{
7878
Body: "Profile Skipped No User Data",
7979
StatusCode: 200,
@@ -94,7 +94,7 @@ func handler(d *utils.Deps, request events.APIGatewayProxyRequest) (events.APIGa
9494

9595
utils.LogHealth(d.Client, d.Ctx, userId, isServiceRunning, sessionId)
9696
if !isServiceRunning {
97-
utils.LogProfileSkipped(d.Client, d.Ctx, userId, "Profile Service Down", sessionId)
97+
utils.LogProfileSkipped(d.Client, d.Ctx, "Profile Service Down", userId, sessionId)
9898
utils.SetProfileStatusBlocked(d.Client, d.Ctx, userId, "Profile Service Down", sessionId, discordId)
9999
return events.APIGatewayProxyResponse{
100100
Body: "Profile Skipped Service Down",

health-check/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ func callProfileHealth(userUrl string) {
2929
requestURL := fmt.Sprintf("%shealth", userUrl)
3030
_, err1 := utils.GetWithContext(context.Background(), requestURL, 2*time.Second)
3131
if err1 != nil {
32-
logger.WarnWithError("Service not running", map[string]interface{}{
32+
logger.WarnWithError("Service not running", err1, map[string]interface{}{
3333
"function": "callProfileHealth",
3434
"profileURL": userUrl,
35-
}, err1)
35+
})
3636
}
3737
}
3838

layer/utils/errors.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var (
1919
type ProfileError struct {
2020
Code string
2121
Message string
22+
Status int
2223
Err error
2324
}
2425

@@ -33,26 +34,28 @@ func (e *ProfileError) Unwrap() error {
3334
return e.Err
3435
}
3536

36-
func NewProfileError(code, message string, err error) *ProfileError {
37+
func NewProfileError(code, message string, status int, err error) *ProfileError {
3738
return &ProfileError{
3839
Code: code,
3940
Message: message,
41+
Status: status,
4042
Err: err,
4143
}
4244
}
4345

4446
func HandleLambdaError(err error) (events.APIGatewayProxyResponse, error) {
4547
if err == nil {
46-
return events.APIGatewayProxyResponse{
47-
Body: "Internal server error",
48-
StatusCode: 500,
49-
}, nil
48+
panic("HandleLambdaError called with nil error - this indicates a programming error")
5049
}
5150

5251
if profileErr, ok := err.(*ProfileError); ok {
52+
statusCode := profileErr.Status
53+
if statusCode == 0 {
54+
statusCode = 400
55+
}
5356
return events.APIGatewayProxyResponse{
5457
Body: fmt.Sprintf("Profile Error: %s", profileErr.Message),
55-
StatusCode: 400,
58+
StatusCode: statusCode,
5659
}, nil
5760
}
5861

layer/utils/firestore.go

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,28 @@ func SetProfileStatusBlocked(client *firestore.Client, ctx context.Context, user
9292
discordURL := os.Getenv(Constants["DISCORD_BOT_URL"]) + "/profile/blocked"
9393
req, err := http.NewRequestWithContext(ctx, "POST", discordURL, responseBody)
9494
if err != nil {
95-
LogWarnWithError("Failed to create Discord bot request", map[string]interface{}{
95+
LogWarnWithError("Failed to create Discord bot request", err, map[string]interface{}{
9696
"function": "SetProfileStatusBlocked",
9797
"userId": userId,
98-
}, err)
98+
})
9999
} else {
100100
req.Header.Add("Content-Type", "application/json")
101101
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", tokenString))
102102

103-
httpClient := CreateHTTPClient(10 * time.Second)
104-
_, err = DoRequestWithContext(ctx, httpClient, req)
103+
// Create context with timeout and use client without timeout
104+
reqCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
105+
defer cancel()
106+
req = req.WithContext(reqCtx)
107+
httpClient := &http.Client{} // No timeout - rely on context
108+
resp, err := httpClient.Do(req)
109+
if resp != nil && resp.Body != nil {
110+
resp.Body.Close()
111+
}
105112
if err != nil {
106-
LogWarnWithError("Failed to notify Discord bot", map[string]interface{}{
113+
LogWarnWithError("Failed to notify Discord bot", err, map[string]interface{}{
107114
"function": "SetProfileStatusBlocked",
108115
"userId": userId,
109-
}, err)
116+
})
110117
}
111118
}
112119
}
@@ -134,73 +141,76 @@ func Getdata(client *firestore.Client, ctx context.Context, userId string, userU
134141
hashedChaincode, err := bcrypt.GenerateFromPassword([]byte(chaincode), bcrypt.DefaultCost)
135142
if err != nil {
136143
errMsg := fmt.Sprintf("chaincode encryption failed: %v", err)
137-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
144+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
138145
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
139146
return fmt.Errorf("chaincode not encrypted: %w", err)
140147
}
141148

142-
httpClient := CreateHTTPClient(30 * time.Second)
143-
req, err := http.NewRequestWithContext(ctx, "GET", userUrl, nil)
149+
// Create context with timeout and use client without timeout
150+
reqCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
151+
defer cancel()
152+
req, err := http.NewRequestWithContext(reqCtx, "GET", userUrl, nil)
144153
if err != nil {
145154
errMsg := fmt.Sprintf("failed to create request: %v", err)
146-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
155+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
147156
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
148157
return fmt.Errorf("error creating request: %w", err)
149158
}
150159
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", string(hashedChaincode)))
151-
resp, err := DoRequestWithContext(ctx, httpClient, req)
160+
httpClient := &http.Client{} // No timeout - rely on context
161+
resp, err := httpClient.Do(req)
152162
if err != nil {
153163
errMsg := fmt.Sprintf("failed to get profile data: %v", err)
154-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
164+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
155165
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
156166
return fmt.Errorf("error getting profile data: %w", err)
157167
}
158168
defer resp.Body.Close()
159169

160170
if resp.StatusCode == 401 {
161171
errMsg := "Unauthenticated Access to Profile Data"
162-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
172+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
163173
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
164-
return NewProfileError("UNAUTHENTICATED", errMsg, nil)
174+
return NewProfileError("UNAUTHENTICATED", errMsg, 401, nil)
165175
}
166176
if resp.StatusCode != 200 {
167177
errMsg := "Error in getting Profile Data"
168-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
178+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
169179
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
170180
return fmt.Errorf("error in getting profile data: status code %d", resp.StatusCode)
171181
}
172182

173183
r, err := io.ReadAll(resp.Body)
174184
if err != nil {
175185
errMsg := fmt.Sprintf("failed to read response: %v", err)
176-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
186+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
177187
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
178188
return fmt.Errorf("error reading profile data: %w", err)
179189
}
180190
var res Res
181191
err = json.Unmarshal([]byte(r), &res)
182192
if err != nil {
183193
errMsg := fmt.Sprintf("failed to unmarshal JSON: %v", err)
184-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
194+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
185195
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
186196
return fmt.Errorf("error converting data to json: %w", err)
187197
}
188198

189199
err = res.Validate()
190200
if err != nil {
191201
errMsg := fmt.Sprintf("validation failed: %v", err)
192-
LogProfileSkipped(client, ctx, userId, errMsg, sessionId)
202+
LogProfileSkipped(client, ctx, errMsg, userId, sessionId)
193203
SetProfileStatusBlocked(client, ctx, userId, errMsg, sessionId, discordId)
194204
return fmt.Errorf("error in validation: %w", err)
195205
}
196206

197207
lastPendingDiff, lastPendingDiffId, err := getLastDiff(client, ctx, userId, "PENDING")
198208
if err != nil {
199209
// Log error but continue processing
200-
LogWarnWithError("Failed to get last pending diff", map[string]interface{}{
210+
LogWarnWithError("Failed to get last pending diff", err, map[string]interface{}{
201211
"function": "Getdata",
202212
"userId": userId,
203-
}, err)
213+
})
204214
}
205215

206216
if lastPendingDiff != res && userData != res {
@@ -209,30 +219,30 @@ func Getdata(client *firestore.Client, ctx context.Context, userId string, userU
209219
}
210220
lastRejectedDiff, lastRejectedDiffId, err := getLastDiff(client, ctx, userId, Constants["NOT_APPROVED"])
211221
if err != nil {
212-
LogWarnWithError("Failed to get last rejected diff", map[string]interface{}{
222+
LogWarnWithError("Failed to get last rejected diff", err, map[string]interface{}{
213223
"function": "Getdata",
214224
"userId": userId,
215-
}, err)
225+
})
216226
}
217227
if lastRejectedDiff != res {
218228
err = generateAndStoreDiff(client, ctx, res, userId, sessionId)
219229
if err != nil {
220230
return fmt.Errorf("failed to generate and store diff: %w", err)
221231
}
222232
} else {
223-
LogProfileSkipped(client, ctx, userId, "Last Rejected Diff is same as New Profile Data. Rejected Diff Id: "+lastRejectedDiffId, sessionId)
233+
LogProfileSkipped(client, ctx, "Last Rejected Diff is same as New Profile Data. Rejected Diff Id: "+lastRejectedDiffId, userId, sessionId)
224234
// This is not an error, just a skip reason
225235
return nil
226236
}
227237
} else if userData == res {
228-
LogProfileSkipped(client, ctx, userId, "Current User Data is same as New Profile Data", sessionId)
238+
LogProfileSkipped(client, ctx, "Current User Data is same as New Profile Data", userId, sessionId)
229239
if lastPendingDiffId != "" {
230240
SetNotApproved(client, ctx, lastPendingDiffId)
231241
}
232242
// This is not an error, just a skip reason
233243
return nil
234244
} else {
235-
LogProfileSkipped(client, ctx, userId, "Last Pending Diff is same as New Profile Data", sessionId)
245+
LogProfileSkipped(client, ctx, "Last Pending Diff is same as New Profile Data", userId, sessionId)
236246
// This is not an error, just a skip reason
237247
return nil
238248
}

layer/utils/handler.go

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

33
import (
44
"context"
5+
"time"
56

67
"cloud.google.com/go/firestore"
78

@@ -17,23 +18,32 @@ type Deps struct {
1718
type HandlerFunc func(*Deps, events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error)
1819

1920
func InitializeLambdaWithFirestore(functionName string, handlerFunc HandlerFunc) {
20-
ctx := context.Background()
21+
initCtx := context.Background()
2122
logger := GetLogger()
2223

23-
client, err := InitializeFirestoreClient(ctx)
24+
client, err := InitializeFirestoreClient(initCtx)
2425
if err != nil {
2526
logger.Error("Failed to initialize Firestore client", err, map[string]interface{}{
2627
"function": functionName,
2728
})
29+
lambda.Start(func(request events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) {
30+
return events.APIGatewayProxyResponse{
31+
StatusCode: 500,
32+
Body: "Service initialization failed",
33+
}, nil
34+
})
2835
return
2936
}
3037

31-
deps := &Deps{
32-
Client: client,
33-
Ctx: ctx,
34-
}
35-
3638
wrappedHandler := func(request events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) {
39+
reqCtx, cancel := context.WithTimeout(context.Background(), 55*time.Second)
40+
defer cancel()
41+
42+
deps := &Deps{
43+
Client: client,
44+
Ctx: reqCtx,
45+
}
46+
3747
return handlerFunc(deps, request)
3848
}
3949

0 commit comments

Comments
 (0)