fix(core): Add structured error handling and responsive lock release
Improves core infrastructure with robot-friendly error output and faster lock release for better sync behavior. Error handling improvements (error.rs): - ErrorCode::exit_code(): Unique exit codes per error type (1-13) for programmatic error handling in scripts/agents - GiError::suggestion(): Helpful hints for common error recovery - GiError::to_robot_error(): Structured JSON error conversion - RobotError/RobotErrorOutput: Serializable error types with code, message, and optional suggestion fields Lock improvements (lock.rs): - Heartbeat thread now polls every 100ms for release flag, only updating database heartbeat at full interval (5s default) - Eliminates 5-10s delay after sync completion when waiting for heartbeat thread to notice release - Reduces lock hold time after operation completes Database (db.rs): - Bump expected schema version to 6 for MR migration The exit code mapping enables shell scripts and CI/CD pipelines to distinguish between configuration errors (2-4), GitLab API errors (5-8), and database errors (9-11) for appropriate retry/alert logic. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -15,8 +15,18 @@ const MIGRATIONS: &[(&str, &str)] = &[
|
|||||||
("001", include_str!("../../migrations/001_initial.sql")),
|
("001", include_str!("../../migrations/001_initial.sql")),
|
||||||
("002", include_str!("../../migrations/002_issues.sql")),
|
("002", include_str!("../../migrations/002_issues.sql")),
|
||||||
("003", include_str!("../../migrations/003_indexes.sql")),
|
("003", include_str!("../../migrations/003_indexes.sql")),
|
||||||
("004", include_str!("../../migrations/004_discussions_payload.sql")),
|
(
|
||||||
("005", include_str!("../../migrations/005_assignees_milestone_duedate.sql")),
|
"004",
|
||||||
|
include_str!("../../migrations/004_discussions_payload.sql"),
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"005",
|
||||||
|
include_str!("../../migrations/005_assignees_milestone_duedate.sql"),
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"006",
|
||||||
|
include_str!("../../migrations/006_merge_requests.sql"),
|
||||||
|
),
|
||||||
];
|
];
|
||||||
|
|
||||||
/// Create a database connection with production-grade pragmas.
|
/// Create a database connection with production-grade pragmas.
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
//!
|
//!
|
||||||
//! Uses thiserror for ergonomic error definitions with structured error codes.
|
//! Uses thiserror for ergonomic error definitions with structured error codes.
|
||||||
|
|
||||||
|
use serde::Serialize;
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
|
|
||||||
/// Error codes for programmatic error handling.
|
/// Error codes for programmatic error handling.
|
||||||
@@ -43,6 +44,27 @@ impl std::fmt::Display for ErrorCode {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl ErrorCode {
|
||||||
|
/// Get the exit code for this error (for robot mode).
|
||||||
|
pub fn exit_code(&self) -> i32 {
|
||||||
|
match self {
|
||||||
|
Self::InternalError => 1,
|
||||||
|
Self::ConfigNotFound => 2,
|
||||||
|
Self::ConfigInvalid => 3,
|
||||||
|
Self::TokenNotSet => 4,
|
||||||
|
Self::GitLabAuthFailed => 5,
|
||||||
|
Self::GitLabNotFound => 6,
|
||||||
|
Self::GitLabRateLimited => 7,
|
||||||
|
Self::GitLabNetworkError => 8,
|
||||||
|
Self::DatabaseLocked => 9,
|
||||||
|
Self::DatabaseError => 10,
|
||||||
|
Self::MigrationFailed => 11,
|
||||||
|
Self::IoError => 12,
|
||||||
|
Self::TransformError => 13,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Main error type for gitlab-inbox.
|
/// Main error type for gitlab-inbox.
|
||||||
#[derive(Error, Debug)]
|
#[derive(Error, Debug)]
|
||||||
pub enum GiError {
|
pub enum GiError {
|
||||||
@@ -132,6 +154,63 @@ impl GiError {
|
|||||||
Self::Other(_) => ErrorCode::InternalError,
|
Self::Other(_) => ErrorCode::InternalError,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Get a suggestion for how to fix this error.
|
||||||
|
pub fn suggestion(&self) -> Option<&'static str> {
|
||||||
|
match self {
|
||||||
|
Self::ConfigNotFound { .. } => Some("Run 'gi init' to create configuration"),
|
||||||
|
Self::ConfigInvalid { .. } => Some("Check config file syntax or run 'gi init' to recreate"),
|
||||||
|
Self::GitLabAuthFailed => Some("Verify token has read_api scope and is not expired"),
|
||||||
|
Self::GitLabNotFound { .. } => Some("Check the resource path exists and you have access"),
|
||||||
|
Self::GitLabRateLimited { .. } => Some("Wait and retry, or reduce request frequency"),
|
||||||
|
Self::GitLabNetworkError { .. } => Some("Check network connection and GitLab URL"),
|
||||||
|
Self::DatabaseLocked { .. } => Some("Wait for other sync to complete or use --force"),
|
||||||
|
Self::MigrationFailed { .. } => Some("Check database file permissions or reset with 'gi reset'"),
|
||||||
|
Self::TokenNotSet { .. } => Some("Export the token environment variable"),
|
||||||
|
Self::Database(_) => Some("Check database file permissions or reset with 'gi reset'"),
|
||||||
|
Self::Http(_) => Some("Check network connection"),
|
||||||
|
Self::NotFound(_) => Some("Verify the entity exists using 'gi list'"),
|
||||||
|
Self::Ambiguous(_) => Some("Use --project flag to disambiguate"),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Get the exit code for this error.
|
||||||
|
pub fn exit_code(&self) -> i32 {
|
||||||
|
self.code().exit_code()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Convert to robot-mode JSON error output.
|
||||||
|
pub fn to_robot_error(&self) -> RobotError {
|
||||||
|
RobotError {
|
||||||
|
code: self.code().to_string(),
|
||||||
|
message: self.to_string(),
|
||||||
|
suggestion: self.suggestion().map(String::from),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Structured error for robot mode JSON output.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct RobotError {
|
||||||
|
pub code: String,
|
||||||
|
pub message: String,
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub suggestion: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Wrapper for robot mode error output.
|
||||||
|
#[derive(Debug, Serialize)]
|
||||||
|
pub struct RobotErrorOutput {
|
||||||
|
pub error: RobotError,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl From<&GiError> for RobotErrorOutput {
|
||||||
|
fn from(e: &GiError) -> Self {
|
||||||
|
Self {
|
||||||
|
error: e.to_robot_error(),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub type Result<T> = std::result::Result<T, GiError>;
|
pub type Result<T> = std::result::Result<T, GiError>;
|
||||||
|
|||||||
@@ -42,10 +42,7 @@ pub struct AppLock {
|
|||||||
impl AppLock {
|
impl AppLock {
|
||||||
/// Create a new app lock instance.
|
/// Create a new app lock instance.
|
||||||
pub fn new(conn: Connection, options: LockOptions) -> Self {
|
pub fn new(conn: Connection, options: LockOptions) -> Self {
|
||||||
let db_path = conn
|
let db_path = conn.path().map(PathBuf::from).unwrap_or_default();
|
||||||
.path()
|
|
||||||
.map(PathBuf::from)
|
|
||||||
.unwrap_or_default();
|
|
||||||
|
|
||||||
Self {
|
Self {
|
||||||
conn,
|
conn,
|
||||||
@@ -73,7 +70,9 @@ impl AppLock {
|
|||||||
let now = now_ms();
|
let now = now_ms();
|
||||||
|
|
||||||
// Use IMMEDIATE transaction to prevent race conditions
|
// Use IMMEDIATE transaction to prevent race conditions
|
||||||
let tx = self.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
|
let tx = self
|
||||||
|
.conn
|
||||||
|
.transaction_with_behavior(TransactionBehavior::Immediate)?;
|
||||||
|
|
||||||
// Check for existing lock within the transaction
|
// Check for existing lock within the transaction
|
||||||
let existing: Option<(String, i64, i64)> = tx
|
let existing: Option<(String, i64, i64)> = tx
|
||||||
@@ -176,9 +175,21 @@ impl AppLock {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
loop {
|
// Poll frequently for early exit, but only update heartbeat at full interval
|
||||||
thread::sleep(interval);
|
const POLL_INTERVAL: Duration = Duration::from_millis(100);
|
||||||
|
|
||||||
|
loop {
|
||||||
|
// Sleep in small increments, checking released flag frequently
|
||||||
|
let mut elapsed = Duration::ZERO;
|
||||||
|
while elapsed < interval {
|
||||||
|
thread::sleep(POLL_INTERVAL);
|
||||||
|
elapsed += POLL_INTERVAL;
|
||||||
|
if released.load(Ordering::SeqCst) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check once more after full interval elapsed
|
||||||
if released.load(Ordering::SeqCst) {
|
if released.load(Ordering::SeqCst) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user