Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cli/src/services/app_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ where
W: Write,
{
if let Some(log) = logger {
log.log_classified_error(error, None);
log.log_classified_error(error, None, error.user_facing_presentation().is_none());
}
write_error_diagnostic(stderr, error);
ExitCode::from(error.class().exit_code())
Expand All @@ -159,6 +159,12 @@ fn write_stdout_payload<W: Write>(writer: &mut W, payload: &str) -> Result<(), C
}

fn write_error_diagnostic<W: Write>(writer: &mut W, error: &ClassifiedError) {
if let Some(presentation) = error.user_facing_presentation() {
let message = services::security::redact_sensitive_text(presentation.message());
writeln!(writer, "{message}").expect("writing error diagnostic to writer should not fail");
return;
}

let rendered = if error.message().contains("Try:") {
error.message().to_string()
} else {
Expand Down
50 changes: 50 additions & 0 deletions cli/src/services/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,42 @@ impl FailureClass {
}
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct UserFacingPresentation {
message: String,
reason: Option<String>,
}

impl UserFacingPresentation {
pub fn new(message: impl Into<String>) -> Self {
Self {
message: message.into(),
reason: None,
}
}

#[allow(dead_code)]
pub fn with_reason(mut self, reason: impl Into<String>) -> Self {
self.reason = Some(reason.into());
self
}

pub fn message(&self) -> &str {
&self.message
}

#[allow(dead_code)]
pub fn reason(&self) -> Option<&str> {
self.reason.as_deref()
}
}

#[derive(Debug)]
pub struct ClassifiedError {
class: FailureClass,
code: &'static str,
message: String,
user_facing_presentation: Option<UserFacingPresentation>,
}

impl ClassifiedError {
Expand All @@ -52,6 +83,7 @@ impl ClassifiedError {
class: FailureClass::Parse,
code: "SCE-ERR-PARSE",
message: message.into(),
user_facing_presentation: None,
}
}

Expand All @@ -60,6 +92,7 @@ impl ClassifiedError {
class: FailureClass::Validation,
code: "SCE-ERR-VALIDATION",
message: message.into(),
user_facing_presentation: None,
}
}

Expand All @@ -68,6 +101,7 @@ impl ClassifiedError {
class: FailureClass::Runtime,
code: "SCE-ERR-RUNTIME",
message: message.into(),
user_facing_presentation: None,
}
}

Expand All @@ -76,9 +110,21 @@ impl ClassifiedError {
class: FailureClass::Dependency,
code: "SCE-ERR-DEPENDENCY",
message: message.into(),
user_facing_presentation: None,
}
}

#[allow(dead_code)]
pub fn with_user_facing_message(mut self, message: impl Into<String>) -> Self {
self.user_facing_presentation = Some(UserFacingPresentation::new(message));
self
}

pub fn with_user_facing_presentation(mut self, presentation: UserFacingPresentation) -> Self {
self.user_facing_presentation = Some(presentation);
self
}

pub fn class(&self) -> FailureClass {
self.class
}
Expand All @@ -90,6 +136,10 @@ impl ClassifiedError {
pub fn message(&self) -> &str {
&self.message
}

pub fn user_facing_presentation(&self) -> Option<&UserFacingPresentation> {
self.user_facing_presentation.as_ref()
}
}

impl std::fmt::Display for ClassifiedError {
Expand Down
32 changes: 16 additions & 16 deletions cli/src/services/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl Logger {
fields: &[(&str, &str)],
session_id: Option<&str>,
) {
self.log_forced(LogLevel::Warn, event_id, message, fields, session_id);
self.log_forced(LogLevel::Warn, event_id, message, fields, session_id, true);
}

#[cfg_attr(not(test), allow(dead_code))]
Expand All @@ -136,9 +136,14 @@ impl Logger {
self.log(LogLevel::Error, event_id, message, fields, session_id);
}

pub fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>) {
pub(crate) fn log_classified_error(
&self,
error: &ClassifiedError,
session_id: Option<&str>,
emit_stderr: bool,
) {
let event_id = format!("sce.error.{}", error.code());
self.log(
self.log_forced(
LogLevel::Error,
&event_id,
error.message(),
Expand All @@ -147,6 +152,7 @@ impl Logger {
("error_class", error.class().as_str()),
],
session_id,
emit_stderr,
);
}

Expand All @@ -162,7 +168,7 @@ impl Logger {
return;
}

self.log_forced(level, event_id, message, fields, session_id);
self.log_forced(level, event_id, message, fields, session_id, true);
}

fn log_forced(
Expand All @@ -172,19 +178,18 @@ impl Logger {
message: &str,
fields: &[(&str, &str)],
session_id: Option<&str>,
emit_stderr: bool,
) {
emit_tracing_event(level, event_id, message, fields);

let line = self.render_line(level, event_id, message, fields);
let redacted_line = redact_sensitive_text(&line);
emit_stderr_line(&redacted_line);

if let Err(error) = self.write_log_line(&redacted_line, session_id) {
let diagnostic = redact_sensitive_text(&format!(
"Failed to write SCE log file: {error}. Logging continues on stderr."
));
emit_stderr_line(&diagnostic);
if emit_stderr {
emit_stderr_line(&redacted_line);
}

let _ = self.write_log_line(&redacted_line, session_id);
}

fn write_log_line(&self, redacted_line: &str, session_id: Option<&str>) -> Result<()> {
Expand Down Expand Up @@ -388,12 +393,7 @@ where
{
if write_target == LogWriteTarget::Created {
if let Some(parent) = path.parent() {
if let Err(error) = cleanup(parent) {
let diagnostic = redact_sensitive_text(&format!(
"Failed to clean up SCE log files: {error}. Logging continues on stderr."
));
emit_stderr_line(&diagnostic);
}
let _ = cleanup(parent);
}
}
}
Expand Down
24 changes: 20 additions & 4 deletions cli/src/services/observability/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ pub trait Logger: Send + Sync {
session_id: Option<&str>,
);

fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>);
fn log_classified_error(
&self,
error: &ClassifiedError,
session_id: Option<&str>,
emit_stderr: bool,
);
}

pub trait Telemetry: Send + Sync {
Expand Down Expand Up @@ -84,7 +89,13 @@ impl Logger for NoopLogger {
) {
}

fn log_classified_error(&self, _error: &ClassifiedError, _session_id: Option<&str>) {}
fn log_classified_error(
&self,
_error: &ClassifiedError,
_session_id: Option<&str>,
_emit_stderr: bool,
) {
}
}

impl Logger for super::Logger {
Expand Down Expand Up @@ -128,8 +139,13 @@ impl Logger for super::Logger {
super::Logger::error(self, event_id, message, fields, session_id);
}

fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>) {
super::Logger::log_classified_error(self, error, session_id);
fn log_classified_error(
&self,
error: &ClassifiedError,
session_id: Option<&str>,
emit_stderr: bool,
) {
super::Logger::log_classified_error(self, error, session_id, emit_stderr);
}
}

Expand Down
20 changes: 18 additions & 2 deletions cli/src/services/sync/command.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::io::Write;

use crate::app::ContextWithRepoRoot;
use crate::services::error::ClassifiedError;
use crate::services::agent_trace_sync::control_plane::ControlPlaneError;
use crate::services::error::{ClassifiedError, UserFacingPresentation};
use crate::services::sync::progress::{
IndicatifProgressReporter, NoopProgressReporter, ProgressReporter,
};
Expand Down Expand Up @@ -31,7 +32,22 @@ where

#[allow(clippy::needless_pass_by_value)]
fn classify_sync_error(err: TraceSyncError) -> ClassifiedError {
ClassifiedError::runtime(format!("{err}"))
let is_unauthenticated = matches!(
&err,
TraceSyncError::ControlPlane(
ControlPlaneError::MissingCredentials | ControlPlaneError::AuthenticationFailed(_)
)
);
let classified = ClassifiedError::runtime(format!("{err}"));

if is_unauthenticated {
classified.with_user_facing_presentation(UserFacingPresentation::new(format!(
"You are not logged in. Please log in using the {} command.",
crate::services::style::success("sce auth login")
)))
} else {
classified
}
}

impl SyncCommand {
Expand Down
4 changes: 2 additions & 2 deletions context/architecture.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading