Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ) | ||
| { | ||
| // Set dictionaries | ||
| SetSignalTypes(); | ||
| SetCompanies(); | ||
| SetCompanies(); |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix “generic catch clause” issues, identify which exceptions the try block is expected to throw in normal error conditions and catch only those types; let other exceptions bubble up. If there is a requirement to be very defensive, you can still add a narrow “last resort” catch around very specific infrastructure exceptions, or rethrow unknown ones.
For this method, the risky operations are the database operations and configuration usage: constructing AdoDataConnection with ConfigSettings.Instance, creating TableOperations<Measurement>, and calling QueryRecordWhere. These are likely to throw DataException, InvalidOperationException, or potentially ConfigurationErrorsException (or ConfigurationException depending on framework) in expected error scenarios. We should:
- Replace
catch (Exception ex)with one or more specific catches (e.g.,DataException,InvalidOperationException) that log and returnnull. - Optionally add a final
catch (Exception ex)that logs and then rethrows, but that would change behavior (currently it swallows everything), so we should avoid that. - Since the existing code intentionally swallows for robustness, the least intrusive change is to catch the primary data/config exceptions explicitly and keep the logging +
nullreturn. This narrows the catch while preserving external behavior for the common failure modes.
We only need to change the catch block around lines 167–171 in GuessBaseKV inside MeasurementExpressionParser.cs. No new imports are strictly required if DataException is already available via System.Data (which is imported at line 24). We can add an additional catch (InvalidOperationException ex) without new imports as it is in System.
| @@ -164,11 +164,16 @@ | ||
| Measurement? record = measurementTable.QueryRecordWhere("SignalReference = {0}", SignalReference.ToString(deviceAcronym, SignalKind.Magnitude, 1)); | ||
| return record?.PointTag; | ||
| } | ||
| catch (Exception ex) | ||
| catch (DataException ex) | ||
| { | ||
| Logger.SwallowException(ex, $"Failed while looking up first phasor tag associated with device '{deviceAcronym}'"); | ||
| return null; | ||
| } | ||
| catch (InvalidOperationException ex) | ||
| { | ||
| Logger.SwallowException(ex, $"Failed while looking up first phasor tag associated with device '{deviceAcronym}'"); | ||
| return null; | ||
| } | ||
| }); | ||
| } | ||
|
|
2 participants
No description provided.