Skip to content

feat(processWorker): add next version for process worker #1351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Phil91 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(processWorker): add next version for process worker #1351

Phil91 wants to merge 1 commit into from

Conversation

@Phil91
Copy link
Member

@Phil91 Phil91 commented May 5, 2025

Description

Enhance the process worker to enable the usage of multiple ProcessStepTypeIds

Why

To enable the process worker to be used with multiple ProcessStepTypeIds

Issue

N/A

Checklist

  • I have followed the contributing guidelines
  • I have performed IP checks for added or updated 3rd party libraries
  • I have created and linked IP issues or requested their creation by a committer
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have added tests that prove my changes work
  • I have checked that new and existing tests pass locally with my changes
  • I have commented my code, particularly in hard-to-understand areas

@Phil91 Phil91 requested a review from ntruchsess May 5, 2025 06:38
@Phil91 Phil91 moved this to IN PROGRESS in Portal May 5, 2025
@Phil91 Phil91 added enhancement New feature or request Back-End labels May 5, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems May 5, 2025
View reviewed changes
src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs
if (context.ProcessStepTypeId.HasValue)
{
context.ProcessRepositories.GetInstance<IProcessStepRepository>().AttachAndModifyProcessSteps(
ModifyStepStatusRange(context.ProcessSteps.Where(step => step.ProcessStepTypeId.Equals(context.ProcessStepTypeId.Value)), ProcessStepStatusId.FAILED, message));

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
context.ProcessStepTypeId
Loading
may be null at this access because it has a nullable type.

Copilot Autofix

AI 9 months ago

To fix the issue, we need to ensure that context.ProcessStepTypeId is not null before dereferencing it. This can be achieved by adding a null check before line 137. If context.ProcessStepTypeId is null, the method should either throw an exception or return early, depending on the intended behavior. Since the method already throws exceptions in other cases, throwing an exception when context.ProcessStepTypeId is null would be consistent with the existing design.

Suggested changeset 1
src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs b/src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs
--- a/src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs
+++ b/src/framework/Framework.Processes.Library.Next/Extensions/ManualProcessStepDataExtensions.cs
@@ -133,8 +133,10 @@
     {
-        if (context.ProcessStepTypeId.HasValue)
+        if (!context.ProcessStepTypeId.HasValue)
         {
-            context.ProcessRepositories.GetInstance<IProcessStepRepository>().AttachAndModifyProcessSteps(
-                ModifyStepStatusRange(context.ProcessSteps.Where(step => step.ProcessStepTypeId.Equals(context.ProcessStepTypeId.Value)), ProcessStepStatusId.FAILED, message));
+            throw new ArgumentNullException(nameof(context.ProcessStepTypeId), "ProcessStepTypeId cannot be null.");
         }
 
+        context.ProcessRepositories.GetInstance<IProcessStepRepository>().AttachAndModifyProcessSteps(
+            ModifyStepStatusRange(context.ProcessSteps.Where(step => step.ProcessStepTypeId.Equals(context.ProcessStepTypeId.Value)), ProcessStepStatusId.FAILED, message));
+
         context.ProcessRepositories.Attach(context.Process);
EOF
@@ -133,8 +133,10 @@
{
if (context.ProcessStepTypeId.HasValue)
if (!context.ProcessStepTypeId.HasValue)
{
context.ProcessRepositories.GetInstance<IProcessStepRepository>().AttachAndModifyProcessSteps(
ModifyStepStatusRange(context.ProcessSteps.Where(step => step.ProcessStepTypeId.Equals(context.ProcessStepTypeId.Value)), ProcessStepStatusId.FAILED, message));
throw new ArgumentNullException(nameof(context.ProcessStepTypeId), "ProcessStepTypeId cannot be null.");
}

context.ProcessRepositories.GetInstance<IProcessStepRepository>().AttachAndModifyProcessSteps(
ModifyStepStatusRange(context.ProcessSteps.Where(step => step.ProcessStepTypeId.Equals(context.ProcessStepTypeId.Value)), ProcessStepStatusId.FAILED, message));

context.ProcessRepositories.Attach(context.Process);
Copilot is powered by AI and may make mistakes. Always verify output.
src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs
Comment on lines +121 to +125
catch (Exception ex)
{
Environment.ExitCode = 1;
_logger.LogError(ex, "processing failed with following Exception {ExceptionMessage}", ex.Message);
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

Copilot Autofix

AI 9 months ago

To fix the issue, replace the generic catch (Exception ex) clause with specific catch blocks for exceptions that are expected and can be handled appropriately. For unexpected exceptions, rethrow them or handle them in a way that ensures they are not silently suppressed. In this case, we will:

  1. Catch specific exceptions such as OperationCanceledException (which is common in async operations with cancellation tokens) and log them appropriately.
  2. Add a fallback catch block for other exceptions, but rethrow critical exceptions like OutOfMemoryException or StackOverflowException that should not be handled in this context.
Suggested changeset 1
src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs b/src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs
--- a/src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs
+++ b/src/framework/Framework.Processes.Worker.Library.Next/ProcessExecutionService.cs
@@ -120,6 +120,10 @@
         }
-        catch (Exception ex)
+        catch (OperationCanceledException ex)
+        {
+            _logger.LogWarning(ex, "Operation was canceled: {ExceptionMessage}", ex.Message);
+        }
+        catch (Exception ex) when (ex is not OutOfMemoryException && ex is not StackOverflowException)
         {
             Environment.ExitCode = 1;
-            _logger.LogError(ex, "processing failed with following Exception {ExceptionMessage}", ex.Message);
+            _logger.LogError(ex, "Processing failed with the following exception: {ExceptionMessage}", ex.Message);
         }
EOF
@@ -120,6 +120,10 @@
}
catch (Exception ex)
catch (OperationCanceledException ex)
{
_logger.LogWarning(ex, "Operation was canceled: {ExceptionMessage}", ex.Message);
}
catch (Exception ex) when (ex is not OutOfMemoryException && ex is not StackOverflowException)
{
Environment.ExitCode = 1;
_logger.LogError(ex, "processing failed with following Exception {ExceptionMessage}", ex.Message);
_logger.LogError(ex, "Processing failed with the following exception: {ExceptionMessage}", ex.Message);
}
Copilot is powered by AI and may make mistakes. Always verify output.
@Phil91 Phil91 force-pushed the feature/process-worker-next branch from ef79fd7 to 2cf3c76 Compare May 5, 2025 07:16
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 5, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
6.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntruchsess ntruchsess Awaiting requested review from ntruchsess

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Back-End enhancement New feature or request

Projects

Portal
Status: IN PROGRESS

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Phil91