Skip to content

Conversation

@Hritikraj8804
Copy link
Contributor

✨ Description

This PR fixes a NullPointerException in the SetVariables task when execution variables are not initialized.

When overwrite: false is used, the task assumed execution.getVariables() was non-null and attempted to call containsKey(...), which caused a crash for webhook-triggered executions (and some manual executions).

The fix defensively initializes a local variables map before performing duplicate checks and merges, preserving existing behavior while preventing the NPE.

🔗 Related Issue

Closes #13756

🛠️ Backend Checklist

  • Code compiles successfully
  • Bug reproduced locally (manual + webhook execution)
  • Fix verified locally

📝 Additional Notes

The issue author mentioned that webhook-triggered executions may not initialize variables at creation time.
This fix makes SetVariables resilient to that case without changing execution semantics.

@loicmathieu
Copy link
Member

I wonder if the correct fix should not be to add flow variables to the execution when we trigger an execution from a Webhook.

@loicmathieu loicmathieu self-requested a review December 19, 2025 15:40
@loicmathieu loicmathieu added the kind/external Pull requests raised by community contributors label Dec 19, 2025
@Hritikraj8804
Copy link
Contributor Author

Thanks for the insight that makes sense 👍

My intention with this PR was to provide a minimal and defensive fix at the task level, since execution.getVariables() is not guaranteed to be initialized in all execution paths (as seen with webhook-triggered executions). This avoids the NPE without changing execution creation semantics.

That said, I agree that initializing flow variables when creating the execution from a webhook could be a more correct and consistent fix, especially since other triggers already do this. If you think that’s the preferred approach, I’m happy to adjust the PR to initialize variables at execution creation time instead (or complement this fix accordingly).

Let me know which direction you’d prefer.

@loicmathieu
Copy link
Member

I'm OK to keep a defensive code in this place as it would prevent other source of bugs (a NPE is always a bug), but we should also fix missing flow variable for Webhook to align with other kind of triggers.

@MilosPaunovic MilosPaunovic added the area/backend Needs backend code changes label Dec 22, 2025
@Hritikraj8804
Copy link
Contributor Author

I’ve added initialization of execution variables in the Webhook trigger using flow.getVariables(), aligning webhook executions with other triggers while keeping the defensive fix in SetVariables.

Let me know if anything else should be adjusted.

Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

can you also revert most of your code format change (line lenght) to keep only real changes in the PR as it will be easier to follow.

@Hritikraj8804
Copy link
Contributor Author

Done 👍
Reverted formatting-only changes and replaced new HashMap<>() with Collections.emptyMap() as suggested.

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

Labels

area/backend Needs backend code changes kind/external Pull requests raised by community contributors

Projects

Status: To review

Development

Successfully merging this pull request may close these issues.

Flow Variables are not set in Webhook-triggered Executions (NullPointerException)

3 participants