[Fix #1529] Printing error message#1530
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #1529 by surfacing failures from scheduled workflow instance execution, and reduces overall test runtime by shortening wait durations and tightening timeouts in lifecycle-related tests and samples.
Changes:
- Add error logging when a scheduled workflow instance completes exceptionally.
- Reduce wait durations in the
wait-setworkflow sample and the Java DSL wait workflow used in tests. - Speed up
LifeCycleEventsTestby removing a wait-based suspend/resume test and reducing a timeout assertion window.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| impl/test/src/test/resources/workflows-samples/wait-set.yaml | Shortens the wait duration in the sample workflow to reduce test time. |
| impl/test/src/test/java/io/serverlessworkflow/impl/test/LifeCycleEventsTest.java | Removes a slow wait-based suspend/resume test and tightens a timeout assertion; updates DSL wait to 200ms. |
| impl/core/src/main/java/io/serverlessworkflow/impl/scheduler/ScheduledInstanceRunnable.java | Adds logging for scheduled workflow execution failures via whenComplete. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ricardozanini
approved these changes
Jul 10, 2026
Also improve the time spent by LifeCycleEventTest Signed-off-by: fjtirado <ftirados@ibm.com>
| for (String type : AbstractLifeCyclePublisher.getLifeCycleTypes()) { | ||
| eventBroker.register(type, ce -> publishedEvents.add(ce)); | ||
| } | ||
| ; |
| .withExecutorFactory( | ||
| new ExecutorServiceFactory() { | ||
|
|
||
| private ExecutorService service = Executors.newFixedThreadPool(2); |
Comment on lines
+92
to
+94
| public void close() throws Exception { | ||
| service.shutdownNow(); | ||
| } |
Comment on lines
+59
to
+68
| .whenComplete( | ||
| (model, ex) -> { | ||
| if (ex != null) { | ||
| logger.error( | ||
| "Scheduled workflow instance {} of definition {} has failed", | ||
| instance.id(), | ||
| definition.id(), | ||
| ex); | ||
| } | ||
| })); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also improve the time spent by LifeCycleEventTest