mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
feat(errors): add structured failure diagnostics (#277)
## Summary
- Make `FailureDetail` the canonical rich diagnostic shape for stage and
terminal failures, with terminal `RunFailure` carrying `{ reason, detail
}`.
- Preserve cause chains and move process stdout/stderr diagnostics into
sanitized `exec_output_tail` instead of embedding them in messages or
causes.
- Update ACP error plumbing, CLI/server/store rendering, OpenAPI, and
the generated TypeScript API client for the nested failure contract.
Closes #273
## Test Plan
- `cargo nextest run -p fabro-types -p fabro-core -p fabro-acp -p
fabro-api -p fabro-store -p fabro-server -p fabro-workflow -p fabro-cli
--no-fail-fast -E 'not test(/returns_svg/)' --status-level fail
--final-status-level fail`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D
warnings`
- `bun run typecheck` in `lib/packages/fabro-api-client`
- `bun run typecheck` in `apps/fabro-web`
This commit is contained in:
parent
87950295bd
commit
66519ee12a
39 changed files with 721 additions and 435 deletions
|
|
@ -5005,12 +5005,11 @@ components:
|
|||
type: boolean
|
||||
default: false
|
||||
|
||||
RunFailure:
|
||||
description: Rich terminal run failure diagnostics.
|
||||
FailureDetail:
|
||||
description: Rich diagnostic detail for a failed stage or terminal run.
|
||||
type: object
|
||||
required:
|
||||
- message
|
||||
- reason
|
||||
- category
|
||||
properties:
|
||||
message:
|
||||
|
|
@ -5019,8 +5018,6 @@ components:
|
|||
type: array
|
||||
items:
|
||||
type: string
|
||||
reason:
|
||||
$ref: "#/components/schemas/FailureReason"
|
||||
category:
|
||||
$ref: "#/components/schemas/FailureCategory"
|
||||
system_actor:
|
||||
|
|
@ -5036,6 +5033,18 @@ components:
|
|||
- $ref: "#/components/schemas/ExecOutputTail"
|
||||
- type: "null"
|
||||
|
||||
RunFailure:
|
||||
description: Terminal run failure reason and rich diagnostics.
|
||||
type: object
|
||||
required:
|
||||
- reason
|
||||
- detail
|
||||
properties:
|
||||
reason:
|
||||
$ref: "#/components/schemas/FailureReason"
|
||||
detail:
|
||||
$ref: "#/components/schemas/FailureDetail"
|
||||
|
||||
RunManifest:
|
||||
description: Self-contained workflow run manifest.
|
||||
type: object
|
||||
|
|
|
|||
|
|
@ -1,5 +1,27 @@
|
|||
use fabro_types::{CommandTermination, ExecOutputTail};
|
||||
|
||||
use crate::command::AcpCommandError;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct AcpProcessExit {
|
||||
pub termination: CommandTermination,
|
||||
pub exit_code: Option<i32>,
|
||||
pub exec_output_tail: Option<ExecOutputTail>,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for AcpProcessExit {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
let exit_code = self
|
||||
.exit_code
|
||||
.map_or_else(|| "unknown".to_string(), |code| code.to_string());
|
||||
write!(
|
||||
f,
|
||||
"ACP process exited before protocol completed: termination={}, exit_code={exit_code}",
|
||||
self.termination
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum AcpError {
|
||||
#[error(transparent)]
|
||||
|
|
@ -15,7 +37,12 @@ pub enum AcpError {
|
|||
Cancelled,
|
||||
|
||||
#[error("ACP turn timed out")]
|
||||
TimedOut { stderr: String },
|
||||
TimedOut {
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
},
|
||||
|
||||
#[error("{0}")]
|
||||
ProcessExited(AcpProcessExit),
|
||||
|
||||
#[error("ACP prompt stopped with {stop_reason}: {text}")]
|
||||
StopReason {
|
||||
|
|
@ -24,6 +51,18 @@ pub enum AcpError {
|
|||
},
|
||||
}
|
||||
|
||||
impl AcpError {
|
||||
#[must_use]
|
||||
pub fn exec_output_tail(&self) -> Option<ExecOutputTail> {
|
||||
match self {
|
||||
Self::TimedOut { exec_output_tail } => exec_output_tail.clone(),
|
||||
Self::ProcessExited(exit) => exit.exec_output_tail.clone(),
|
||||
Self::Sandbox(source) => source.default_redacted_output_tail(),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<agent_client_protocol::Error> for AcpError {
|
||||
fn from(error: agent_client_protocol::Error) -> Self {
|
||||
Self::Protocol(error)
|
||||
|
|
|
|||
|
|
@ -8,5 +8,5 @@ pub mod test_support;
|
|||
mod transport;
|
||||
|
||||
pub use command::{AcpCommand, AcpCommandError, resolve_acp_command};
|
||||
pub use error::AcpError;
|
||||
pub use error::{AcpError, AcpProcessExit};
|
||||
pub use session::{AcpRunRequest, AcpRunResult, render_stop_reason, run_acp_turn};
|
||||
|
|
|
|||
|
|
@ -102,7 +102,7 @@ pub async fn run_acp_turn(request: AcpRunRequest) -> Result<AcpRunResult, AcpErr
|
|||
return Err(AcpError::Cancelled);
|
||||
}
|
||||
Err(AcpError::TimedOut {
|
||||
stderr: state.stderr_tail().await,
|
||||
exec_output_tail: state.exec_output_tail().await,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -130,6 +130,9 @@ pub async fn run_acp_turn(request: AcpRunRequest) -> Result<AcpRunResult, AcpErr
|
|||
if let Some(startup_error) = state.take_startup_error().await {
|
||||
return Err(AcpError::Sandbox(startup_error));
|
||||
}
|
||||
if let Some(process_exit) = state.take_process_exit().await {
|
||||
return Err(AcpError::ProcessExited(process_exit));
|
||||
}
|
||||
return Err(map_protocol_error(error));
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -9,10 +9,10 @@ use agent_client_protocol::{
|
|||
Agent, Client, ConnectTo, Error as ProtocolError, Lines, Result as AcpProtocolResult,
|
||||
};
|
||||
use fabro_sandbox::{
|
||||
Error as SandboxError, Result as SandboxResult, Sandbox, StderrCollector, StdioProcessHandle,
|
||||
StdioProcessTermination,
|
||||
DEFAULT_EXEC_OUTPUT_TAIL_BYTES, Error as SandboxError, Result as SandboxResult, Sandbox,
|
||||
StderrCollector, StdioProcessHandle, StdioProcessTermination,
|
||||
};
|
||||
use fabro_types::CommandTermination;
|
||||
use fabro_types::{CommandTermination, ExecOutputTail};
|
||||
use futures::io::BufReader;
|
||||
use futures::sink::unfold;
|
||||
use futures::{AsyncBufReadExt, AsyncWriteExt, Stream};
|
||||
|
|
@ -21,6 +21,7 @@ use tokio::time::timeout;
|
|||
use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
|
||||
|
||||
use crate::command::AcpCommand;
|
||||
use crate::error::AcpProcessExit;
|
||||
|
||||
const CLEAN_EXIT_PROTOCOL_GRACE: Duration = Duration::from_millis(500);
|
||||
|
||||
|
|
@ -29,6 +30,7 @@ pub(crate) struct TransportState {
|
|||
handle: Arc<TokioMutex<Option<StdioProcessHandle>>>,
|
||||
stderr: Arc<TokioMutex<Option<StderrCollector>>>,
|
||||
startup_error: Arc<TokioMutex<Option<SandboxError>>>,
|
||||
process_exit: Arc<TokioMutex<Option<AcpProcessExit>>>,
|
||||
}
|
||||
|
||||
impl TransportState {
|
||||
|
|
@ -37,6 +39,7 @@ impl TransportState {
|
|||
handle: Arc::new(TokioMutex::new(None)),
|
||||
stderr: Arc::new(TokioMutex::new(None)),
|
||||
startup_error: Arc::new(TokioMutex::new(None)),
|
||||
process_exit: Arc::new(TokioMutex::new(None)),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -49,10 +52,22 @@ impl TransportState {
|
|||
*self.startup_error.lock().await = Some(error);
|
||||
}
|
||||
|
||||
async fn set_process_exit(&self, termination: StdioProcessTermination, stderr: &str) {
|
||||
*self.process_exit.lock().await = Some(AcpProcessExit {
|
||||
termination: termination.termination,
|
||||
exit_code: termination.exit_code,
|
||||
exec_output_tail: redacted_stderr_tail(stderr),
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) async fn take_startup_error(&self) -> Option<SandboxError> {
|
||||
self.startup_error.lock().await.take()
|
||||
}
|
||||
|
||||
pub(crate) async fn take_process_exit(&self) -> Option<AcpProcessExit> {
|
||||
self.process_exit.lock().await.take()
|
||||
}
|
||||
|
||||
pub(crate) async fn terminate(&self) -> SandboxResult<()> {
|
||||
if let Some(handle) = self.handle.lock().await.as_ref().cloned() {
|
||||
handle.terminate().await?;
|
||||
|
|
@ -66,6 +81,11 @@ impl TransportState {
|
|||
}
|
||||
String::new()
|
||||
}
|
||||
|
||||
pub(crate) async fn exec_output_tail(&self) -> Option<ExecOutputTail> {
|
||||
let stderr = self.stderr_tail().await;
|
||||
redacted_stderr_tail(&stderr)
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) struct SandboxAcpTransport {
|
||||
|
|
@ -156,21 +176,17 @@ impl ConnectTo<Client> for SandboxAcpTransport {
|
|||
return result;
|
||||
}
|
||||
}
|
||||
Err(process_exited_before_protocol_completed(termination, &stderr))
|
||||
self.state.set_process_exit(termination, &stderr).await;
|
||||
Err(process_exited_before_protocol_completed())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn process_exited_before_protocol_completed(
|
||||
termination: StdioProcessTermination,
|
||||
stderr: &str,
|
||||
) -> ProtocolError {
|
||||
let exit_code = termination
|
||||
.exit_code
|
||||
.map_or_else(|| "unknown".to_string(), |code| code.to_string());
|
||||
internal_error(format!(
|
||||
"ACP process exited before protocol completed: termination={}, exit_code={exit_code}, stderr={stderr}",
|
||||
termination.termination,
|
||||
))
|
||||
fn redacted_stderr_tail(stderr: &str) -> Option<ExecOutputTail> {
|
||||
fabro_sandbox::redacted_output_tail("", stderr, DEFAULT_EXEC_OUTPUT_TAIL_BYTES)
|
||||
}
|
||||
|
||||
fn process_exited_before_protocol_completed() -> ProtocolError {
|
||||
internal_error("ACP process exited before protocol completed")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -370,7 +370,7 @@ async fn timeout_terminates_process_and_returns_timeout() {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn malformed_json_returns_protocol_error() {
|
||||
async fn malformed_json_returns_diagnostic_without_raw_stderr_in_message() {
|
||||
let tempdir = tempfile::tempdir().expect("create tempdir");
|
||||
|
||||
let err = run_fake_agent(
|
||||
|
|
@ -382,11 +382,33 @@ async fn malformed_json_returns_protocol_error() {
|
|||
.await
|
||||
.expect_err("malformed JSON should error");
|
||||
|
||||
assert!(matches!(err, AcpError::Protocol(_)));
|
||||
match err {
|
||||
AcpError::Protocol(error) => {
|
||||
let message = error.to_string();
|
||||
assert!(
|
||||
!message.contains("malformed json"),
|
||||
"protocol error display should not include raw stderr: {message}"
|
||||
);
|
||||
}
|
||||
AcpError::ProcessExited(exit) => {
|
||||
let message = exit.to_string();
|
||||
assert!(
|
||||
!message.contains("malformed json"),
|
||||
"process exit display should not include raw stderr: {message}"
|
||||
);
|
||||
assert_eq!(
|
||||
exit.exec_output_tail
|
||||
.as_ref()
|
||||
.and_then(|tail| tail.stderr.as_deref()),
|
||||
Some("malformed json\n")
|
||||
);
|
||||
}
|
||||
other => panic!("expected protocol or process exit error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn early_exit_returns_protocol_error_with_stderr() {
|
||||
async fn early_exit_returns_process_exit_with_redacted_stderr_tail() {
|
||||
let tempdir = tempfile::tempdir().expect("create tempdir");
|
||||
|
||||
let err = run_fake_agent(
|
||||
|
|
@ -398,17 +420,23 @@ async fn early_exit_returns_protocol_error_with_stderr() {
|
|||
.await
|
||||
.expect_err("early exit should error");
|
||||
|
||||
let AcpError::Protocol(error) = err else {
|
||||
panic!("expected protocol error");
|
||||
let AcpError::ProcessExited(exit) = err else {
|
||||
panic!("expected process exit error");
|
||||
};
|
||||
let message = error.to_string();
|
||||
let message = exit.to_string();
|
||||
assert!(
|
||||
message.contains("exit_code=2"),
|
||||
"early exit should include exit code in diagnostic: {message}"
|
||||
);
|
||||
assert!(
|
||||
message.contains("early boom"),
|
||||
"early exit should include stderr tail in diagnostic: {message}"
|
||||
!message.contains("early boom"),
|
||||
"early exit display should not include raw stderr: {message}"
|
||||
);
|
||||
assert_eq!(
|
||||
exit.exec_output_tail
|
||||
.as_ref()
|
||||
.and_then(|tail| tail.stderr.as_deref()),
|
||||
Some("early boom\n")
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -191,6 +191,7 @@ fn main() {
|
|||
("FailureReason", "fabro_types::status::FailureReason", &[]),
|
||||
("FailureCategory", "fabro_types::FailureCategory", &[]),
|
||||
("FailureSignature", "fabro_types::FailureSignature", &[]),
|
||||
("FailureDetail", "fabro_types::FailureDetail", &[]),
|
||||
("RunFailure", "fabro_types::RunFailure", &[]),
|
||||
("BlockedReason", "fabro_types::status::BlockedReason", &[]),
|
||||
(
|
||||
|
|
|
|||
|
|
@ -33,17 +33,17 @@ pub mod types {
|
|||
};
|
||||
pub use fabro_types::{
|
||||
AuthMethod, BilledTokenCounts, CommandTermination, Conclusion, DiffStats, DiffSummary,
|
||||
DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, FailureSignature, GitContext,
|
||||
IdpIdentity, InterviewOption, InterviewQuestionRecord, PendingInterviewRecord,
|
||||
PreRunPushOutcome, Principal, PullRequest, PullRequestDetails, PullRequestDetailsStatus,
|
||||
PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse,
|
||||
QuestionType, RepositoryRef, Run, RunClientProvenance, RunEvent, RunFailure, RunProjection,
|
||||
RunProvenance, RunSandbox, RunSandboxRuntime, RunServerProvenance, SandboxDetails,
|
||||
SandboxNetwork, SandboxNetworkPolicy, SandboxNetworkPolicyMode, SandboxProvider,
|
||||
SandboxResources, SandboxService, SandboxServiceListResponse, SandboxState,
|
||||
SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, StageCompletion,
|
||||
StageHandler, StageOutcome, StageProjection, StageState, SystemActorKind, UserPrincipal,
|
||||
WorkflowSettings,
|
||||
DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, FailureDetail,
|
||||
FailureSignature, GitContext, IdpIdentity, InterviewOption, InterviewQuestionRecord,
|
||||
PendingInterviewRecord, PreRunPushOutcome, Principal, PullRequest, PullRequestDetails,
|
||||
PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, PullRequestLink,
|
||||
PullRequestMeta, PullRequestResponse, QuestionType, RepositoryRef, Run,
|
||||
RunClientProvenance, RunEvent, RunFailure, RunProjection, RunProvenance, RunSandbox,
|
||||
RunSandboxRuntime, RunServerProvenance, SandboxDetails, SandboxNetwork,
|
||||
SandboxNetworkPolicy, SandboxNetworkPolicyMode, SandboxProvider, SandboxResources,
|
||||
SandboxService, SandboxServiceListResponse, SandboxState, SandboxTimestamps,
|
||||
SecretMetadata, SecretType, ServerSettings, StageCompletion, StageHandler, StageOutcome,
|
||||
StageProjection, StageState, SystemActorKind, UserPrincipal, WorkflowSettings,
|
||||
};
|
||||
|
||||
pub use crate::generated::types::*;
|
||||
|
|
|
|||
|
|
@ -2,12 +2,12 @@ use std::any::{TypeId, type_name};
|
|||
|
||||
use fabro_api::types::{
|
||||
Conclusion as ApiConclusion, ExecOutputTail as ApiExecOutputTail,
|
||||
FailureCategory as ApiFailureCategory, FailureSignature as ApiFailureSignature,
|
||||
RunFailure as ApiRunFailure,
|
||||
FailureCategory as ApiFailureCategory, FailureDetail as ApiFailureDetail,
|
||||
FailureSignature as ApiFailureSignature, RunFailure as ApiRunFailure,
|
||||
};
|
||||
use fabro_types::{
|
||||
Conclusion, ExecOutputTail, FailureCategory, FailureReason, FailureSignature, RunFailure,
|
||||
StageOutcome,
|
||||
Conclusion, ExecOutputTail, FailureCategory, FailureDetail, FailureReason, FailureSignature,
|
||||
RunFailure, StageOutcome,
|
||||
};
|
||||
use serde::Serialize;
|
||||
use serde_json::{Value, json};
|
||||
|
|
@ -16,6 +16,7 @@ use serde_json::{Value, json};
|
|||
fn run_failure_family_reuses_domain_types() {
|
||||
assert_same_type::<ApiConclusion, Conclusion>();
|
||||
assert_same_type::<ApiRunFailure, RunFailure>();
|
||||
assert_same_type::<ApiFailureDetail, FailureDetail>();
|
||||
assert_same_type::<ApiFailureCategory, FailureCategory>();
|
||||
assert_same_type::<ApiFailureSignature, FailureSignature>();
|
||||
assert_same_type::<ApiExecOutputTail, ExecOutputTail>();
|
||||
|
|
@ -25,28 +26,35 @@ fn run_failure_family_reuses_domain_types() {
|
|||
fn run_failure_json_matches_openapi_shape() {
|
||||
assert_json(
|
||||
RunFailure {
|
||||
message: "Failed to initialize sandbox".to_string(),
|
||||
causes: vec!["connection refused".to_string()],
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
category: FailureCategory::TransientInfra,
|
||||
system_actor: None,
|
||||
signature: Some(FailureSignature("init|transient_infra|docker".to_string())),
|
||||
exec_output_tail: Some(ExecOutputTail {
|
||||
stdout: None,
|
||||
stderr: Some("last stderr line".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
}),
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
detail: {
|
||||
let mut detail = FailureDetail::new(
|
||||
"Failed to initialize sandbox",
|
||||
FailureCategory::TransientInfra,
|
||||
);
|
||||
detail.causes = vec!["connection refused".to_string()];
|
||||
detail.signature =
|
||||
Some(FailureSignature("init|transient_infra|docker".to_string()));
|
||||
detail.exec_output_tail = Some(ExecOutputTail {
|
||||
stdout: None,
|
||||
stderr: Some("last stderr line".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
});
|
||||
detail
|
||||
},
|
||||
},
|
||||
json!({
|
||||
"message": "Failed to initialize sandbox",
|
||||
"causes": ["connection refused"],
|
||||
"reason": "sandbox_init_failed",
|
||||
"category": "transient_infra",
|
||||
"signature": "init|transient_infra|docker",
|
||||
"exec_output_tail": {
|
||||
"stderr": "last stderr line",
|
||||
"stderr_truncated": true
|
||||
"detail": {
|
||||
"message": "Failed to initialize sandbox",
|
||||
"causes": ["connection refused"],
|
||||
"category": "transient_infra",
|
||||
"signature": "init|transient_infra|docker",
|
||||
"exec_output_tail": {
|
||||
"stderr": "last stderr line",
|
||||
"stderr_truncated": true
|
||||
}
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
|
@ -64,13 +72,8 @@ fn conclusion_json_uses_failure_object() {
|
|||
},
|
||||
duration_ms: 42,
|
||||
failure: Some(RunFailure {
|
||||
message: "boom".to_string(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: FailureDetail::new("boom", FailureCategory::Deterministic),
|
||||
}),
|
||||
final_git_commit_sha: None,
|
||||
stages: Vec::new(),
|
||||
|
|
@ -83,9 +86,11 @@ fn conclusion_json_uses_failure_object() {
|
|||
"status": "failed",
|
||||
"duration_ms": 42,
|
||||
"failure": {
|
||||
"message": "boom",
|
||||
"reason": "workflow_error",
|
||||
"category": "deterministic"
|
||||
"detail": {
|
||||
"message": "boom",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"total_retries": 0,
|
||||
"diff": {}
|
||||
|
|
|
|||
|
|
@ -455,7 +455,7 @@ fn format_event_pretty_value(envelope: &serde_json::Value, styles: &Styles) -> O
|
|||
}
|
||||
"run.failed" => {
|
||||
let error = prop_field(envelope, "failure")
|
||||
.and_then(|failure| failure.get("message"))
|
||||
.and_then(failure_message)
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.unwrap_or("unknown error");
|
||||
Some(format!(
|
||||
|
|
@ -809,6 +809,13 @@ fn prop_str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str
|
|||
prop_field(value, key)?.as_str()
|
||||
}
|
||||
|
||||
fn failure_message(failure: &serde_json::Value) -> Option<&serde_json::Value> {
|
||||
failure
|
||||
.get("detail")
|
||||
.and_then(|detail| detail.get("message"))
|
||||
.or_else(|| failure.get("message"))
|
||||
}
|
||||
|
||||
fn format_pull_request_record_event(
|
||||
envelope: &serde_json::Value,
|
||||
styles: &Styles,
|
||||
|
|
@ -1313,7 +1320,7 @@ mod tests {
|
|||
#[test]
|
||||
fn pretty_workflow_run_failed() {
|
||||
let styles = no_color_styles();
|
||||
let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.failed","properties":{"failure":{"message":"sandbox timeout","reason":"workflow_error","category":"deterministic"}}}"#;
|
||||
let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.failed","properties":{"failure":{"reason":"workflow_error","detail":{"message":"sandbox timeout","category":"deterministic"}}}}"#;
|
||||
let result = format_event_pretty(line, &styles).unwrap();
|
||||
assert!(result.contains("Failed"), "got: {result}");
|
||||
assert!(result.contains("sandbox timeout"), "got: {result}");
|
||||
|
|
|
|||
|
|
@ -244,7 +244,7 @@ pub(crate) fn print_run_conclusion(
|
|||
}
|
||||
|
||||
if let Some(ref failure) = conclusion.failure {
|
||||
let rendered = render_with_causes(&failure.message, &failure.causes);
|
||||
let rendered = render_with_causes(&failure.detail.message, &failure.detail.causes);
|
||||
fabro_util::printerr!(printer, "Failure: {}", styles.red.apply_to(rendered));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ use std::convert::TryFrom;
|
|||
|
||||
use chrono::{DateTime, Utc};
|
||||
use fabro_types::{BilledModelUsage, EventBody, RunEvent};
|
||||
use fabro_util::error;
|
||||
use fabro_workflow::event::RunNoticeLevel;
|
||||
use serde_json::Value;
|
||||
|
||||
|
|
@ -361,7 +362,7 @@ pub(super) fn from_run_event(stored: &RunEvent) -> Option<ProgressEvent> {
|
|||
name: node_label,
|
||||
error: props.failure.as_ref().map_or_else(
|
||||
|| "unknown error".to_string(),
|
||||
|failure| failure.message.clone(),
|
||||
|failure| error::render_compact_with_causes(&failure.message, &failure.causes),
|
||||
),
|
||||
}),
|
||||
EventBody::StageRetrying(props) => Some(ProgressEvent::StageRetrying {
|
||||
|
|
|
|||
|
|
@ -659,8 +659,8 @@ mod tests {
|
|||
RunFailedProps, RunStatusTransitionProps,
|
||||
};
|
||||
use fabro_types::{
|
||||
AuthMethod, EventBody, FailureCategory, FailureReason, IdpIdentity, Principal,
|
||||
QuestionType, RunFailure, SuccessReason, fixtures,
|
||||
AuthMethod, EventBody, FailureCategory, FailureDetail, FailureReason, IdpIdentity,
|
||||
Principal, QuestionType, RunFailure, SuccessReason, fixtures,
|
||||
};
|
||||
use fabro_vault::{SecretType, Vault};
|
||||
use fabro_workflow::event::RunEventSink;
|
||||
|
|
@ -786,13 +786,8 @@ mod tests {
|
|||
assert_eq!(
|
||||
worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps {
|
||||
failure: RunFailure {
|
||||
message: "cancelled".to_string(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::Cancelled,
|
||||
category: FailureCategory::Canceled,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::Cancelled,
|
||||
detail: FailureDetail::new("cancelled", FailureCategory::Canceled),
|
||||
},
|
||||
duration_ms: 10,
|
||||
final_git_commit_sha: None,
|
||||
|
|
@ -805,13 +800,8 @@ mod tests {
|
|||
assert_eq!(
|
||||
worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps {
|
||||
failure: RunFailure {
|
||||
message: "boom".to_string(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::Terminated,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::Terminated,
|
||||
detail: FailureDetail::new("boom", FailureCategory::Deterministic),
|
||||
},
|
||||
duration_ms: 10,
|
||||
final_git_commit_sha: None,
|
||||
|
|
|
|||
|
|
@ -135,8 +135,8 @@ fn print_human_output(
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_types::{
|
||||
BilledTokenCounts, FailureCategory, FailureReason, RunDiff, RunFailure, RunStatus,
|
||||
StageOutcome, SuccessReason, fixtures,
|
||||
BilledTokenCounts, FailureCategory, FailureDetail, FailureReason, RunDiff, RunFailure,
|
||||
RunStatus, StageOutcome, SuccessReason, fixtures,
|
||||
};
|
||||
use fabro_workflow::records::Conclusion;
|
||||
|
||||
|
|
@ -213,13 +213,8 @@ mod tests {
|
|||
},
|
||||
duration_ms: 500,
|
||||
failure: Some(RunFailure {
|
||||
message: "error".into(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: FailureDetail::new("error", FailureCategory::Deterministic),
|
||||
}),
|
||||
final_git_commit_sha: None,
|
||||
stages: vec![],
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use std::fmt;
|
||||
|
||||
use fabro_types::SystemActorKind;
|
||||
|
||||
use crate::outcome::{FailureCategory, FailureDetail, Outcome, OutcomeMeta, StageOutcome};
|
||||
use crate::outcome::{FailureDetail, Outcome, OutcomeMeta, StageOutcome};
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum VisitLimitSource {
|
||||
|
|
@ -24,16 +22,13 @@ impl fmt::Display for VisitLimitSource {
|
|||
/// to_fail_outcome().
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct HandlerErrorDetail {
|
||||
pub message: String,
|
||||
pub retryable: bool,
|
||||
pub category: Option<FailureCategory>,
|
||||
pub system_actor: Option<SystemActorKind>,
|
||||
pub signature: Option<String>,
|
||||
pub retryable: bool,
|
||||
pub failure: FailureDetail,
|
||||
}
|
||||
|
||||
impl fmt::Display for HandlerErrorDetail {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
write!(f, "{}", self.message)
|
||||
write!(f, "{}", self.failure.message)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -59,14 +54,16 @@ pub enum Error {
|
|||
#[error("stall timeout on node \"{node_id}\"")]
|
||||
StallTimeout { node_id: String },
|
||||
#[error("{detail}")]
|
||||
Handler { detail: HandlerErrorDetail },
|
||||
Handler { detail: Box<HandlerErrorDetail> },
|
||||
#[error("{0}")]
|
||||
Other(String),
|
||||
}
|
||||
|
||||
impl Error {
|
||||
pub fn handler(detail: HandlerErrorDetail) -> Self {
|
||||
Self::Handler { detail }
|
||||
Self::Handler {
|
||||
detail: Box::new(detail),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn blocked(message: impl Into<String>) -> Self {
|
||||
|
|
@ -85,12 +82,7 @@ impl Error {
|
|||
status: StageOutcome::Failed {
|
||||
retry_requested: false,
|
||||
},
|
||||
failure: Some(FailureDetail {
|
||||
message: detail.message.clone(),
|
||||
category: detail.category.unwrap_or(FailureCategory::Deterministic),
|
||||
system_actor: detail.system_actor,
|
||||
signature: detail.signature.clone(),
|
||||
}),
|
||||
failure: Some(detail.failure.clone()),
|
||||
..Outcome::default()
|
||||
},
|
||||
other => Outcome::fail(&other.to_string()),
|
||||
|
|
@ -103,6 +95,7 @@ pub type Result<T> = std::result::Result<T, Error>;
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::outcome::FailureCategory;
|
||||
|
||||
#[test]
|
||||
fn core_error_display() {
|
||||
|
|
@ -148,33 +141,27 @@ mod tests {
|
|||
#[test]
|
||||
fn core_error_handler_is_retryable() {
|
||||
let retryable = Error::handler(HandlerErrorDetail {
|
||||
message: "timeout".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: true,
|
||||
failure: FailureDetail::new("timeout", FailureCategory::TransientInfra),
|
||||
});
|
||||
assert!(retryable.is_retryable());
|
||||
|
||||
let not_retryable = Error::handler(HandlerErrorDetail {
|
||||
message: "bad input".into(),
|
||||
retryable: false,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: false,
|
||||
failure: FailureDetail::new("bad input", FailureCategory::Deterministic),
|
||||
});
|
||||
assert!(!not_retryable.is_retryable());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn core_error_handler_to_fail_outcome() {
|
||||
use crate::outcome::FailureCategory;
|
||||
let err = Error::handler(HandlerErrorDetail {
|
||||
message: "api down".into(),
|
||||
retryable: true,
|
||||
category: Some(FailureCategory::TransientInfra),
|
||||
system_actor: None,
|
||||
signature: Some("sig123".into()),
|
||||
retryable: true,
|
||||
failure: {
|
||||
let mut failure = FailureDetail::new("api down", FailureCategory::TransientInfra);
|
||||
failure.signature = Some(fabro_types::FailureSignature("sig123".into()));
|
||||
failure
|
||||
},
|
||||
});
|
||||
let outcome: Outcome = err.to_fail_outcome();
|
||||
assert_eq!(outcome.status, StageOutcome::Failed {
|
||||
|
|
@ -183,7 +170,14 @@ mod tests {
|
|||
let failure = outcome.failure.unwrap();
|
||||
assert_eq!(failure.message, "api down");
|
||||
assert_eq!(failure.category, FailureCategory::TransientInfra);
|
||||
assert_eq!(failure.signature.as_deref(), Some("sig123"));
|
||||
assert_eq!(
|
||||
failure
|
||||
.signature
|
||||
.as_ref()
|
||||
.map(ToString::to_string)
|
||||
.as_deref(),
|
||||
Some("sig123")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -448,12 +448,24 @@ mod tests {
|
|||
use crate::context::Context;
|
||||
use crate::error::HandlerErrorDetail;
|
||||
use crate::lifecycle::RunLifecycle;
|
||||
use crate::outcome::StageOutcome;
|
||||
use crate::outcome::{FailureCategory, FailureDetail, StageOutcome};
|
||||
use crate::retry::{BackoffPolicy, RetryPolicy};
|
||||
use crate::test_fixtures::*;
|
||||
|
||||
type NextNodeLog = Arc<Mutex<Vec<(String, Option<String>)>>>;
|
||||
|
||||
fn handler_error(message: &str, retryable: bool) -> HandlerErrorDetail {
|
||||
let category = if retryable {
|
||||
FailureCategory::TransientInfra
|
||||
} else {
|
||||
FailureCategory::Deterministic
|
||||
};
|
||||
HandlerErrorDetail {
|
||||
retryable,
|
||||
failure: FailureDetail::new(message, category),
|
||||
}
|
||||
}
|
||||
|
||||
// Helper to build and run an executor with default settings
|
||||
async fn run_linear(
|
||||
node_ids: &[&str],
|
||||
|
|
@ -1015,20 +1027,8 @@ mod tests {
|
|||
async fn executor_retry_on_retryable_error() {
|
||||
let handler = Arc::new(
|
||||
CountingHandler::new(vec![
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "fail1".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
})),
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "fail2".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
})),
|
||||
Err(Error::handler(handler_error("fail1", true))),
|
||||
Err(Error::handler(handler_error("fail2", true))),
|
||||
Ok(Outcome::success()),
|
||||
])
|
||||
.with_retry_policy(RetryPolicy {
|
||||
|
|
@ -1092,14 +1092,8 @@ mod tests {
|
|||
#[tokio::test]
|
||||
async fn executor_retry_non_retryable_error_no_retry() {
|
||||
let handler = Arc::new(
|
||||
CountingHandler::new(vec![Err(Error::handler(HandlerErrorDetail {
|
||||
message: "fatal".into(),
|
||||
retryable: false,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
}))])
|
||||
.with_retry_policy(RetryPolicy::with_max_attempts(3)),
|
||||
CountingHandler::new(vec![Err(Error::handler(handler_error("fatal", false)))])
|
||||
.with_retry_policy(RetryPolicy::with_max_attempts(3)),
|
||||
);
|
||||
let result = run_linear(
|
||||
&["start", "end"],
|
||||
|
|
@ -1116,13 +1110,7 @@ mod tests {
|
|||
async fn executor_retry_no_retry_by_default() {
|
||||
// Default policy is RetryPolicy::none() (max_attempts=1)
|
||||
let handler = Arc::new(CountingHandler::new(vec![Err(Error::handler(
|
||||
HandlerErrorDetail {
|
||||
message: "fail".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
},
|
||||
handler_error("fail", true),
|
||||
))]));
|
||||
let result = run_linear(
|
||||
&["start", "end"],
|
||||
|
|
@ -1239,13 +1227,7 @@ mod tests {
|
|||
}
|
||||
let handler = Arc::new(
|
||||
CountingHandler::new(vec![
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "r".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
})),
|
||||
Err(Error::handler(handler_error("r", true))),
|
||||
Ok(Outcome::success()),
|
||||
])
|
||||
.with_retry_policy(RetryPolicy {
|
||||
|
|
@ -1284,13 +1266,7 @@ mod tests {
|
|||
}
|
||||
let handler = Arc::new(
|
||||
CountingHandler::new(vec![
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "r".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
})),
|
||||
Err(Error::handler(handler_error("r", true))),
|
||||
Ok(Outcome::success()),
|
||||
])
|
||||
.with_retry_policy(RetryPolicy {
|
||||
|
|
@ -1335,13 +1311,7 @@ mod tests {
|
|||
}
|
||||
let handler = Arc::new(
|
||||
CountingHandler::new(vec![
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "r".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
})),
|
||||
Err(Error::handler(handler_error("r", true))),
|
||||
Ok(Outcome::success()), // should not be reached
|
||||
])
|
||||
.with_retry_policy(RetryPolicy {
|
||||
|
|
@ -2139,13 +2109,7 @@ mod tests {
|
|||
if c == 0 {
|
||||
// First call: fail with retryable, then cancel stall during backoff
|
||||
self.stall.cancel();
|
||||
Err(Error::handler(HandlerErrorDetail {
|
||||
message: "transient".into(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
}))
|
||||
Err(Error::handler(handler_error("transient", true)))
|
||||
} else {
|
||||
Ok(Outcome::success())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use crate::context::Context;
|
|||
use crate::error::{Error, HandlerErrorDetail, Result};
|
||||
use crate::graph::{EdgeSelection, EdgeSpec, Graph, NodeSpec};
|
||||
use crate::handler::NodeHandler;
|
||||
use crate::outcome::{Outcome, StageOutcome};
|
||||
use crate::outcome::{FailureCategory, FailureDetail, Outcome, StageOutcome};
|
||||
use crate::retry::RetryPolicy;
|
||||
|
||||
// ---- Test node ----
|
||||
|
|
@ -388,11 +388,8 @@ impl ErrorHandler {
|
|||
pub fn retryable(message: &str, policy: RetryPolicy) -> Self {
|
||||
Self {
|
||||
detail: HandlerErrorDetail {
|
||||
message: message.to_string(),
|
||||
retryable: true,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: true,
|
||||
failure: FailureDetail::new(message, FailureCategory::TransientInfra),
|
||||
},
|
||||
retry_policy: policy,
|
||||
}
|
||||
|
|
@ -401,11 +398,8 @@ impl ErrorHandler {
|
|||
pub fn non_retryable(message: &str) -> Self {
|
||||
Self {
|
||||
detail: HandlerErrorDetail {
|
||||
message: message.to_string(),
|
||||
retryable: false,
|
||||
category: None,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: false,
|
||||
failure: FailureDetail::new(message, FailureCategory::Deterministic),
|
||||
},
|
||||
retry_policy: RetryPolicy::none(),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -85,7 +85,9 @@ use fabro_types::{
|
|||
EventBody, InterviewQuestionRecord, Principal, PullRequestRecord, QuestionType, RunBlobId,
|
||||
RunControlAction, RunEvent, RunId, ServerSettings, SessionCapability,
|
||||
};
|
||||
use fabro_util::error::{SharedError, collect_causes, render_with_causes};
|
||||
use fabro_util::error::{
|
||||
SharedError, collect_causes, render_compact_with_causes, render_with_causes,
|
||||
};
|
||||
use fabro_util::version::FABRO_VERSION;
|
||||
use fabro_vault::{Error as VaultError, SecretType, Vault};
|
||||
use fabro_workflow::artifact_upload::ArtifactSink;
|
||||
|
|
@ -2413,7 +2415,10 @@ fn update_live_run_from_event(state: &AppState, run_id: RunId, event: &RunEvent)
|
|||
managed_run.status = RunStatus::Failed {
|
||||
reason: props.failure.reason,
|
||||
};
|
||||
managed_run.error = Some(props.failure.message.clone());
|
||||
managed_run.error = Some(render_compact_with_causes(
|
||||
&props.failure.detail.message,
|
||||
&props.failure.detail.causes,
|
||||
));
|
||||
managed_run.active_api_stages.clear();
|
||||
managed_run.active_non_steerable_agent_stages.clear();
|
||||
}
|
||||
|
|
@ -3362,10 +3367,9 @@ async fn execute_run_subprocess(state: Arc<AppState>, run_id: RunId) {
|
|||
.conclusion
|
||||
.as_ref()
|
||||
.and_then(|conclusion| {
|
||||
conclusion
|
||||
.failure
|
||||
.as_ref()
|
||||
.map(|failure| failure.message.clone())
|
||||
conclusion.failure.as_ref().map(|failure| {
|
||||
render_compact_with_causes(&failure.detail.message, &failure.detail.causes)
|
||||
})
|
||||
})
|
||||
.or_else(|| managed_run.error.clone());
|
||||
managed_run.checkpoint = final_state.current_checkpoint().cloned();
|
||||
|
|
|
|||
|
|
@ -353,7 +353,7 @@ async fn cancel_at_human_gate_persists_cancelled_terminal_event() {
|
|||
event["properties"]["failure"]["reason"]
|
||||
.as_str()
|
||||
.map(ToOwned::to_owned),
|
||||
event["properties"]["failure"]["message"]
|
||||
event["properties"]["failure"]["detail"]["message"]
|
||||
.as_str()
|
||||
.map(ToOwned::to_owned),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ use fabro_types::{
|
|||
RunStatus, RunSummary, RunTimestamps, SandboxProvider, StageCompletion, StageHandler, StageId,
|
||||
StageOutcome, StageProjection, StageState, StartRecord, WorkflowRef, first_event_seq,
|
||||
};
|
||||
use fabro_util::error::render_compact_with_causes;
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::{Error, EventEnvelope, Result};
|
||||
|
|
@ -856,7 +857,7 @@ fn stage_completion_from_outcome(
|
|||
failure_reason: outcome
|
||||
.failure
|
||||
.as_ref()
|
||||
.map(|failure| failure.message.clone()),
|
||||
.map(|failure| render_compact_with_causes(&failure.message, &failure.causes)),
|
||||
timestamp,
|
||||
}
|
||||
}
|
||||
|
|
@ -1600,7 +1601,7 @@ mod tests {
|
|||
"index": 0,
|
||||
"failure": {
|
||||
"message": "provider failed",
|
||||
"failure_class": "transient_infra"
|
||||
"category": "transient_infra"
|
||||
},
|
||||
"will_retry": false,
|
||||
"duration_ms": 654,
|
||||
|
|
@ -2185,13 +2186,8 @@ mod tests {
|
|||
1,
|
||||
EventBody::RunFailed(RunFailedProps {
|
||||
failure: fabro_types::RunFailure {
|
||||
message: "boom".to_string(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: FailureDetail::new("boom", FailureCategory::Deterministic),
|
||||
},
|
||||
duration_ms: 42,
|
||||
final_git_commit_sha: Some("abc123".to_string()),
|
||||
|
|
@ -2305,9 +2301,11 @@ mod tests {
|
|||
"run.failed",
|
||||
&json!({
|
||||
"failure": {
|
||||
"message": "boom",
|
||||
"reason": "workflow_error",
|
||||
"category": "deterministic"
|
||||
"detail": {
|
||||
"message": "boom",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"duration_ms": 42,
|
||||
"diff_summary": {
|
||||
|
|
@ -2337,16 +2335,18 @@ mod tests {
|
|||
1,
|
||||
EventBody::RunFailed(RunFailedProps {
|
||||
failure: fabro_types::RunFailure {
|
||||
message: "Failed to initialize sandbox".to_string(),
|
||||
causes: vec![
|
||||
"Failed to pull Docker image buildpack-deps:noble".to_string(),
|
||||
"connection refused".to_string(),
|
||||
],
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::TransientInfra,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: {
|
||||
let mut detail = FailureDetail::new(
|
||||
"Failed to initialize sandbox",
|
||||
FailureCategory::TransientInfra,
|
||||
);
|
||||
detail.causes = vec![
|
||||
"Failed to pull Docker image buildpack-deps:noble".to_string(),
|
||||
"connection refused".to_string(),
|
||||
];
|
||||
detail
|
||||
},
|
||||
},
|
||||
duration_ms: 42,
|
||||
final_git_commit_sha: None,
|
||||
|
|
@ -2359,8 +2359,8 @@ mod tests {
|
|||
.unwrap();
|
||||
|
||||
let failure = state.conclusion.unwrap().failure.unwrap();
|
||||
assert_eq!(failure.message, "Failed to initialize sandbox");
|
||||
assert_eq!(failure.causes, vec![
|
||||
assert_eq!(failure.detail.message, "Failed to initialize sandbox");
|
||||
assert_eq!(failure.detail.causes, vec![
|
||||
"Failed to pull Docker image buildpack-deps:noble".to_string(),
|
||||
"connection refused".to_string(),
|
||||
]);
|
||||
|
|
@ -2370,15 +2370,19 @@ mod tests {
|
|||
fn run_failed_projection_uses_nested_failure_reason_and_conclusion() {
|
||||
let mut state = running_projection();
|
||||
let failure = fabro_types::RunFailure {
|
||||
message: "Failed to initialize sandbox".to_string(),
|
||||
causes: vec!["connection refused".to_string()],
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
category: FailureCategory::TransientInfra,
|
||||
system_actor: Some(fabro_types::SystemActorKind::Engine),
|
||||
signature: Some(fabro_types::FailureSignature(
|
||||
"init|transient_infra|docker".to_string(),
|
||||
)),
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
detail: {
|
||||
let mut detail = FailureDetail::new(
|
||||
"Failed to initialize sandbox",
|
||||
FailureCategory::TransientInfra,
|
||||
);
|
||||
detail.causes = vec!["connection refused".to_string()];
|
||||
detail.system_actor = Some(fabro_types::SystemActorKind::Engine);
|
||||
detail.signature = Some(fabro_types::FailureSignature(
|
||||
"init|transient_infra|docker".to_string(),
|
||||
));
|
||||
detail
|
||||
},
|
||||
};
|
||||
state
|
||||
.apply_event(&test_event(
|
||||
|
|
|
|||
|
|
@ -725,9 +725,11 @@ mod tests {
|
|||
"run.failed",
|
||||
&serde_json::json!({
|
||||
"failure": {
|
||||
"message": "cancelled",
|
||||
"reason": "cancelled",
|
||||
"category": "canceled"
|
||||
"detail": {
|
||||
"message": "cancelled",
|
||||
"category": "canceled"
|
||||
}
|
||||
},
|
||||
"duration_ms": 1,
|
||||
}),
|
||||
|
|
@ -1126,9 +1128,11 @@ mod tests {
|
|||
"run.failed",
|
||||
&serde_json::json!({
|
||||
"failure": {
|
||||
"message": "workflow failed",
|
||||
"reason": "workflow_error",
|
||||
"category": "deterministic"
|
||||
"detail": {
|
||||
"message": "workflow failed",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"duration_ms": 1,
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -10,3 +10,18 @@ impl fmt::Display for FailureSignature {
|
|||
f.write_str(&self.0)
|
||||
}
|
||||
}
|
||||
|
||||
impl FailureSignature {
|
||||
#[must_use]
|
||||
pub fn as_str(&self) -> &str {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
impl std::ops::Deref for FailureSignature {
|
||||
type Target = str;
|
||||
|
||||
fn deref(&self) -> &Self::Target {
|
||||
self.as_str()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
|
|||
use serde_json::Value;
|
||||
use strum::{Display, EnumString, IntoStaticStr};
|
||||
|
||||
use crate::SystemActorKind;
|
||||
use crate::{ExecOutputTail, FailureSignature, SystemActorKind};
|
||||
|
||||
pub trait OutcomeMeta:
|
||||
Default + Clone + Send + Sync + fmt::Debug + Serialize + DeserializeOwned + 'static
|
||||
|
|
@ -229,26 +229,27 @@ impl FailureCategory {
|
|||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct FailureDetail {
|
||||
pub message: String,
|
||||
#[serde(rename = "failure_class")]
|
||||
pub category: FailureCategory,
|
||||
pub message: String,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub causes: Vec<String>,
|
||||
pub category: FailureCategory,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub system_actor: Option<SystemActorKind>,
|
||||
#[serde(
|
||||
rename = "failure_signature",
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none"
|
||||
)]
|
||||
pub signature: Option<String>,
|
||||
pub system_actor: Option<SystemActorKind>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub signature: Option<FailureSignature>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub exec_output_tail: Option<ExecOutputTail>,
|
||||
}
|
||||
|
||||
impl FailureDetail {
|
||||
pub fn new(message: impl Into<String>, category: FailureCategory) -> Self {
|
||||
Self {
|
||||
message: message.into(),
|
||||
causes: Vec::new(),
|
||||
category,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -305,10 +306,12 @@ impl<M: OutcomeMeta> Outcome<M> {
|
|||
retry_requested: false,
|
||||
},
|
||||
failure: Some(FailureDetail {
|
||||
message: message.to_string(),
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
message: message.to_string(),
|
||||
causes: Vec::new(),
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
}),
|
||||
..Self::default()
|
||||
}
|
||||
|
|
@ -327,7 +330,7 @@ impl<M: OutcomeMeta> Outcome<M> {
|
|||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use super::{StageOutcome, StageState};
|
||||
use super::{FailureCategory, FailureDetail, StageOutcome, StageState};
|
||||
|
||||
#[test]
|
||||
fn stage_outcome_failed_serde_is_lossy_for_retry_intent() {
|
||||
|
|
@ -366,6 +369,35 @@ mod tests {
|
|||
assert!(!StageState::Retrying.is_terminal());
|
||||
assert!(!StageState::Running.is_terminal());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failure_detail_serializes_rich_diagnostics() {
|
||||
use crate::{ExecOutputTail, FailureSignature};
|
||||
|
||||
let mut failure = FailureDetail::new("ACP turn failed", FailureCategory::Deterministic);
|
||||
failure.causes = vec![
|
||||
"ACP protocol error".to_string(),
|
||||
"agent exited before initialize completed".to_string(),
|
||||
];
|
||||
failure.signature = Some(FailureSignature("work|deterministic|acp".to_string()));
|
||||
failure.exec_output_tail = Some(ExecOutputTail {
|
||||
stdout: None,
|
||||
stderr: Some("redacted stderr tail".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
});
|
||||
|
||||
let value = serde_json::to_value(&failure).expect("failure detail should serialize");
|
||||
|
||||
assert_eq!(value["message"], "ACP turn failed");
|
||||
assert_eq!(value["causes"][0], "ACP protocol error");
|
||||
assert_eq!(value["category"], "deterministic");
|
||||
assert_eq!(value["signature"], "work|deterministic|acp");
|
||||
assert_eq!(value["exec_output_tail"]["stderr"], "redacted stderr tail");
|
||||
assert_eq!(value["exec_output_tail"]["stderr_truncated"], true);
|
||||
assert!(value.get("failure_class").is_none());
|
||||
assert!(value.get("failure_signature").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
|
|
|
|||
|
|
@ -1095,9 +1095,11 @@ mod tests {
|
|||
"run.failed",
|
||||
json!({
|
||||
"failure": {
|
||||
"message": "boom",
|
||||
"reason": "workflow_error",
|
||||
"category": "deterministic"
|
||||
"detail": {
|
||||
"message": "boom",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"duration_ms": 42,
|
||||
"diff_summary": {
|
||||
|
|
|
|||
|
|
@ -1,18 +1,41 @@
|
|||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::{ExecOutputTail, FailureCategory, FailureReason, FailureSignature, SystemActorKind};
|
||||
use crate::{FailureDetail, FailureReason};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct RunFailure {
|
||||
pub message: String,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub causes: Vec<String>,
|
||||
pub reason: FailureReason,
|
||||
pub category: FailureCategory,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub system_actor: Option<SystemActorKind>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub signature: Option<FailureSignature>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub exec_output_tail: Option<ExecOutputTail>,
|
||||
pub reason: FailureReason,
|
||||
pub detail: FailureDetail,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use crate::{FailureCategory, FailureDetail, FailureReason, RunFailure};
|
||||
|
||||
#[test]
|
||||
fn run_failure_wraps_failure_detail_with_reason() {
|
||||
let detail = FailureDetail::new("ACP turn failed", FailureCategory::Deterministic);
|
||||
let failure = RunFailure {
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail,
|
||||
};
|
||||
|
||||
let value = serde_json::to_value(&failure).expect("run failure should serialize");
|
||||
|
||||
assert_eq!(value["reason"], "workflow_error");
|
||||
assert_eq!(value["detail"]["message"], "ACP turn failed");
|
||||
assert_eq!(value["detail"]["category"], "deterministic");
|
||||
assert_eq!(
|
||||
value,
|
||||
json!({
|
||||
"reason": "workflow_error",
|
||||
"detail": {
|
||||
"message": "ACP turn failed",
|
||||
"category": "deterministic"
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use fabro_types::run_event::run::RunFailedProps;
|
||||
use fabro_types::{
|
||||
Conclusion, EventBody, ExecOutputTail, FailureCategory, FailureReason, FailureSignature,
|
||||
RunDiff, RunFailure, StageOutcome, SystemActorKind,
|
||||
Conclusion, EventBody, ExecOutputTail, FailureCategory, FailureDetail, FailureReason,
|
||||
FailureSignature, RunDiff, RunFailure, StageOutcome, SystemActorKind,
|
||||
};
|
||||
use serde_json::json;
|
||||
|
||||
|
|
@ -9,23 +9,28 @@ use serde_json::json;
|
|||
fn run_failed_serializes_nested_failure_contract() {
|
||||
let body = EventBody::RunFailed(RunFailedProps {
|
||||
failure: RunFailure {
|
||||
message: "Failed to initialize sandbox".to_string(),
|
||||
causes: vec![
|
||||
"Failed to pull Docker image buildpack-deps:noble".to_string(),
|
||||
"connection refused".to_string(),
|
||||
],
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
category: FailureCategory::TransientInfra,
|
||||
system_actor: Some(SystemActorKind::Engine),
|
||||
signature: Some(FailureSignature(
|
||||
"init|transient_infra|docker-pull".to_string(),
|
||||
)),
|
||||
exec_output_tail: Some(ExecOutputTail {
|
||||
stdout: Some("last stdout line".to_string()),
|
||||
stderr: Some("last stderr line".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
}),
|
||||
reason: FailureReason::SandboxInitFailed,
|
||||
detail: {
|
||||
let mut detail = FailureDetail::new(
|
||||
"Failed to initialize sandbox",
|
||||
FailureCategory::TransientInfra,
|
||||
);
|
||||
detail.causes = vec![
|
||||
"Failed to pull Docker image buildpack-deps:noble".to_string(),
|
||||
"connection refused".to_string(),
|
||||
];
|
||||
detail.system_actor = Some(SystemActorKind::Engine);
|
||||
detail.signature = Some(FailureSignature(
|
||||
"init|transient_infra|docker-pull".to_string(),
|
||||
));
|
||||
detail.exec_output_tail = Some(ExecOutputTail {
|
||||
stdout: Some("last stdout line".to_string()),
|
||||
stderr: Some("last stderr line".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
});
|
||||
detail
|
||||
},
|
||||
},
|
||||
duration_ms: 42,
|
||||
final_git_commit_sha: Some("abc123".to_string()),
|
||||
|
|
@ -41,19 +46,21 @@ fn run_failed_serializes_nested_failure_contract() {
|
|||
value["properties"],
|
||||
json!({
|
||||
"failure": {
|
||||
"message": "Failed to initialize sandbox",
|
||||
"causes": [
|
||||
"Failed to pull Docker image buildpack-deps:noble",
|
||||
"connection refused"
|
||||
],
|
||||
"reason": "sandbox_init_failed",
|
||||
"category": "transient_infra",
|
||||
"system_actor": "engine",
|
||||
"signature": "init|transient_infra|docker-pull",
|
||||
"exec_output_tail": {
|
||||
"stdout": "last stdout line",
|
||||
"stderr": "last stderr line",
|
||||
"stderr_truncated": true
|
||||
"detail": {
|
||||
"message": "Failed to initialize sandbox",
|
||||
"causes": [
|
||||
"Failed to pull Docker image buildpack-deps:noble",
|
||||
"connection refused"
|
||||
],
|
||||
"category": "transient_infra",
|
||||
"system_actor": "engine",
|
||||
"signature": "init|transient_infra|docker-pull",
|
||||
"exec_output_tail": {
|
||||
"stdout": "last stdout line",
|
||||
"stderr": "last stderr line",
|
||||
"stderr_truncated": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"duration_ms": 42,
|
||||
|
|
@ -71,13 +78,8 @@ fn run_failed_serializes_nested_failure_contract() {
|
|||
fn run_failed_omits_empty_failure_optional_fields() {
|
||||
let body = EventBody::RunFailed(RunFailedProps {
|
||||
failure: RunFailure {
|
||||
message: "boom".to_string(),
|
||||
causes: Vec::new(),
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: FailureDetail::new("boom", FailureCategory::Deterministic),
|
||||
},
|
||||
duration_ms: 1,
|
||||
final_git_commit_sha: None,
|
||||
|
|
@ -92,9 +94,11 @@ fn run_failed_omits_empty_failure_optional_fields() {
|
|||
value["properties"],
|
||||
json!({
|
||||
"failure": {
|
||||
"message": "boom",
|
||||
"reason": "workflow_error",
|
||||
"category": "deterministic"
|
||||
"detail": {
|
||||
"message": "boom",
|
||||
"category": "deterministic"
|
||||
}
|
||||
},
|
||||
"duration_ms": 1
|
||||
})
|
||||
|
|
@ -112,13 +116,12 @@ fn conclusion_serializes_rich_failure() {
|
|||
},
|
||||
duration_ms: 42,
|
||||
failure: Some(RunFailure {
|
||||
message: "run failed".to_string(),
|
||||
causes: vec!["leaf cause".to_string()],
|
||||
reason: FailureReason::WorkflowError,
|
||||
category: FailureCategory::Deterministic,
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
exec_output_tail: None,
|
||||
reason: FailureReason::WorkflowError,
|
||||
detail: {
|
||||
let mut detail = FailureDetail::new("run failed", FailureCategory::Deterministic);
|
||||
detail.causes = vec!["leaf cause".to_string()];
|
||||
detail
|
||||
},
|
||||
}),
|
||||
final_git_commit_sha: None,
|
||||
stages: Vec::new(),
|
||||
|
|
@ -129,7 +132,7 @@ fn conclusion_serializes_rich_failure() {
|
|||
|
||||
let value = serde_json::to_value(&conclusion).expect("conclusion should serialize");
|
||||
|
||||
assert_eq!(value["failure"]["message"], "run failed");
|
||||
assert_eq!(value["failure"]["causes"], json!(["leaf cause"]));
|
||||
assert_eq!(value["failure"]["detail"]["message"], "run failed");
|
||||
assert_eq!(value["failure"]["detail"]["causes"], json!(["leaf cause"]));
|
||||
assert!(value.get("failure_reason").is_none());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,9 +57,20 @@ pub fn render_with_causes(message: &str, causes: &[String]) -> String {
|
|||
rendered
|
||||
}
|
||||
|
||||
pub fn render_compact_with_causes(message: &str, causes: &[String]) -> String {
|
||||
let Some(cause) = causes.first() else {
|
||||
return message.to_string();
|
||||
};
|
||||
if cause == message {
|
||||
message.to_string()
|
||||
} else {
|
||||
format!("{message}: {cause}")
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::SharedError;
|
||||
use super::{SharedError, render_compact_with_causes};
|
||||
|
||||
#[test]
|
||||
fn shared_error_preserves_chain_without_duplicating_top_level() {
|
||||
|
|
@ -92,4 +103,22 @@ mod tests {
|
|||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compact_cause_rendering_adds_first_cause_without_multiline_noise() {
|
||||
assert_eq!(
|
||||
render_compact_with_causes("Failed to initialize sandbox", &[
|
||||
"connection refused".to_string()
|
||||
]),
|
||||
"Failed to initialize sandbox: connection refused"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compact_cause_rendering_deduplicates_matching_cause() {
|
||||
assert_eq!(
|
||||
render_compact_with_causes("boom", &["boom".to_string()]),
|
||||
"boom"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use fabro_graphviz::Error as GraphvizError;
|
|||
use fabro_llm::{Error as LlmError, ProviderErrorKind};
|
||||
pub use fabro_types::failure_signature::FailureSignature;
|
||||
pub use fabro_types::outcome::FailureCategory;
|
||||
use fabro_types::{FailureReason, RunFailure};
|
||||
use fabro_types::{ExecOutputTail, FailureReason, RunFailure};
|
||||
use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes};
|
||||
use fabro_validate::Diagnostic;
|
||||
use thiserror::Error as ThisError;
|
||||
|
|
@ -207,18 +207,20 @@ pub enum Error {
|
|||
|
||||
#[error("Engine error: {message}")]
|
||||
Engine {
|
||||
message: String,
|
||||
failure_class: FailureCategory,
|
||||
message: String,
|
||||
failure_class: FailureCategory,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
#[source]
|
||||
source: Option<SharedError>,
|
||||
source: Option<SharedError>,
|
||||
},
|
||||
|
||||
#[error("Handler error: {message}")]
|
||||
Handler {
|
||||
message: String,
|
||||
failure_class: FailureCategory,
|
||||
message: String,
|
||||
failure_class: FailureCategory,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
#[source]
|
||||
source: Option<SharedError>,
|
||||
source: Option<SharedError>,
|
||||
},
|
||||
|
||||
#[error("LLM error: {0}")]
|
||||
|
|
@ -255,6 +257,21 @@ impl Error {
|
|||
Self::Handler {
|
||||
message,
|
||||
failure_class,
|
||||
exec_output_tail: None,
|
||||
source: None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn handler_with_exec_output_tail(
|
||||
message: impl Into<String>,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
) -> Self {
|
||||
let message = message.into();
|
||||
let failure_class = classify_failure_reason(&message);
|
||||
Self::Handler {
|
||||
message,
|
||||
failure_class,
|
||||
exec_output_tail,
|
||||
source: None,
|
||||
}
|
||||
}
|
||||
|
|
@ -262,6 +279,14 @@ impl Error {
|
|||
pub fn handler_with_source(
|
||||
message: impl Into<String>,
|
||||
source: impl Into<anyhow::Error>,
|
||||
) -> Self {
|
||||
Self::handler_with_source_and_exec_output_tail(message, source, None)
|
||||
}
|
||||
|
||||
pub fn handler_with_source_and_exec_output_tail(
|
||||
message: impl Into<String>,
|
||||
source: impl Into<anyhow::Error>,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
) -> Self {
|
||||
let message = message.into();
|
||||
let source = SharedError::new(source.into());
|
||||
|
|
@ -271,6 +296,7 @@ impl Error {
|
|||
Self::Handler {
|
||||
message,
|
||||
failure_class,
|
||||
exec_output_tail,
|
||||
source: Some(source),
|
||||
}
|
||||
}
|
||||
|
|
@ -287,6 +313,7 @@ impl Error {
|
|||
Self::Engine {
|
||||
message,
|
||||
failure_class,
|
||||
exec_output_tail: None,
|
||||
source: None,
|
||||
}
|
||||
}
|
||||
|
|
@ -303,6 +330,7 @@ impl Error {
|
|||
Self::Engine {
|
||||
message,
|
||||
failure_class,
|
||||
exec_output_tail: None,
|
||||
source: Some(source),
|
||||
}
|
||||
}
|
||||
|
|
@ -374,21 +402,42 @@ impl Error {
|
|||
/// Return a stable failure signature hint when structured error info is
|
||||
/// available.
|
||||
#[must_use]
|
||||
pub fn failure_signature_hint(&self) -> Option<String> {
|
||||
pub fn failure_signature_hint(&self) -> Option<FailureSignature> {
|
||||
match self {
|
||||
Self::Llm(sdk_err) => Some(sdk_err.failure_signature_hint()),
|
||||
Self::Llm(sdk_err) => Some(FailureSignature(sdk_err.failure_signature_hint())),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn to_failure_detail(&self) -> FailureDetail {
|
||||
let message = match self {
|
||||
Self::Engine { message, .. } | Self::Handler { message, .. } => message.clone(),
|
||||
_ => self.to_string(),
|
||||
};
|
||||
let explicit_exec_output_tail = match self {
|
||||
Self::Engine {
|
||||
exec_output_tail, ..
|
||||
}
|
||||
| Self::Handler {
|
||||
exec_output_tail, ..
|
||||
} => exec_output_tail.clone(),
|
||||
_ => None,
|
||||
};
|
||||
FailureDetail {
|
||||
message,
|
||||
causes: self.causes(),
|
||||
category: self.failure_category(),
|
||||
system_actor: None,
|
||||
signature: self.failure_signature_hint(),
|
||||
exec_output_tail: explicit_exec_output_tail
|
||||
.or_else(|| fabro_sandbox::default_redacted_output_tail(self)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Build a fail `Outcome` with structured `FailureDetail`.
|
||||
pub fn to_fail_outcome(&self) -> Outcome {
|
||||
let failure = FailureDetail {
|
||||
message: self.display_with_causes(),
|
||||
category: self.failure_category(),
|
||||
system_actor: None,
|
||||
signature: self.failure_signature_hint(),
|
||||
};
|
||||
let failure = self.to_failure_detail();
|
||||
Outcome {
|
||||
status: StageOutcome::Failed {
|
||||
retry_requested: false,
|
||||
|
|
@ -401,18 +450,9 @@ impl Error {
|
|||
|
||||
#[must_use]
|
||||
pub fn run_failure_from_error(error: &Error, reason: FailureReason) -> RunFailure {
|
||||
let message = match error {
|
||||
Error::Engine { message, .. } | Error::Handler { message, .. } => message.clone(),
|
||||
_ => error.to_string(),
|
||||
};
|
||||
RunFailure {
|
||||
message,
|
||||
causes: error.causes(),
|
||||
reason,
|
||||
category: error.failure_category(),
|
||||
system_actor: None,
|
||||
signature: error.failure_signature_hint().map(FailureSignature),
|
||||
exec_output_tail: fabro_sandbox::default_redacted_output_tail(error),
|
||||
detail: error.to_failure_detail(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -422,13 +462,8 @@ pub fn run_failure_from_outcome_failure(
|
|||
reason: FailureReason,
|
||||
) -> RunFailure {
|
||||
RunFailure {
|
||||
message: failure.message.clone(),
|
||||
causes: Vec::new(),
|
||||
reason,
|
||||
category: failure.category,
|
||||
system_actor: failure.system_actor,
|
||||
signature: failure.signature.clone().map(FailureSignature),
|
||||
exec_output_tail: None,
|
||||
detail: failure.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1663,7 +1698,9 @@ mod tests {
|
|||
});
|
||||
assert_eq!(
|
||||
err.failure_signature_hint(),
|
||||
Some("api_deterministic|openai|authentication".to_string())
|
||||
Some(FailureSignature(
|
||||
"api_deterministic|openai|authentication".to_string()
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -1912,10 +1949,10 @@ mod tests {
|
|||
let err = Error::handler("connection refused");
|
||||
let failure = run_failure_from_error(&err, FailureReason::WorkflowError);
|
||||
|
||||
assert_eq!(failure.message, "connection refused");
|
||||
assert_eq!(failure.causes, Vec::<String>::new());
|
||||
assert_eq!(failure.detail.message, "connection refused");
|
||||
assert_eq!(failure.detail.causes, Vec::<String>::new());
|
||||
assert_eq!(failure.reason, FailureReason::WorkflowError);
|
||||
assert_eq!(failure.category, FailureCategory::TransientInfra);
|
||||
assert_eq!(failure.detail.category, FailureCategory::TransientInfra);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -1442,7 +1442,7 @@ mod tests {
|
|||
assert_eq!(stored.event_name(), "stage.failed");
|
||||
let properties = stored.properties().unwrap();
|
||||
assert_eq!(properties["failure"]["message"], "lint failed");
|
||||
assert_eq!(properties["failure"]["failure_class"], "deterministic");
|
||||
assert_eq!(properties["failure"]["category"], "deterministic");
|
||||
assert_eq!(properties["will_retry"], true);
|
||||
assert_eq!(properties["billing"], serde_json::to_value(&usage).unwrap());
|
||||
}
|
||||
|
|
@ -1550,7 +1550,7 @@ mod tests {
|
|||
|
||||
assert_eq!(stored.event_name(), "run.failed");
|
||||
let properties = stored.properties().unwrap();
|
||||
assert_eq!(properties["failure"]["message"], "boom");
|
||||
assert_eq!(properties["failure"]["detail"]["message"], "boom");
|
||||
assert_eq!(properties["duration_ms"], 900);
|
||||
}
|
||||
|
||||
|
|
@ -1570,11 +1570,11 @@ mod tests {
|
|||
|
||||
let properties = stored.properties().unwrap();
|
||||
assert_eq!(
|
||||
properties["failure"]["message"],
|
||||
properties["failure"]["detail"]["message"],
|
||||
"Failed to initialize sandbox"
|
||||
);
|
||||
assert_eq!(
|
||||
properties["failure"]["causes"],
|
||||
properties["failure"]["detail"]["causes"],
|
||||
serde_json::json!(["connection refused"])
|
||||
);
|
||||
}
|
||||
|
|
@ -1596,15 +1596,18 @@ mod tests {
|
|||
assert_eq!(stored.event_name(), "run.failed");
|
||||
let properties = stored.properties().unwrap();
|
||||
assert_eq!(
|
||||
properties["failure"]["message"],
|
||||
properties["failure"]["detail"]["message"],
|
||||
"Failed to initialize sandbox"
|
||||
);
|
||||
assert_eq!(
|
||||
properties["failure"]["causes"],
|
||||
properties["failure"]["detail"]["causes"],
|
||||
serde_json::json!(["connection refused"])
|
||||
);
|
||||
assert_eq!(properties["failure"]["reason"], "sandbox_init_failed");
|
||||
assert_eq!(properties["failure"]["category"], "transient_infra");
|
||||
assert_eq!(
|
||||
properties["failure"]["detail"]["category"],
|
||||
"transient_infra"
|
||||
);
|
||||
assert_eq!(properties["duration_ms"], 900);
|
||||
assert_eq!(properties["final_git_commit_sha"], "abc123");
|
||||
assert!(properties.get("error").is_none());
|
||||
|
|
|
|||
|
|
@ -826,15 +826,16 @@ impl Event {
|
|||
duration_ms,
|
||||
..
|
||||
} => {
|
||||
let detail = &failure.detail;
|
||||
let tail =
|
||||
fabro_types::ExecOutputTail::trace_summary(failure.exec_output_tail.as_ref());
|
||||
fabro_types::ExecOutputTail::trace_summary(detail.exec_output_tail.as_ref());
|
||||
error!(
|
||||
message = %failure.message,
|
||||
message = %detail.message,
|
||||
reason = %failure.reason,
|
||||
category = %failure.category,
|
||||
system_actor = ?failure.system_actor,
|
||||
signature = ?failure.signature,
|
||||
cause_count = failure.causes.len(),
|
||||
category = %detail.category,
|
||||
system_actor = ?detail.system_actor,
|
||||
signature = ?detail.signature,
|
||||
cause_count = detail.causes.len(),
|
||||
exec_output_tail_present = tail.present,
|
||||
exec_stdout_tail_bytes = tail.stdout_bytes,
|
||||
exec_stderr_tail_bytes = tail.stderr_bytes,
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ pub enum CodergenResult {
|
|||
files_touched: Vec<String>,
|
||||
last_file_touched: Option<String>,
|
||||
},
|
||||
Full(Outcome),
|
||||
Full(Box<Outcome>),
|
||||
}
|
||||
|
||||
pub struct CodergenRunRequest<'a> {
|
||||
|
|
@ -313,7 +313,7 @@ impl Handler for AgentHandler {
|
|||
})
|
||||
.await;
|
||||
match result {
|
||||
Ok(CodergenResult::Full(outcome)) => return Ok(outcome),
|
||||
Ok(CodergenResult::Full(outcome)) => return Ok(*outcome),
|
||||
Ok(CodergenResult::Text {
|
||||
text,
|
||||
usage,
|
||||
|
|
|
|||
|
|
@ -188,7 +188,11 @@ impl AgentAcpBackend {
|
|||
);
|
||||
return Err(Error::Cancelled);
|
||||
}
|
||||
Err(AcpError::TimedOut { stderr }) => {
|
||||
Err(AcpError::TimedOut { exec_output_tail }) => {
|
||||
let stderr = exec_output_tail
|
||||
.as_ref()
|
||||
.and_then(|tail| tail.stderr.clone())
|
||||
.unwrap_or_default();
|
||||
emitter.emit_scoped(
|
||||
&Event::AgentAcpTimedOut {
|
||||
node_id: node.id.clone(),
|
||||
|
|
@ -198,7 +202,9 @@ impl AgentAcpBackend {
|
|||
},
|
||||
stage_scope,
|
||||
);
|
||||
return Err(acp_error_to_workflow(AcpError::TimedOut { stderr }));
|
||||
return Err(acp_error_to_workflow(AcpError::TimedOut {
|
||||
exec_output_tail,
|
||||
}));
|
||||
}
|
||||
Err(AcpError::StopReason { stop_reason, text }) => {
|
||||
emitter.emit_scoped(
|
||||
|
|
@ -285,18 +291,21 @@ fn acp_command_error_to_workflow(error: AcpCommandError) -> Error {
|
|||
fn acp_error_to_workflow(error: AcpError) -> Error {
|
||||
match error {
|
||||
AcpError::Cancelled => Error::Cancelled,
|
||||
AcpError::TimedOut { stderr } => {
|
||||
if stderr.is_empty() {
|
||||
Error::handler("ACP turn timed out")
|
||||
} else {
|
||||
Error::handler(format!("ACP turn timed out: {stderr}"))
|
||||
}
|
||||
AcpError::TimedOut { exec_output_tail } => {
|
||||
Error::handler_with_exec_output_tail("ACP turn timed out", exec_output_tail)
|
||||
}
|
||||
AcpError::StopReason { stop_reason, text } => {
|
||||
Error::handler(format!("ACP prompt stopped with {stop_reason}: {text}"))
|
||||
}
|
||||
AcpError::Sandbox(source) => Error::handler_with_source("ACP turn failed", source),
|
||||
other => Error::handler_with_source("ACP turn failed", other),
|
||||
other => {
|
||||
let exec_output_tail = other.exec_output_tail();
|
||||
Error::handler_with_source_and_exec_output_tail(
|
||||
"ACP turn failed",
|
||||
other,
|
||||
exec_output_tail,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -306,14 +315,15 @@ mod tests {
|
|||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use fabro_acp::test_support::fake_acp_agent_script;
|
||||
use fabro_acp::{AcpError, AcpProcessExit};
|
||||
use fabro_agent::{LocalSandbox, Sandbox, shell_quote};
|
||||
use fabro_graphviz::graph::{AttrValue, Node};
|
||||
use fabro_model::ProviderId;
|
||||
use fabro_sandbox::test_support::MockSandbox;
|
||||
use fabro_types::EventBody;
|
||||
use fabro_types::{CommandTermination, EventBody, ExecOutputTail};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use super::AgentAcpBackend;
|
||||
use super::{AgentAcpBackend, acp_error_to_workflow};
|
||||
use crate::context::Context;
|
||||
use crate::event::{Emitter, StageScope};
|
||||
use crate::handler::agent::{
|
||||
|
|
@ -660,6 +670,59 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acp_timeout_maps_stderr_to_exec_tail_not_message() {
|
||||
let tail = ExecOutputTail {
|
||||
stdout: None,
|
||||
stderr: Some("redacted stderr tail".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: true,
|
||||
};
|
||||
let err = acp_error_to_workflow(AcpError::TimedOut {
|
||||
exec_output_tail: Some(tail.clone()),
|
||||
});
|
||||
|
||||
let detail = err.to_failure_detail();
|
||||
assert_eq!(detail.message, "ACP turn timed out");
|
||||
assert!(detail.causes.is_empty());
|
||||
assert_eq!(detail.exec_output_tail, Some(tail));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acp_process_exit_maps_stderr_to_exec_tail_not_cause_text() {
|
||||
let tail = ExecOutputTail {
|
||||
stdout: None,
|
||||
stderr: Some("early boom".to_string()),
|
||||
stdout_truncated: false,
|
||||
stderr_truncated: false,
|
||||
};
|
||||
let err = acp_error_to_workflow(AcpError::ProcessExited(AcpProcessExit {
|
||||
termination: CommandTermination::Exited,
|
||||
exit_code: Some(2),
|
||||
exec_output_tail: Some(tail.clone()),
|
||||
}));
|
||||
|
||||
let detail = err.to_failure_detail();
|
||||
assert_eq!(detail.message, "ACP turn failed");
|
||||
assert_eq!(detail.exec_output_tail, Some(tail));
|
||||
assert!(
|
||||
detail
|
||||
.causes
|
||||
.iter()
|
||||
.any(|cause| cause.contains("exit_code=2")),
|
||||
"cause chain should retain process exit context: {:?}",
|
||||
detail.causes
|
||||
);
|
||||
assert!(
|
||||
!detail
|
||||
.causes
|
||||
.iter()
|
||||
.any(|cause| cause.contains("early boom")),
|
||||
"raw stderr belongs in exec_output_tail, not causes: {:?}",
|
||||
detail.causes
|
||||
);
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "unit test initializes an isolated git repository with the system git binary"
|
||||
|
|
|
|||
|
|
@ -139,7 +139,7 @@ impl Handler for PromptHandler {
|
|||
})
|
||||
.await;
|
||||
match result {
|
||||
Ok(CodergenResult::Full(outcome)) => return Ok(outcome),
|
||||
Ok(CodergenResult::Full(outcome)) => return Ok(*outcome),
|
||||
Ok(CodergenResult::Text {
|
||||
text,
|
||||
usage,
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ use crate::context::Context;
|
|||
use crate::error::Error;
|
||||
use crate::graph::{WorkflowGraph, WorkflowNode};
|
||||
use crate::handler::{EngineServices, dispatch_handler, format_panic_message};
|
||||
use crate::outcome::{Outcome, StageOutcome};
|
||||
use crate::outcome::{FailureDetail, Outcome, StageOutcome};
|
||||
use crate::retry::build_retry_policy;
|
||||
|
||||
/// Production node handler that bridges fabro-core's NodeHandler to the
|
||||
|
|
@ -51,11 +51,8 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
|
|||
.await
|
||||
.map_err(|err| {
|
||||
CoreError::handler(HandlerErrorDetail {
|
||||
message: err.to_string(),
|
||||
retryable: true,
|
||||
category: Some(FailureCategory::TransientInfra),
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: true,
|
||||
failure: err.to_failure_detail(),
|
||||
})
|
||||
})?;
|
||||
let execution_snapshot = wf_context.snapshot();
|
||||
|
|
@ -79,12 +76,14 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
|
|||
match timeout(duration, panic_safe).await {
|
||||
Ok(inner) => inner,
|
||||
Err(_elapsed) => {
|
||||
let mut failure = FailureDetail::new(
|
||||
format!("handler timed out after {}ms", duration.as_millis()),
|
||||
FailureCategory::TransientInfra,
|
||||
);
|
||||
failure.system_actor = Some(SystemActorKind::Timeout);
|
||||
return Err(CoreError::handler(HandlerErrorDetail {
|
||||
message: format!("handler timed out after {}ms", duration.as_millis()),
|
||||
retryable: true,
|
||||
category: Some(FailureCategory::TransientInfra),
|
||||
system_actor: Some(SystemActorKind::Timeout),
|
||||
signature: None,
|
||||
retryable: true,
|
||||
failure,
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
|
@ -108,21 +107,15 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
|
|||
Ok(Err(fabro_err)) => {
|
||||
let retryable = handler.should_retry(&fabro_err);
|
||||
Err(CoreError::handler(HandlerErrorDetail {
|
||||
message: fabro_err.to_string(),
|
||||
retryable,
|
||||
category: Some(fabro_err.failure_category()),
|
||||
system_actor: None,
|
||||
signature: fabro_err.failure_signature_hint(),
|
||||
failure: fabro_err.to_failure_detail(),
|
||||
}))
|
||||
}
|
||||
Err(panic_payload) => {
|
||||
let msg = format_panic_message(&panic_payload);
|
||||
Err(CoreError::handler(HandlerErrorDetail {
|
||||
message: msg,
|
||||
retryable: false,
|
||||
category: Some(FailureCategory::Deterministic),
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: false,
|
||||
failure: FailureDetail::new(msg, FailureCategory::Deterministic),
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
|
@ -137,11 +130,8 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
|
|||
.await
|
||||
.map_err(|err| {
|
||||
CoreError::handler(HandlerErrorDetail {
|
||||
message: err.to_string(),
|
||||
retryable: true,
|
||||
category: Some(FailureCategory::TransientInfra),
|
||||
system_actor: None,
|
||||
signature: None,
|
||||
retryable: true,
|
||||
failure: err.to_failure_detail(),
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use fabro_model::{
|
|||
};
|
||||
pub use fabro_types::BilledModelUsage;
|
||||
|
||||
use crate::error::{Error, classify_failure_reason};
|
||||
use crate::error::{Error, FailureSignature, classify_failure_reason};
|
||||
|
||||
pub type Outcome = fabro_core::Outcome<Option<BilledModelUsage>>;
|
||||
|
||||
|
|
@ -109,7 +109,7 @@ impl OutcomeExt for Outcome {
|
|||
|
||||
fn with_signature(mut self, sig: Option<impl Into<String>>) -> Self {
|
||||
if let Some(ref mut failure) = self.failure {
|
||||
failure.signature = sig.map(Into::into);
|
||||
failure.signature = sig.map(|sig| FailureSignature(sig.into()));
|
||||
}
|
||||
self
|
||||
}
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ models/execute-query-request.ts
|
|||
models/execute-query-response-rows-inner-inner.ts
|
||||
models/execute-query-response.ts
|
||||
models/failure-category.ts
|
||||
models/failure-detail.ts
|
||||
models/failure-reason.ts
|
||||
models/features-namespace.ts
|
||||
models/file-checkpoint.ts
|
||||
|
|
|
|||
42
lib/packages/fabro-api-client/src/models/failure-detail.ts
Normal file
42
lib/packages/fabro-api-client/src/models/failure-detail.ts
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
/* tslint:disable */
|
||||
/* eslint-disable */
|
||||
/**
|
||||
* Fabro Run API
|
||||
* HTTP API for managing Fabro workflow run executions.
|
||||
*
|
||||
* The version of the OpenAPI document: 0.1.0
|
||||
*
|
||||
*
|
||||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
|
||||
* https://openapi-generator.tech
|
||||
* Do not edit the class manually.
|
||||
*/
|
||||
|
||||
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { ExecOutputTail } from './exec-output-tail';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { FailureCategory } from './failure-category';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { SystemActorKind } from './system-actor-kind';
|
||||
|
||||
/**
|
||||
* Rich diagnostic detail for a failed stage or terminal run.
|
||||
*/
|
||||
export interface FailureDetail {
|
||||
'message': string;
|
||||
'causes'?: Array<string>;
|
||||
'category': FailureCategory;
|
||||
'system_actor'?: SystemActorKind | null;
|
||||
/**
|
||||
* Stable normalized signature for grouping related failures.
|
||||
*/
|
||||
'signature'?: string | null;
|
||||
'exec_output_tail'?: ExecOutputTail | null;
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
|
@ -78,6 +78,7 @@ export * from './execute-query-request';
|
|||
export * from './execute-query-response';
|
||||
export * from './execute-query-response-rows-inner-inner';
|
||||
export * from './failure-category';
|
||||
export * from './failure-detail';
|
||||
export * from './failure-reason';
|
||||
export * from './features-namespace';
|
||||
export * from './file-checkpoint';
|
||||
|
|
|
|||
|
|
@ -15,31 +15,17 @@
|
|||
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { ExecOutputTail } from './exec-output-tail';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { FailureCategory } from './failure-category';
|
||||
import type { FailureDetail } from './failure-detail';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { FailureReason } from './failure-reason';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { SystemActorKind } from './system-actor-kind';
|
||||
|
||||
/**
|
||||
* Rich terminal run failure diagnostics.
|
||||
* Terminal run failure reason and rich diagnostics.
|
||||
*/
|
||||
export interface RunFailure {
|
||||
'message': string;
|
||||
'causes'?: Array<string>;
|
||||
'reason': FailureReason;
|
||||
'category': FailureCategory;
|
||||
'system_actor'?: SystemActorKind | null;
|
||||
/**
|
||||
* Stable normalized signature for grouping related failures.
|
||||
*/
|
||||
'signature'?: string | null;
|
||||
'exec_output_tail'?: ExecOutputTail | null;
|
||||
'detail': FailureDetail;
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue