Skip to content

Fixing workflow reload logic#7324

Merged
sfmskywalker merged 2 commits intorelease/3.6.0from
bugfix/workflow-reloader
Feb 25, 2026
Merged

Fixing workflow reload logic#7324
sfmskywalker merged 2 commits intorelease/3.6.0from
bugfix/workflow-reloader

Conversation

@lukhipolito-nexxbiz
Copy link
Contributor

  1. TriggerScheduler was returning a null for a payload, which would cause NullReferenceException, and this would break the populator which prevented other workflow definitions from being imported/published correctly.

  2. The reloader was only executing a part of the actual loading logic, meaning that workflow definitions used as activities were not correcly resolved to the expected versions

1. TriggerScheduler was returning a null for a payload, which would cause NullReferenceException, and this would break the populator which prevented other workflow definitions from being imported/published correctly.

2. The reloader was only executing a part of the actual loading logic, meaning that workflow definitions used as activities were not correcly resolved to the expected versions
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

This PR fixes two critical issues with workflow reload logic:

  • Fixed NullReferenceException in DefaultTriggerScheduler: Added null check for CronTriggerPayload before accessing properties, preventing crashes when trigger payload is missing. This prevents the populator from failing and blocking other workflow definitions from being imported/published.

  • Fixed incomplete reload logic in WorkflowDefinitionsReloader: Replaced the partial reload implementation with IRegistriesPopulator.PopulateAsync(), which properly handles the multi-stage reload process. The new approach correctly resolves workflow definitions used as activities by following the proper sequence: populate activity registry → populate workflow store → re-populate activity registry → re-update workflow store → publish notification.

The WorkflowDefinitionsReloader change is particularly important as DefaultRegistriesPopulator implements a 5-stage process (lines 17-35 in DefaultRegistriesPopulator.cs) that ensures workflow definitions used as activities are resolved to their expected versions, which the old implementation was missing.

Confidence Score: 4/5

  • This PR is safe to merge with low risk
  • The changes correctly address real production issues. The WorkflowDefinitionsReloader change is well-architected and delegates to the proper multi-stage reload process. The DefaultTriggerScheduler null check prevents crashes and follows the same pattern used elsewhere in the file. However, similar null checks are missing for Timer and StartAt triggers, which could cause the same issue.
  • Pay attention to DefaultTriggerScheduler.cs - consider adding null checks for Timer and StartAt triggers as well

Important Files Changed

Filename Overview
src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs Added null check for CronTriggerPayload to prevent NullReferenceException when payload is missing
src/modules/Elsa.Workflows.Runtime/Services/WorkflowDefinitionsReloader.cs Replaced partial reload logic with full IRegistriesPopulator to properly handle workflow definitions used as activities

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ReloadWorkflowDefinitionsAsync Called] --> B[IRegistriesPopulator.PopulateAsync]
    B --> C[Stage 1: Populate Activity Registry]
    C --> D[Stage 2: Populate Workflow Store<br/>skipUnregistered=false]
    D --> E[Stage 3: Re-populate Activity Registry<br/>with workflow definitions as activities]
    E --> F[Stage 4: Re-update Workflow Store<br/>skipUnregistered=true]
    F --> G[Stage 5: Publish WorkflowDefinitionsReloaded<br/>notification to other nodes]
    G --> H[Reload Complete]
    
    style B fill:#e1f5ff
    style E fill:#fff4e1
    style G fill:#e8f5e9
Loading

Last reviewed commit: 42fb7f9

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Additional Comments (2)

src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs
Similar null payload issue exists here - if TimerTriggerPayload is null, the tuple deconstruction will throw NullReferenceException. Consider adding the same null check before this line:

            var payload = trigger.GetPayload<TimerTriggerPayload>();
            if (payload is null)
            {
                logger.LogWarning("Timer trigger payload is empty. TriggerId: {TriggerId}. Skipping scheduling of this trigger", trigger.Id);
                continue;
            }
            
            var (startAt, interval) = payload;

src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs
Same null payload issue - if StartAtPayload is null, this will throw NullReferenceException. Add null check:

            var payload = trigger.GetPayload<StartAtPayload>();
            if (payload is null)
            {
                logger.LogWarning("StartAt trigger payload is empty. TriggerId: {TriggerId}. Skipping scheduling of this trigger", trigger.Id);
                continue;
            }
            
            var executeAt = payload.ExecuteAt;

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@sfmskywalker sfmskywalker merged commit 5816f6e into release/3.6.0 Feb 25, 2026
2 checks passed
@sfmskywalker sfmskywalker deleted the bugfix/workflow-reloader branch February 25, 2026 18:14
@sfmskywalker sfmskywalker linked an issue Feb 25, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle uncaught exceptions when resolving trigger payload

4 participants