This is an automated email from the ASF dual-hosted git repository.
yuluo-yx pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new df843942cc fix(ai): improve SOP schedule error handling (#4301)
df843942cc is described below
commit df843942cccac7ae49b14bd3ba647337e2a54f27
Author: shown <[email protected]>
AuthorDate: Thu Aug 6 23:47:00 2026 +0800
fix(ai): improve SOP schedule error handling (#4301)
Signed-off-by: yuluo-yx <[email protected]>
---
.../hertzbeat/ai/controller/SopController.java | 3 +-
.../hertzbeat/ai/schedule/SopScheduleExecutor.java | 5 +-
.../ai/service/impl/SopScheduleServiceImpl.java | 15 ++++--
.../hertzbeat/ai/controller/SopControllerTest.java | 61 ++++++++++++++++++++++
.../ai/schedule/SopScheduleExecutorTest.java | 17 ++++++
.../service/impl/SopScheduleServiceImplTest.java | 58 ++++++++++++++++++++
6 files changed, 151 insertions(+), 8 deletions(-)
diff --git
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/controller/SopController.java
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/controller/SopController.java
index d2b0f31029..92491e5f0c 100644
---
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/controller/SopController.java
+++
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/controller/SopController.java
@@ -30,6 +30,7 @@ import org.apache.hertzbeat.ai.sop.model.SopDefinition;
import org.apache.hertzbeat.ai.sop.model.SopResult;
import org.apache.hertzbeat.ai.sop.registry.SkillRegistry;
import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
@@ -143,7 +144,7 @@ public class SopController {
.status("FAILED")
.error("SOP skill not found: " + skillName)
.build();
- return ResponseEntity.notFound().build();
+ return
ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResult);
}
Map<String, Object> inputParams = params != null ? params : new
HashMap<>();
diff --git
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutor.java
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutor.java
index be861e3d45..2466b84625 100644
---
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutor.java
+++
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutor.java
@@ -105,9 +105,8 @@ public class SopScheduleExecutor {
// Check if skill exists
var definition = skillRegistry.getSkill(schedule.getSopName());
if (definition == null) {
- log.warn("Skill {} not found, skipping schedule {}",
- schedule.getSopName(), schedule.getId());
- return;
+ // Do not silently skip an invalid schedule because its
execution time will still be advanced.
+ throw new IllegalStateException("SOP skill not found: " +
schedule.getSopName());
}
// Parse parameters
diff --git
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImpl.java
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImpl.java
index 88c8d0a23b..7e6f0f0850 100644
---
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImpl.java
+++
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImpl.java
@@ -148,10 +148,17 @@ public class SopScheduleServiceImpl implements
SopScheduleService {
private LocalDateTime calculateNextRunTime(String cronExpression) {
try {
CronExpression cron = CronExpression.parse(cronExpression);
- return cron.next(LocalDateTime.now());
- } catch (Exception e) {
- log.error("Failed to calculate next run time for cron: {}",
cronExpression, e);
- return null;
+ LocalDateTime nextRunTime = cron.next(LocalDateTime.now());
+ if (nextRunTime == null) {
+ // Expressions such as February 31 are syntactically valid but
can never be triggered.
+ throw new IllegalArgumentException(
+ "Cron expression has no future execution time: " +
cronExpression);
+ }
+ return nextRunTime;
+ } catch (IllegalArgumentException e) {
+ throw e;
+ } catch (RuntimeException e) {
+ throw new IllegalArgumentException("Failed to calculate next run
time: " + cronExpression, e);
}
}
}
diff --git
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/controller/SopControllerTest.java
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/controller/SopControllerTest.java
new file mode 100644
index 0000000000..00081b64e0
--- /dev/null
+++
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/controller/SopControllerTest.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hertzbeat.ai.controller;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.when;
+
+import org.apache.hertzbeat.ai.sop.engine.SopEngine;
+import org.apache.hertzbeat.ai.sop.model.SopResult;
+import org.apache.hertzbeat.ai.sop.registry.SkillRegistry;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+
+/**
+ * Verifies that synchronous SOP failures include a diagnostic response body.
+ */
+@ExtendWith(MockitoExtension.class)
+class SopControllerTest {
+
+ @Mock
+ private SkillRegistry skillRegistry;
+
+ @Mock
+ private SopEngine sopEngine;
+
+ @InjectMocks
+ private SopController controller;
+
+ @Test
+ void executeSopSyncShouldReturnFailureBodyWhenSkillDoesNotExist() {
+ when(skillRegistry.getSkill("missing")).thenReturn(null);
+
+ ResponseEntity<SopResult> response =
controller.executeSopSync("missing", null);
+
+ assertEquals(HttpStatus.NOT_FOUND, response.getStatusCode());
+ assertNotNull(response.getBody());
+ assertEquals("FAILED", response.getBody().getStatus());
+ assertEquals("SOP skill not found: missing",
response.getBody().getError());
+ }
+}
diff --git
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutorTest.java
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutorTest.java
index e4f6bcfe3c..77c7d18843 100644
---
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutorTest.java
+++
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/schedule/SopScheduleExecutorTest.java
@@ -17,6 +17,7 @@
package org.apache.hertzbeat.ai.schedule;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.Mockito.doThrow;
@@ -37,6 +38,7 @@ import org.apache.hertzbeat.common.entity.ai.SopSchedule;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -100,6 +102,21 @@ class SopScheduleExecutorTest {
verify(scheduleService).updateAfterExecution(1L);
}
+ @Test
+ void checkShouldPushErrorWhenScheduledSkillNoLongerExists() {
+ SopSchedule schedule = schedule(1L, null);
+ when(scheduleService.getDueSchedules()).thenReturn(List.of(schedule));
+ when(skillRegistry.getSkill("daily_inspection")).thenReturn(null);
+
+ executor.checkAndExecuteDueSchedules();
+
+ ArgumentCaptor<ChatMessage> messageCaptor =
ArgumentCaptor.forClass(ChatMessage.class);
+ verify(chatMessageDao).save(messageCaptor.capture());
+ assertTrue(messageCaptor.getValue().getContent().contains("SOP skill
not found: daily_inspection"));
+ verifyNoInteractions(sopEngine);
+ verify(scheduleService).updateAfterExecution(1L);
+ }
+
private SopSchedule schedule(Long id, String params) {
return SopSchedule.builder()
.id(id)
diff --git
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImplTest.java
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImplTest.java
new file mode 100644
index 0000000000..5eb588b872
--- /dev/null
+++
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/SopScheduleServiceImplTest.java
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hertzbeat.ai.service.impl;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+import org.apache.hertzbeat.ai.dao.SopScheduleDao;
+import org.apache.hertzbeat.common.entity.ai.SopSchedule;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+/**
+ * Verifies that SOP schedules with no future execution time are not persisted.
+ */
+@ExtendWith(MockitoExtension.class)
+class SopScheduleServiceImplTest {
+
+ @Mock
+ private SopScheduleDao sopScheduleDao;
+
+ @InjectMocks
+ private SopScheduleServiceImpl scheduleService;
+
+ @Test
+ void createScheduleShouldRejectCronWithoutFutureExecutionTime() {
+ SopSchedule schedule = SopSchedule.builder()
+ .conversationId(1L)
+ .sopName("daily_inspection")
+ .cronExpression("0 0 0 31 2 *")
+ .build();
+
+ IllegalArgumentException exception = assertThrows(
+ IllegalArgumentException.class, () ->
scheduleService.createSchedule(schedule));
+
+ assertTrue(exception.getMessage().contains("no future execution
time"));
+ verifyNoInteractions(sopScheduleDao);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
