Skip to content

[4.x] Fix #603 - Add guard code to collections, tidy up Workflow serializer #604

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
merged 1 commit into from
Jun 5, 2025

Conversation

ricardozanini
Copy link
Member

@ricardozanini ricardozanini commented May 30, 2025

Many thanks for submitting your Pull Request ❤️!
Fix #603

What this PR does / why we need it:

Special notes for reviewers:
@fjtirado I've made a "breaking" change. Previously, the Workflow.toJson() methods would swallow all exceptions, logging them to the console. Since the API did not come with an SLF4J implementation, the message will not be displayed, and the exception will be lost forever.

Instead, we now throw an IllegalException to fail fast and avoid forcing client code to catch exceptions.

@gmunozfe, mind taking a look too? Perhaps we can give it a try with Kogito and assess this change impact.

Additional information (if needed):

@fjtirado
Copy link
Collaborator

fjtirado commented Jun 2, 2025

@ricardozanini actually toJson and to toYaml behaviour was inconsistent with the fact that fromSource already throw an IllegalArgumentException ( I guess that what you mean, you did not touch fromSource, but toJson and toYaml)

@ricardozanini ricardozanini changed the title Fix #603 - Add guard code to collections, tidy up Workflow serializer [4.x] Fix #603 - Add guard code to collections, tidy up Workflow serializer Jun 2, 2025
@ricardozanini ricardozanini merged commit 8edf0af into serverlessworkflow:4.x Jun 5, 2025
2 checks passed
@ricardozanini ricardozanini deleted the issue-603 branch June 5, 2025 18:35
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.

2 participants