Skip to content

Conversation

@Zzih96
Copy link
Contributor

@Zzih96 Zzih96 commented Dec 24, 2025

Purpose of the pull request

fix #17817

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

@SbloodyS SbloodyS changed the title [Fix-17817] [Master]add workflow timeout [Fix-17817] [Master] Fix workflow timeout alerts failed Dec 24, 2025
@SbloodyS SbloodyS added the bug Something isn't working label Dec 24, 2025
@SbloodyS SbloodyS added this to the 3.4.0 milestone Dec 24, 2025
@sonarqubecloud
Copy link

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM, it's better to add IT case.

* @param modifyBy modifyBy
*/
public void sendWorkflowTimeoutAlert(WorkflowInstance workflowInstance, ProjectUser projectUser) {
public void sendWorkflowTimeoutAlert(WorkflowInstance workflowInstance, ProjectUser projectUser, String modifyBy) {
Copy link
Member

Choose a reason for hiding this comment

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

Please don't change this, this PR should only fix the alert bug.

Comment on lines +52 to +55
// Calculate remaining time until timeout: timeout - elapsed time
long delayTime = TimeUnit.MINUTES.toMillis(timeout)
- (System.currentTimeMillis() - workflowInstance.getStartTime().getTime());
// Ensure delayTime is not negative (trigger immediately if already timeout)
Copy link
Member

Choose a reason for hiding this comment

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

I am not clear in which case the delayTime might be negative, since System.currentTimeMillis() - workflowInstance.getStartTime().getTime() should always > 0.

Comment on lines +63 to +65
private void doWorkflowTimeoutAlert(final WorkflowInstance workflowInstance) {
// ProjectUser will be built in WorkflowAlertManager
workflowAlertManager.sendWorkflowTimeoutAlert(workflowInstance, null);
Copy link
Member

Choose a reason for hiding this comment

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

Should fix like #17818, otherwise will throw NPE

@SbloodyS SbloodyS modified the milestones: 3.4.0, 3.4.1 Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Master] Missing workflow timeout alerts

3 participants