diff --git a/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/DispatchResult.java b/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/DispatchResult.java new file mode 100644 index 00000000..8367ac0a --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/DispatchResult.java @@ -0,0 +1,44 @@ +package vip.mate.trigger.dispatch; + +/** + * Outcome of a trigger fire. The dispatcher used to return either a + * {@code WorkflowRunResult} or {@code null}, which led the ingest and + * scheduler paths to treat null as "fired" — incrementing + * {@code fireCount} / {@code lastFiredAt} even when the dispatch was + * a no-op or an error. This record makes the outcome explicit so each + * caller can update bookkeeping honestly. + * + * + */ +public record DispatchResult(Kind kind, Long runId, String reason) { + + public enum Kind { FIRED, SKIPPED, FAILED } + + public boolean fired() { return kind == Kind.FIRED; } + + public static DispatchResult fired(Long runId) { + return new DispatchResult(Kind.FIRED, runId, null); + } + + public static DispatchResult skipped(String reason) { + return new DispatchResult(Kind.SKIPPED, null, reason); + } + + public static DispatchResult failed(String message) { + return new DispatchResult(Kind.FAILED, null, message); + } + + public static DispatchResult failed(Long runId, String message) { + return new DispatchResult(Kind.FAILED, runId, message); + } +} diff --git a/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/TriggerDispatcher.java b/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/TriggerDispatcher.java index 14f44109..d038b1f2 100644 --- a/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/TriggerDispatcher.java +++ b/mateclaw-server/src/main/java/vip/mate/trigger/dispatch/TriggerDispatcher.java @@ -42,33 +42,60 @@ public class TriggerDispatcher { /** * Dispatch a single fire of {@code trigger}. {@code event} is the - * source-event context (cron tick metadata, channel message, etc.) — its - * top-level fields are exposed to the payload template under - * {@code event.*}. Returns {@code null} when the trigger is not - * dispatchable (no published revision, unsupported target type) so the - * caller can record the skip without erroring. + * source-event context (cron tick metadata, channel message, etc.) — + * its top-level fields are exposed to the payload template under + * {@code event.*}. Returns a {@link DispatchResult} so the caller + * can distinguish a real fire from a pre-flight skip or a runner + * failure and update {@code fireCount} / {@code lastFiredAt} / + * {@code lastError} accordingly. */ - public WorkflowRunResult dispatch(TriggerEntity trigger, Map event) { + public DispatchResult dispatch(TriggerEntity trigger, Map event) { if (!"workflow".equalsIgnoreCase(trigger.getTargetType())) { log.warn("Trigger {} target_type {} not supported in v0; skipping fire", trigger.getId(), trigger.getTargetType()); - return null; + return DispatchResult.skipped( + "unsupported target_type: " + trigger.getTargetType()); } WorkflowGraphLoader.Loaded loaded = graphLoader.load(trigger.getTargetId()); if (loaded.graph() == null) { log.info("Trigger {} dispatch skipped: no published revision for workflow {}", trigger.getId(), trigger.getTargetId()); - return null; + return DispatchResult.skipped( + "no published revision for workflow " + trigger.getTargetId()); } - Map inputs = renderInputs(trigger, event); + Map inputs; + try { + inputs = renderInputs(trigger, event); + } catch (Exception e) { + return DispatchResult.failed("payload render failed: " + e.getMessage()); + } WorkflowRunRequest req = new WorkflowRunRequest( trigger.getTargetId(), loaded.revisionId(), trigger.getWorkspaceId(), "trigger:" + trigger.getId(), inputs); - return runner.run(loaded.graph(), req); + try { + WorkflowRunResult result = runner.run(loaded.graph(), req); + if (result == null) { + return DispatchResult.failed("runner returned null result"); + } + // The runner's state taxonomy: succeeded / paused / running / + // failed. Anything other than failed counts as a real fire — a + // paused run still consumed the trigger and produced a + // workflow_run row that the operator can resume. + if ("failed".equalsIgnoreCase(result.state())) { + return DispatchResult.failed(result.runId(), + "workflow run failed: " + + (result.errorMessage() == null ? "(no message)" : result.errorMessage())); + } + return DispatchResult.fired(result.runId()); + } catch (Exception e) { + log.error("Trigger {} dispatch failed for workflow {}: {}", + trigger.getId(), trigger.getTargetId(), e.getMessage(), e); + return DispatchResult.failed("runner threw: " + e.getMessage()); + } } private Map renderInputs(TriggerEntity trigger, Map event) { diff --git a/mateclaw-server/src/main/java/vip/mate/trigger/ingest/TriggerEventIngestService.java b/mateclaw-server/src/main/java/vip/mate/trigger/ingest/TriggerEventIngestService.java index db25c2ee..5ac67cd6 100644 --- a/mateclaw-server/src/main/java/vip/mate/trigger/ingest/TriggerEventIngestService.java +++ b/mateclaw-server/src/main/java/vip/mate/trigger/ingest/TriggerEventIngestService.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.DuplicateKeyException; import org.springframework.stereotype.Service; +import vip.mate.trigger.dispatch.DispatchResult; import vip.mate.trigger.dispatch.TriggerDispatcher; import vip.mate.trigger.model.TriggerEntity; import vip.mate.trigger.model.TriggerEventEntity; @@ -118,14 +119,50 @@ public class TriggerEventIngestService { if (!rateLimiter.tryAcquire(trigger.getId(), limit, Instant.now())) { return IngestResult.dropped(trigger.getId(), Reason.RATE_LIMITED); } + DispatchResult outcome; try { - dispatcher.dispatch(trigger, envelope.data()); + outcome = dispatcher.dispatch(trigger, envelope.data()); } catch (Exception e) { + // Belt-and-suspenders — the dispatcher already wraps its own + // exceptions, but if anything escapes we mark it as DISPATCH_ERROR + // and persist last_error so the UI surfaces *why*. log.error("Trigger {} dispatch threw on event ingest: {}", trigger.getId(), e.getMessage(), e); + persistDispatchOutcome(trigger, DispatchResult.failed("dispatch threw: " + e.getMessage())); return IngestResult.dropped(trigger.getId(), Reason.DISPATCH_ERROR); } - return IngestResult.fired(trigger.getId()); + persistDispatchOutcome(trigger, outcome); + return switch (outcome.kind()) { + case FIRED -> IngestResult.fired(trigger.getId()); + case SKIPPED -> IngestResult.dropped(trigger.getId(), Reason.DISPATCH_SKIPPED); + case FAILED -> IngestResult.dropped(trigger.getId(), Reason.DISPATCH_ERROR); + }; + } + + /** + * Update the trigger row's bookkeeping based on the dispatch outcome. + * Only FIRED bumps {@code fireCount} and {@code lastFiredAt} — SKIPPED + * and FAILED outcomes were treated as fires before, which made the + * stats lie. {@code lastDispatchedAt} stamps every attempt so the UI + * can distinguish "never attempted" from "attempted but skipped". + */ + private void persistDispatchOutcome(TriggerEntity trigger, DispatchResult outcome) { + try { + LocalDateTime now = LocalDateTime.now(); + trigger.setLastDispatchedAt(now); + if (outcome.fired()) { + trigger.setFireCount( + (trigger.getFireCount() == null ? 0L : trigger.getFireCount()) + 1); + trigger.setLastFiredAt(now); + trigger.setLastError(null); + } else { + trigger.setLastError(outcome.reason()); + } + triggerMapper.updateById(trigger); + } catch (Exception e) { + // Best-effort bookkeeping — never let a stats write fail ingest. + log.warn("Trigger {} bookkeeping update failed: {}", trigger.getId(), e.getMessage()); + } } private boolean recordDedupRow(TriggerEntity trigger, TriggerEventEnvelope envelope) { @@ -174,7 +211,11 @@ public class TriggerEventIngestService { } public enum Reason { - PATTERN_MISMATCH, BOT_SELF, DUPLICATE, RATE_LIMITED, EXHAUSTED, DISPATCH_ERROR + PATTERN_MISMATCH, BOT_SELF, DUPLICATE, RATE_LIMITED, EXHAUSTED, + /** Dispatcher returned SKIPPED — pre-flight rejected (no published revision, etc.). */ + DISPATCH_SKIPPED, + /** Dispatcher returned FAILED — runner threw or workflow run ended in failed state. */ + DISPATCH_ERROR } public record IngestResult(long triggerId, boolean fired, Reason droppedReason) { diff --git a/mateclaw-server/src/main/java/vip/mate/trigger/model/TriggerEntity.java b/mateclaw-server/src/main/java/vip/mate/trigger/model/TriggerEntity.java index f6e7f632..543624a1 100644 --- a/mateclaw-server/src/main/java/vip/mate/trigger/model/TriggerEntity.java +++ b/mateclaw-server/src/main/java/vip/mate/trigger/model/TriggerEntity.java @@ -56,6 +56,16 @@ public class TriggerEntity { @TableField(value = "last_fired_at", updateStrategy = FieldStrategy.ALWAYS) private LocalDateTime lastFiredAt; + /** Most recent dispatch outcome message; null on success, populated on + * SKIPPED / FAILED so the UI can show why a trigger has stopped firing. */ + @TableField(value = "last_error", updateStrategy = FieldStrategy.ALWAYS) + private String lastError; + + /** Stamp of the last dispatch attempt regardless of outcome — used to + * distinguish "never attempted" from "attempted but skipped". */ + @TableField(value = "last_dispatched_at", updateStrategy = FieldStrategy.ALWAYS) + private LocalDateTime lastDispatchedAt; + /** Lamport counter — bump on every cron expression / payload template change. */ private Long patternVersion; diff --git a/mateclaw-server/src/main/java/vip/mate/trigger/scheduler/TriggerScheduler.java b/mateclaw-server/src/main/java/vip/mate/trigger/scheduler/TriggerScheduler.java index 8ba37e05..03b1fe62 100644 --- a/mateclaw-server/src/main/java/vip/mate/trigger/scheduler/TriggerScheduler.java +++ b/mateclaw-server/src/main/java/vip/mate/trigger/scheduler/TriggerScheduler.java @@ -230,14 +230,31 @@ public class TriggerScheduler { return; // peer is firing } try { - dispatcher.dispatch(live, Map.of("firedAt", Instant.now().toString())); - // Bump fire bookkeeping post-dispatch so a slow workflow does not - // delay subsequent ticks; this row update is best-effort. - live.setFireCount((live.getFireCount() == null ? 0L : live.getFireCount()) + 1); - live.setLastFiredAt(LocalDateTime.now()); + vip.mate.trigger.dispatch.DispatchResult outcome = + dispatcher.dispatch(live, Map.of("firedAt", Instant.now().toString())); + // Bookkeeping is honest: only a real fire bumps fireCount / + // lastFiredAt. Skipped (no published revision, etc.) and failed + // outcomes still record lastDispatchedAt + lastError so the UI + // can show why a cron stopped firing. + LocalDateTime now = LocalDateTime.now(); + live.setLastDispatchedAt(now); + if (outcome != null && outcome.fired()) { + live.setFireCount((live.getFireCount() == null ? 0L : live.getFireCount()) + 1); + live.setLastFiredAt(now); + live.setLastError(null); + } else { + live.setLastError(outcome == null ? "dispatcher returned null" : outcome.reason()); + } triggerMapper.updateById(live); } catch (Exception e) { log.error("[TriggerScheduler] trigger {} fire failed: {}", triggerId, e.getMessage(), e); + try { + live.setLastDispatchedAt(LocalDateTime.now()); + live.setLastError("scheduler threw: " + e.getMessage()); + triggerMapper.updateById(live); + } catch (Exception ignored) { + // Best-effort — don't let a bookkeeping failure mask the dispatch failure. + } } finally { lock.get().unlock(); } diff --git a/mateclaw-server/src/main/resources/db/migration/h2/V98__trigger_last_error.sql b/mateclaw-server/src/main/resources/db/migration/h2/V98__trigger_last_error.sql new file mode 100644 index 00000000..3ea77051 --- /dev/null +++ b/mateclaw-server/src/main/resources/db/migration/h2/V98__trigger_last_error.sql @@ -0,0 +1,7 @@ +-- Persist the most recent dispatch outcome message on the trigger row +-- itself so the UI can show *why* a trigger has stopped firing without +-- joining trigger_event for forensics. The dispatcher writes a non-null +-- message on SKIPPED / FAILED outcomes and clears it on FIRED. + +ALTER TABLE mate_trigger ADD COLUMN IF NOT EXISTS last_error VARCHAR(2048); +ALTER TABLE mate_trigger ADD COLUMN IF NOT EXISTS last_dispatched_at TIMESTAMP; diff --git a/mateclaw-server/src/main/resources/db/migration/mysql/V98__trigger_last_error.sql b/mateclaw-server/src/main/resources/db/migration/mysql/V98__trigger_last_error.sql new file mode 100644 index 00000000..b136783f --- /dev/null +++ b/mateclaw-server/src/main/resources/db/migration/mysql/V98__trigger_last_error.sql @@ -0,0 +1,29 @@ +-- See the H2 file for context. MySQL 8.0 doesn't support +-- `ADD COLUMN IF NOT EXISTS`, so the existence check goes through +-- INFORMATION_SCHEMA + a prepared statement. + +SET @col_exists := ( + SELECT COUNT(*) FROM information_schema.COLUMNS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'mate_trigger' + AND COLUMN_NAME = 'last_error' +); +SET @ddl := IF(@col_exists = 0, + 'ALTER TABLE mate_trigger ADD COLUMN last_error VARCHAR(2048)', + 'SELECT 1'); +PREPARE stmt FROM @ddl; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; + +SET @col_exists := ( + SELECT COUNT(*) FROM information_schema.COLUMNS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'mate_trigger' + AND COLUMN_NAME = 'last_dispatched_at' +); +SET @ddl := IF(@col_exists = 0, + 'ALTER TABLE mate_trigger ADD COLUMN last_dispatched_at TIMESTAMP NULL', + 'SELECT 1'); +PREPARE stmt FROM @ddl; +EXECUTE stmt; +DEALLOCATE PREPARE stmt;