Skip to content

Fix: Ensure emptyMsgOnFail is triggered on all errors #528

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.

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

Merged

Conversation

bernardmms
Copy link

Which issues are addressed by this Pull Request?
This Pull Request addresses issue #517.

What does this Pull Request change?
The change corrects the behavior of the emptyMsgOnFail option in the modbus-flex-getter node. Previously, when an error occurred during message handling, the function mbBasics.sendEmptyMsgOnFail was being called with an incorrect message reference (msg). This prevented the node from correctly sending an empty message on failure.

The fix consists of changing the call from:

mbBasics.sendEmptyMsgOnFail(node, err, msg)

To:

mbBasics.sendEmptyMsgOnFail(node, err, origMsg)

This ensures the original input message is passed, restoring the expected behavior of emitting an empty message when a failure occurs.

Does this Pull Request introduce any potentially breaking changes?
No breaking changes are introduced. This is a bugfix that restores previously expected functionality for users relying on emptyMsgOnFail. Existing configurations and integrations will continue to work as before.

@grewek grewek self-requested a review May 20, 2025 14:10
@grewek
Copy link

grewek commented May 20, 2025

Thank you for your contribution.

I will take a look at it and merge it when everything looks good!

@grewek grewek self-assigned this May 20, 2025
@grewek grewek changed the base branch from master to feature/pr_empty_msg_fix May 20, 2025 14:33
@grewek grewek removed their request for review May 20, 2025 14:34
@grewek grewek merged commit c53a8e4 into BiancoRoyal:feature/pr_empty_msg_fix May 20, 2025
3 checks passed
@grewek
Copy link

grewek commented May 21, 2025

I just merged your PR with develop so in the next version the fix will be included.

@bernardmms
Copy link
Author

Thank you! Happy to help

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.

3 participants