Skip to content

Commit 784f502

Browse files
fix(security): replace 4 generic catches in LeaseServiceImpl
Added NpgsqlException, TimeoutException, InvalidOperationException, OperationCanceledException handlers Refs: CodeQL cs/catch-of-all-exceptions
1 parent 4091a41 commit 784f502

File tree

3 files changed

+120
-4
lines changed

3 files changed

+120
-4
lines changed

AzuriteConfig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"instaceID":"726a9a34-c43f-4a21-b608-56ba35ff4c72"}

__azurite_db_table__.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"filename":"e:\\Scalable-Process-Agent-System\\__azurite_db_table__.json","collections":[{"name":"$TABLES_COLLECTION$","data":[],"idIndex":null,"binaryIndices":{"account":{"name":"account","dirty":false,"values":[]},"table":{"name":"table","dirty":false,"values":[]}},"constraints":null,"uniqueNames":[],"transforms":{},"objType":"$TABLES_COLLECTION$","dirty":false,"cachedIndex":null,"cachedBinaryIndex":null,"cachedData":null,"adaptiveBinaryIndices":true,"transactional":false,"cloneObjects":false,"cloneMethod":"parse-stringify","asyncListeners":false,"disableMeta":false,"disableChangesApi":true,"disableDeltaChangesApi":true,"autoupdate":false,"serializableIndices":true,"disableFreeze":true,"ttl":null,"maxId":0,"DynamicViews":[],"events":{"insert":[],"update":[],"pre-insert":[],"pre-update":[],"close":[],"flushbuffer":[],"error":[],"delete":[null],"warning":[null]},"changes":[],"dirtyIds":[]},{"name":"$SERVICES_COLLECTION$","data":[],"idIndex":null,"binaryIndices":{},"constraints":null,"uniqueNames":["accountName"],"transforms":{},"objType":"$SERVICES_COLLECTION$","dirty":false,"cachedIndex":null,"cachedBinaryIndex":null,"cachedData":null,"adaptiveBinaryIndices":true,"transactional":false,"cloneObjects":false,"cloneMethod":"parse-stringify","asyncListeners":false,"disableMeta":false,"disableChangesApi":true,"disableDeltaChangesApi":true,"autoupdate":false,"serializableIndices":true,"disableFreeze":true,"ttl":null,"maxId":0,"DynamicViews":[],"events":{"insert":[],"update":[],"pre-insert":[],"pre-update":[],"close":[],"flushbuffer":[],"error":[],"delete":[null],"warning":[null]},"changes":[],"dirtyIds":[]}],"databaseVersion":1.5,"engineVersion":1.5,"autosave":true,"autosaveInterval":5000,"autosaveHandle":null,"throttledSaves":true,"options":{"persistenceMethod":"fs","autosave":true,"autosaveInterval":5000,"serializationMethod":"normal","destructureDelimiter":"$<\n"},"persistenceMethod":"fs","persistenceAdapter":null,"verbose":false,"events":{"init":[null],"loaded":[],"flushChanges":[],"close":[],"changes":[],"warning":[]},"ENV":"NODEJS"}

src/ControlPlane.Api/Grpc/LeaseServiceImpl.cs

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,42 @@ public override async Task Pull(PullRequest request, IServerStreamWriter<Lease>
6060
activity?.SetStatus(ActivityStatusCode.Error);
6161
throw;
6262
}
63+
catch (OperationCanceledException)
64+
{
65+
activity?.SetStatus(ActivityStatusCode.Error);
66+
_logger.LogInformation("Pull request cancelled for node {NodeId}", request.NodeId);
67+
throw new RpcException(new Status(StatusCode.Cancelled, "Request was cancelled"));
68+
}
69+
catch (Npgsql.NpgsqlException npgEx)
70+
{
71+
activity?.SetStatus(ActivityStatusCode.Error);
72+
activity?.SetTag("error.type", "NpgsqlException");
73+
activity?.SetTag("error.message", npgEx.Message);
74+
_logger.LogError(npgEx, "Database error processing Pull request for node {NodeId}", request.NodeId);
75+
throw new RpcException(new Status(StatusCode.Unavailable, "Database temporarily unavailable"));
76+
}
77+
catch (TimeoutException timeoutEx)
78+
{
79+
activity?.SetStatus(ActivityStatusCode.Error);
80+
activity?.SetTag("error.type", "TimeoutException");
81+
activity?.SetTag("error.message", timeoutEx.Message);
82+
_logger.LogError(timeoutEx, "Timeout processing Pull request for node {NodeId}", request.NodeId);
83+
throw new RpcException(new Status(StatusCode.DeadlineExceeded, "Request timeout"));
84+
}
85+
catch (InvalidOperationException invalidOpEx)
86+
{
87+
activity?.SetStatus(ActivityStatusCode.Error);
88+
activity?.SetTag("error.type", "InvalidOperationException");
89+
activity?.SetTag("error.message", invalidOpEx.Message);
90+
_logger.LogError(invalidOpEx, "Invalid operation processing Pull request for node {NodeId}", request.NodeId);
91+
throw new RpcException(new Status(StatusCode.FailedPrecondition, invalidOpEx.Message));
92+
}
6393
catch (Exception ex)
6494
{
6595
activity?.SetStatus(ActivityStatusCode.Error);
6696
activity?.SetTag("error.type", ex.GetType().Name);
6797
activity?.SetTag("error.message", ex.Message);
68-
_logger.LogError(ex, "Error processing Pull request for node {NodeId}", request.NodeId);
98+
_logger.LogError(ex, "Unexpected error processing Pull request for node {NodeId}", request.NodeId);
6999
throw new RpcException(new Status(StatusCode.Internal, "Internal server error"));
70100
}
71101
}
@@ -106,9 +136,36 @@ public override async Task<AckResponse> Ack(AckRequest request, ServerCallContex
106136
Message = success ? "Lease acknowledged" : "Failed to acknowledge lease"
107137
};
108138
}
139+
catch (Npgsql.NpgsqlException npgEx)
140+
{
141+
_logger.LogError(npgEx, "Database error processing Ack request for lease {LeaseId}", request.LeaseId);
142+
return new AckResponse
143+
{
144+
Success = false,
145+
Message = "Database error processing acknowledgement"
146+
};
147+
}
148+
catch (TimeoutException timeoutEx)
149+
{
150+
_logger.LogWarning(timeoutEx, "Timeout processing Ack request for lease {LeaseId}", request.LeaseId);
151+
return new AckResponse
152+
{
153+
Success = false,
154+
Message = "Timeout processing acknowledgement"
155+
};
156+
}
157+
catch (InvalidOperationException invalidOpEx)
158+
{
159+
_logger.LogError(invalidOpEx, "Invalid operation processing Ack request for lease {LeaseId}", request.LeaseId);
160+
return new AckResponse
161+
{
162+
Success = false,
163+
Message = $"Invalid operation: {invalidOpEx.Message}"
164+
};
165+
}
109166
catch (Exception ex)
110167
{
111-
_logger.LogError(ex, "Error processing Ack request for lease {LeaseId}", request.LeaseId);
168+
_logger.LogError(ex, "Unexpected error processing Ack request for lease {LeaseId}", request.LeaseId);
112169
return new AckResponse
113170
{
114171
Success = false,
@@ -165,9 +222,36 @@ public override async Task<CompleteResponse> Complete(CompleteRequest request, S
165222
Message = success ? "Run completed successfully" : "Failed to complete run"
166223
};
167224
}
225+
catch (Npgsql.NpgsqlException npgEx)
226+
{
227+
_logger.LogError(npgEx, "Database error processing Complete request for run {RunId}", request.RunId);
228+
return new CompleteResponse
229+
{
230+
Success = false,
231+
Message = "Database error processing completion"
232+
};
233+
}
234+
catch (TimeoutException timeoutEx)
235+
{
236+
_logger.LogWarning(timeoutEx, "Timeout processing Complete request for run {RunId}", request.RunId);
237+
return new CompleteResponse
238+
{
239+
Success = false,
240+
Message = "Timeout processing completion"
241+
};
242+
}
243+
catch (InvalidOperationException invalidOpEx)
244+
{
245+
_logger.LogError(invalidOpEx, "Invalid operation processing Complete request for run {RunId}", request.RunId);
246+
return new CompleteResponse
247+
{
248+
Success = false,
249+
Message = $"Invalid operation: {invalidOpEx.Message}"
250+
};
251+
}
168252
catch (Exception ex)
169253
{
170-
_logger.LogError(ex, "Error processing Complete request for run {RunId}", request.RunId);
254+
_logger.LogError(ex, "Unexpected error processing Complete request for run {RunId}", request.RunId);
171255
return new CompleteResponse
172256
{
173257
Success = false,
@@ -229,9 +313,39 @@ public override async Task<FailResponse> Fail(FailRequest request, ServerCallCon
229313
ShouldRetry = shouldRetry
230314
};
231315
}
316+
catch (Npgsql.NpgsqlException npgEx)
317+
{
318+
_logger.LogError(npgEx, "Database error processing Fail request for run {RunId}", request.RunId);
319+
return new FailResponse
320+
{
321+
Success = false,
322+
Message = "Database error processing failure",
323+
ShouldRetry = true // Retry on database errors
324+
};
325+
}
326+
catch (TimeoutException timeoutEx)
327+
{
328+
_logger.LogWarning(timeoutEx, "Timeout processing Fail request for run {RunId}", request.RunId);
329+
return new FailResponse
330+
{
331+
Success = false,
332+
Message = "Timeout processing failure",
333+
ShouldRetry = true // Retry on timeout
334+
};
335+
}
336+
catch (InvalidOperationException invalidOpEx)
337+
{
338+
_logger.LogError(invalidOpEx, "Invalid operation processing Fail request for run {RunId}", request.RunId);
339+
return new FailResponse
340+
{
341+
Success = false,
342+
Message = $"Invalid operation: {invalidOpEx.Message}",
343+
ShouldRetry = false // Don't retry on invalid operations
344+
};
345+
}
232346
catch (Exception ex)
233347
{
234-
_logger.LogError(ex, "Error processing Fail request for run {RunId}", request.RunId);
348+
_logger.LogError(ex, "Unexpected error processing Fail request for run {RunId}", request.RunId);
235349
return new FailResponse
236350
{
237351
Success = false,

0 commit comments

Comments
 (0)