ErrHandAct in HTML report #4041
Conversation
WalkthroughThe pull request introduces enhancements to error handling and reporting mechanisms in the Ginger execution engine. Key changes include the addition of the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs (2)
2080-2122: Clarify after-effect on the BusinessFlow.The ExecuteErrorActivities method always calls SetBusinessFlowActivitiesAndActionsSkipStatus() after executing error activities. This might unintentionally change statuses for the entire BusinessFlow, especially if an error activity is only meant to handle a subset of errors. Verify that partial or incremental error-handling doesn't get overshadowed by the blanket skip logic.
You could consider making the skip logic more selective. For instance, remove or skip only the specific error actions or activities that need to be isolated, rather than unconditionally calling SetBusinessFlowActivitiesAndActionsSkipStatus().
4559-4560: Reevaluate second call to error handling post-BusinessFlow.Invoking “ExecuteErrorActivities()” right after the BusinessFlow final checks (line 4559), followed by ExecuteCleanUpActivities(), can lead to unexpected state changes if error handlers modify statuses or skip logic. Ensure this is the intended moment to re-run error handlers, as the BusinessFlow is formally concluding at this stage.
If the desire is to capture leftover failures or short-circuit the next steps, consider explicitly logging or clarifying in comments why tasks are repeated after the flow’s main logic ends.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs(4 hunks)Ginger/GingerCoreNET/Run/RunListenerLib/LiteDBRepository.cs(1 hunks)
🔇 Additional comments (1)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs (1)
3798-3798: Consider interpreting empty Activities differently.
Currently, an Activity with zero actions is treated similarly to an ErrorHandler or is skipped (line 3798). If an empty Activity is actually valid (e.g. a container for external scripts), you might want to mark it as Passed instead of skipping it. Double-check whether the intended behavior is to skip all empty Activities by default.
✅ Verification successful
Based on the code analysis, I can now provide a final response:
Empty Activities are intentionally skipped as part of the execution flow
Looking at the execution engine's code, empty activities (activity.Acts.Count == 0) are deliberately skipped along with other specific cases like error handlers and activities with dependency constraints. This is part of the core execution logic and appears to be the intended behavior, as empty activities don't contribute to the test flow execution.
Key evidence:
- The condition
activity.Acts.Count == 0is grouped with other valid skip conditions in the execution engine - Test cases and execution logs show that skipped status is the expected outcome for activities that meet skip criteria
- There's no indication in the codebase that empty activities should be treated as successful executions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching where an empty Activity is used with empty Acts
rg --context 3 $'Activities: \\[\\]'
Length of output: 35
Script:
#!/bin/bash
# Search for patterns related to empty Activities handling
rg -A 3 "activity\.Acts\.Count == 0"
Length of output: 807
Script:
#!/bin/bash
# Search for Activity class definition and related code
ast-grep --pattern 'class Activity {
$$$
}'
Length of output: 5981
Script:
#!/bin/bash
# Search for Activity status setting patterns
rg -A 3 "activity\.Status = eActivitiesGenStatus\."
Length of output: 53
Script:
#!/bin/bash
# Search for Activity status enum and its usage
rg -A 5 "eActivitiesGenStatus"
Length of output: 30
Script:
#!/bin/bash
# Search for patterns where activity status is set
rg -A 3 "SetActivityStatus|activity\.Status\s*="
Length of output: 3600
Script:
#!/bin/bash
# Look for activity execution related code
rg -A 5 "RunActivity|ExecuteActivity"
Length of output: 18325
| // removing any error handler activity items from the list, this is done for adding ErrAcr in Report, No Impact on others | ||
| liteDbActionList.RemoveAll(k => activity.Acts.Any(x => x.Guid != k.GUID)); |
There was a problem hiding this comment.
💡 Codebase verification
Based on the evidence gathered, I can now provide the final response:
The removal logic is indeed incorrect and needs to be fixed
The current implementation liteDbActionList.RemoveAll(k => activity.Acts.Any(x => x.Guid != k.GUID)) will remove an item if there exists ANY Act with a different GUID, which is the opposite of what's intended. To remove only error handler items (those not in the current Activity's Acts), the logic should be:
liteDbActionList.RemoveAll(k => !activity.Acts.Any(x => x.Guid == k.GUID));This corrected version removes items that don't have a matching GUID in the Acts collection, which aligns with the comment's intention of "removing any error handler activity items from the list".
🔗 Analysis chain
Potentially incorrect removal logic.
This condition removes an item from liteDbActionList if it finds any Act in activity.Acts whose Guid differs from k.GUID, which may be broader than intended. If the intent is to remove only actions that do not exist in the current Activity (i.e., removing just the Error Handler items), consider inverting the check to remove items that are definitely not part of the current Activity’s Act collection.
Below is a script to search for all RemoveAll calls on liteDbActionList, to confirm that no other usage conflicts with the removal logic:
Let me gather more context about the Acts and GUID properties to better understand the data structures involved.
Let me search for the actual usage of Acts collection to understand its relationship with error handlers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg --context 5 "liteDbActionList\.RemoveAll"
Length of output: 2316
Script:
#!/bin/bash
# Search for the Activity class definition and its Acts property
ast-grep --pattern 'class Activity {
$$$
Acts $$$
$$$
}'
# Search for any GUID property definitions or usages in the context
rg --context 3 "GUID" Ginger/GingerCoreNET/Run/RunListenerLib/LiteDBRepository.cs
Length of output: 8049
Script:
#!/bin/bash
# Search for error handler related code and Acts usage
rg -B 3 -A 3 "error.*handler|Acts\." Ginger/GingerCoreNET/Run/RunListenerLib/LiteDBRepository.cs
# Look for the definition of the Act class or interface
ast-grep --pattern 'class Act {
$$$
}'
Length of output: 7811
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs (2)
104-105: Improve list declarations with documentation and naming.The error handler tracking lists would benefit from:
- XML documentation explaining their purpose
- More descriptive names
- Readonly modifier since they are only modified internally
Apply this diff to improve the declarations:
-public List<Act> errActivityActionList = []; -public List<ErrorHandler> errActivityList = []; +/// <summary> +/// Tracks actions executed within error handlers for reporting +/// </summary> +public readonly List<Act> ExecutedErrorHandlerActions = []; + +/// <summary> +/// Tracks error handler activities that have been executed +/// </summary> +public readonly List<ErrorHandler> ExecutedErrorHandlers = [];
2197-2197: Add validation when tracking error handlers.The error handler tracking could be improved with validation checks before adding items to the lists.
Apply this diff to add validation:
-errActivityActionList.Add(act); +if (!errActivityActionList.Any(x => x.Guid == act.Guid)) +{ + errActivityActionList.Add(act); +} -errActivityList.Add(errActivity); +if (!errActivityList.Any(x => x.Guid == errActivity.Guid)) +{ + errActivityList.Add(errActivity); +}Also applies to: 2207-2207
Ginger/GingerCore/Actions/ActSikuli.cs (3)
611-611: Remove unnecessary empty line in catch block.The empty line between the catch declaration and the error logging statement can be removed for better code consistency.
catch (Exception ex) { - Reporter.ToLog(eLogLevel.ERROR, $"In Sikuli Action : {ex.Message}"); }
612-612: Enhance error message with more context.The error message could be more descriptive by including the calculated process name value that caused the exception.
- Reporter.ToLog(eLogLevel.ERROR, $"In Sikuli Action : {ex.Message}"); + Reporter.ToLog(eLogLevel.ERROR, $"Error in Sikuli Action while setting process name for '{calculateValue}': {ex.Message}");
596-613: Add XML documentation for better code maintainability.Consider adding XML documentation to describe the method's purpose, behavior, and potential exceptions.
+ /// <summary> + /// Sets the ProcessNameForSikuliOperation based on the calculated value from input parameters. + /// If a matching process is found in ActiveProcessWindows, sets to the first match. + /// If no match is found, sets to empty string. + /// </summary> + /// <exception cref="Exception">Thrown when there's an error calculating or matching the process name.</exception> private void SetProcessAsPerVE()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Ginger/GingerCore/Actions/ActSikuli.cs(1 hunks)Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs(7 hunks)Ginger/GingerCoreNET/Run/RunListenerLib/LiteDBRepository.cs(0 hunks)
💤 Files with no reviewable changes (1)
- Ginger/GingerCoreNET/Run/RunListenerLib/LiteDBRepository.cs
🔇 Additional comments (1)
Ginger/GingerCore/Actions/ActSikuli.cs (1)
596-613: LGTM! Error handling improvement looks good.
The addition of try-catch block improves the robustness of the code by:
- Properly handling exceptions during process name calculation and matching
- Logging errors for better debugging
- Preventing unhandled exceptions from propagating
| private void AddErrorHandlerActivityInReport() | ||
| { | ||
| try | ||
| { | ||
| ObservableList<ErrorHandler> errorActivities = new ObservableList<ErrorHandler>(CurrentBusinessFlow.Activities.Where(a => a.GetType() == typeof(ErrorHandler) && a.Status != eRunStatus.Skipped && a.Active == true | ||
| ).Cast<ErrorHandler>().ToList()); | ||
|
|
||
| if (errorActivities.Count > 0) | ||
| { | ||
| foreach (ErrorHandler errActivity in errorActivities) | ||
| { | ||
| var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid); | ||
|
|
||
| if (currentActivity != null) | ||
| { | ||
| handlerPostExecutionAction = errActivity.ErrorHandlerPostExecutionAction; | ||
| CurrentBusinessFlow.CurrentActivity = errActivity; | ||
| SetCurrentActivityAgent(); | ||
| Stopwatch stE = new Stopwatch(); | ||
| stE.Start(); | ||
| NotifyActivityStart(currentActivity); | ||
| foreach (Act act in errActivity.Acts) | ||
| { | ||
| var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid); | ||
| if (currentAction != null) | ||
| { | ||
| Stopwatch st = new Stopwatch(); | ||
| st.Start(); | ||
| if (currentAction.Active) | ||
| { | ||
| CurrentBusinessFlow.CurrentActivity.Acts.CurrentItem = currentAction; | ||
| if (currentActivity.HandlerType == eHandlerType.Popup_Handler) | ||
| { | ||
| currentAction.Timeout = 1; | ||
| } | ||
|
|
||
| NotifyActionStart(currentAction); | ||
| ProcessStoretoValue(currentAction); | ||
| UpdateDSReturnValues(currentAction); | ||
| CalculateActionFinalStatus(currentAction); | ||
| st.Stop(); | ||
| NotifyActionEnd(currentAction); | ||
| } | ||
| } | ||
| } | ||
| SetBusinessFlowActivitiesAndActionsSkipStatus(); | ||
| CalculateActivityFinalStatus(currentActivity); | ||
| stE.Stop(); | ||
| NotifyActivityEnd(currentActivity); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
|
|
||
| Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity : {ex.Message}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Method needs refactoring for better maintainability.
The method handles error handler activity reporting but could benefit from the following improvements:
- Split the long method into smaller, focused methods
- Add XML documentation for better code understanding
- Enhance error message with more context
Consider refactoring into smaller methods:
+/// <summary>
+/// Adds error handler activities to the execution report
+/// </summary>
private void AddErrorHandlerActivityInReport()
{
try
{
- ObservableList<ErrorHandler> errorActivities = new ObservableList<ErrorHandler>(CurrentBusinessFlow.Activities.Where(a => a.GetType() == typeof(ErrorHandler) && a.Status != eRunStatus.Skipped && a.Active == true
- ).Cast<ErrorHandler>().ToList());
+ ObservableList<ErrorHandler> errorActivities = GetActiveErrorHandlers();
if (errorActivities.Count > 0)
{
- foreach (ErrorHandler errActivity in errorActivities)
- {
- var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid);
- if (currentActivity != null)
- {
- handlerPostExecutionAction = errActivity.ErrorHandlerPostExecutionAction;
- CurrentBusinessFlow.CurrentActivity = errActivity;
- SetCurrentActivityAgent();
- Stopwatch stE = new Stopwatch();
- stE.Start();
- NotifyActivityStart(currentActivity);
- foreach (Act act in errActivity.Acts)
- {
- var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid);
- if (currentAction != null)
- {
- ProcessErrorHandlerAction(currentAction, currentActivity);
- }
- }
- SetBusinessFlowActivitiesAndActionsSkipStatus();
- CalculateActivityFinalStatus(currentActivity);
- stE.Stop();
- NotifyActivityEnd(currentActivity);
- }
- }
+ ProcessErrorHandlerActivities(errorActivities);
}
}
catch (Exception ex)
{
- Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity : {ex.Message}");
+ Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity to report. Details: {ex.Message}", ex);
}
}
+private ObservableList<ErrorHandler> GetActiveErrorHandlers()
+{
+ return new ObservableList<ErrorHandler>(
+ CurrentBusinessFlow.Activities
+ .Where(a => a.GetType() == typeof(ErrorHandler) &&
+ a.Status != eRunStatus.Skipped &&
+ a.Active == true)
+ .Cast<ErrorHandler>()
+ .ToList());
+}
+private void ProcessErrorHandlerActivities(ObservableList<ErrorHandler> errorActivities)
+{
+ foreach (ErrorHandler errActivity in errorActivities)
+ {
+ var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid);
+ if (currentActivity != null)
+ {
+ ProcessSingleErrorHandler(errActivity, currentActivity);
+ }
+ }
+}
+private void ProcessSingleErrorHandler(ErrorHandler errActivity, ErrorHandler currentActivity)
+{
+ handlerPostExecutionAction = errActivity.ErrorHandlerPostExecutionAction;
+ CurrentBusinessFlow.CurrentActivity = errActivity;
+ SetCurrentActivityAgent();
+
+ Stopwatch stE = new Stopwatch();
+ stE.Start();
+ NotifyActivityStart(currentActivity);
+
+ ProcessErrorHandlerActions(errActivity, currentActivity);
+
+ SetBusinessFlowActivitiesAndActionsSkipStatus();
+ CalculateActivityFinalStatus(currentActivity);
+ stE.Stop();
+ NotifyActivityEnd(currentActivity);
+}
+private void ProcessErrorHandlerActions(ErrorHandler errActivity, ErrorHandler currentActivity)
+{
+ foreach (Act act in errActivity.Acts)
+ {
+ var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid);
+ if (currentAction != null)
+ {
+ ProcessErrorHandlerAction(currentAction, currentActivity);
+ }
+ }
+}
+private void ProcessErrorHandlerAction(Act currentAction, ErrorHandler currentActivity)
+{
+ Stopwatch st = new Stopwatch();
+ st.Start();
+ if (currentAction.Active)
+ {
+ CurrentBusinessFlow.CurrentActivity.Acts.CurrentItem = currentAction;
+ if (currentActivity.HandlerType == eHandlerType.Popup_Handler)
+ {
+ currentAction.Timeout = 1;
+ }
+
+ NotifyActionStart(currentAction);
+ ProcessStoretoValue(currentAction);
+ UpdateDSReturnValues(currentAction);
+ CalculateActionFinalStatus(currentAction);
+ st.Stop();
+ NotifyActionEnd(currentAction);
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void AddErrorHandlerActivityInReport() | |
| { | |
| try | |
| { | |
| ObservableList<ErrorHandler> errorActivities = new ObservableList<ErrorHandler>(CurrentBusinessFlow.Activities.Where(a => a.GetType() == typeof(ErrorHandler) && a.Status != eRunStatus.Skipped && a.Active == true | |
| ).Cast<ErrorHandler>().ToList()); | |
| if (errorActivities.Count > 0) | |
| { | |
| foreach (ErrorHandler errActivity in errorActivities) | |
| { | |
| var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid); | |
| if (currentActivity != null) | |
| { | |
| handlerPostExecutionAction = errActivity.ErrorHandlerPostExecutionAction; | |
| CurrentBusinessFlow.CurrentActivity = errActivity; | |
| SetCurrentActivityAgent(); | |
| Stopwatch stE = new Stopwatch(); | |
| stE.Start(); | |
| NotifyActivityStart(currentActivity); | |
| foreach (Act act in errActivity.Acts) | |
| { | |
| var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid); | |
| if (currentAction != null) | |
| { | |
| Stopwatch st = new Stopwatch(); | |
| st.Start(); | |
| if (currentAction.Active) | |
| { | |
| CurrentBusinessFlow.CurrentActivity.Acts.CurrentItem = currentAction; | |
| if (currentActivity.HandlerType == eHandlerType.Popup_Handler) | |
| { | |
| currentAction.Timeout = 1; | |
| } | |
| NotifyActionStart(currentAction); | |
| ProcessStoretoValue(currentAction); | |
| UpdateDSReturnValues(currentAction); | |
| CalculateActionFinalStatus(currentAction); | |
| st.Stop(); | |
| NotifyActionEnd(currentAction); | |
| } | |
| } | |
| } | |
| SetBusinessFlowActivitiesAndActionsSkipStatus(); | |
| CalculateActivityFinalStatus(currentActivity); | |
| stE.Stop(); | |
| NotifyActivityEnd(currentActivity); | |
| } | |
| } | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity : {ex.Message}"); | |
| } | |
| } | |
| /// <summary> | |
| /// Adds error handler activities to the execution report | |
| /// </summary> | |
| private void AddErrorHandlerActivityInReport() | |
| { | |
| try | |
| { | |
| ObservableList<ErrorHandler> errorActivities = GetActiveErrorHandlers(); | |
| if (errorActivities.Count > 0) | |
| { | |
| ProcessErrorHandlerActivities(errorActivities); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity to report. Details: {ex.Message}", ex); | |
| } | |
| } | |
| private ObservableList<ErrorHandler> GetActiveErrorHandlers() | |
| { | |
| return new ObservableList<ErrorHandler>( | |
| CurrentBusinessFlow.Activities | |
| .Where(a => a.GetType() == typeof(ErrorHandler) && | |
| a.Status != eRunStatus.Skipped && | |
| a.Active == true) | |
| .Cast<ErrorHandler>() | |
| .ToList()); | |
| } | |
| private void ProcessErrorHandlerActivities(ObservableList<ErrorHandler> errorActivities) | |
| { | |
| foreach (ErrorHandler errActivity in errorActivities) | |
| { | |
| var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid); | |
| if (currentActivity != null) | |
| { | |
| ProcessSingleErrorHandler(errActivity, currentActivity); | |
| } | |
| } | |
| } | |
| private void ProcessSingleErrorHandler(ErrorHandler errActivity, ErrorHandler currentActivity) | |
| { | |
| handlerPostExecutionAction = errActivity.ErrorHandlerPostExecutionAction; | |
| CurrentBusinessFlow.CurrentActivity = errActivity; | |
| SetCurrentActivityAgent(); | |
| Stopwatch stE = new Stopwatch(); | |
| stE.Start(); | |
| NotifyActivityStart(currentActivity); | |
| ProcessErrorHandlerActions(errActivity, currentActivity); | |
| SetBusinessFlowActivitiesAndActionsSkipStatus(); | |
| CalculateActivityFinalStatus(currentActivity); | |
| stE.Stop(); | |
| NotifyActivityEnd(currentActivity); | |
| } | |
| private void ProcessErrorHandlerActions(ErrorHandler errActivity, ErrorHandler currentActivity) | |
| { | |
| foreach (Act act in errActivity.Acts) | |
| { | |
| var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid); | |
| if (currentAction != null) | |
| { | |
| ProcessErrorHandlerAction(currentAction, currentActivity); | |
| } | |
| } | |
| } | |
| private void ProcessErrorHandlerAction(Act currentAction, ErrorHandler currentActivity) | |
| { | |
| Stopwatch st = new Stopwatch(); | |
| st.Start(); | |
| if (currentAction.Active) | |
| { | |
| CurrentBusinessFlow.CurrentActivity.Acts.CurrentItem = currentAction; | |
| if (currentActivity.HandlerType == eHandlerType.Popup_Handler) | |
| { | |
| currentAction.Timeout = 1; | |
| } | |
| NotifyActionStart(currentAction); | |
| ProcessStoretoValue(currentAction); | |
| UpdateDSReturnValues(currentAction); | |
| CalculateActionFinalStatus(currentAction); | |
| st.Stop(); | |
| NotifyActionEnd(currentAction); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs (2)
104-105: Add XML documentation for the new error tracking lists.The newly added lists for tracking error handler activities and actions should be documented to explain their purpose and usage.
+/// <summary> +/// Stores actions executed within error handler activities for reporting purposes +/// </summary> public List<Act> errActivityActionList = []; + +/// <summary> +/// Stores error handler activities that were executed for reporting purposes +/// </summary> public List<ErrorHandler> errActivityList = [];
2185-2185: Track error handler execution for reporting.The code adds executed error handler actions and activities to their respective tracking lists. This is good for reporting purposes but could benefit from debug logging.
Add debug logging before adding items to the lists:
+Reporter.ToLog(eLogLevel.DEBUG, $"Adding executed error handler action {act.Description} to tracking list"); errActivityActionList.Add(act); +Reporter.ToLog(eLogLevel.DEBUG, $"Adding executed error handler activity {errActivity.ActivityName} to tracking list"); errActivityList.Add(errActivity);Also applies to: 2195-2195
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build Stage / build
🔇 Additional comments (1)
Ginger/GingerCoreNET/Run/GingerExecutionEngine.cs (1)
2081-2127: 🛠️ Refactor suggestionEnhance error handling and logging in AddErrorHandlerActivityInReport method.
The method has several areas that could be improved:
- The error message lacks context and stack trace
- The method could benefit from more detailed logging
- The empty catch block at line 2124 should include error handling
Apply this diff to improve error handling and logging:
private void AddErrorHandlerActivityInReport() { try { + Reporter.ToLog(eLogLevel.DEBUG, "Starting to add error handler activities to report"); ObservableList<ErrorHandler> errorActivities = new ObservableList<ErrorHandler>(CurrentBusinessFlow.Activities.Where(a => a.GetType() == typeof(ErrorHandler) && a.Status != eRunStatus.Skipped && a.Active == true ).Cast<ErrorHandler>().ToList()); if (errorActivities.Count > 0) { + Reporter.ToLog(eLogLevel.DEBUG, $"Found {errorActivities.Count} error handler activities to process"); foreach (ErrorHandler errActivity in errorActivities) { var currentActivity = errActivityList.Find(k => errActivity.Guid == k.Guid); if (currentActivity != null) { CurrentBusinessFlow.CurrentActivity = errActivity; NotifyActivityStart(currentActivity); foreach (Act act in errActivity.Acts) { var currentAction = errActivityActionList.Find(k => act.Guid == k.Guid); if (currentAction != null) { Stopwatch st = new Stopwatch(); st.Start(); if (currentAction.Active) { CurrentBusinessFlow.CurrentActivity.Acts.CurrentItem = currentAction; NotifyActionStart(currentAction); NotifyActionEnd(currentAction); } } } NotifyActivityEnd(currentActivity); } } } + Reporter.ToLog(eLogLevel.DEBUG, "Completed adding error handler activities to report"); } catch (Exception ex) { - Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity : {ex.Message}"); + Reporter.ToLog(eLogLevel.ERROR, $"Failed to add Error Handler Activity to report. Details: {ex.Message}", ex); } }Likely invalid or redundant comment.
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation