Normalize Error Messages (#3776)

* Lowercase all error messages, remove trailing punctuation, update style guide

* Capitalize proper names, fix API errors.

CODE_STYLE.md

@@ -109,6 +109,16 @@ These assert will not be part of the release binary and won't hurt the execution
 
 **example needed**
 
+## Errors
+
+Error messages should be concise, lowercase (except proper names), and without trailing punctuation.
+
+### Examples
+- "failed to start actor runtimes"
+- "cannot join PostgreSQL URI {} with path {:?}"
+- "could not find split metadata in Metastore {}"
+- "unkown output format {:?}"
+
 ## Comments
 
 We use on the same code style, [rustc's doc comments](https://doc.rust-lang.org/1.0.0/style/style/comments.html).

quickwit/quickwit-actors/src/actor.rs

@@ -44,21 +44,21 @@ pub enum ActorExitStatus {
     ///
     /// (This is equivalent to exit status code 0.)
     /// Note that this is not really an error.
-    #[error("Success")]
+    #[error("success")]
     Success,
 
     /// The actor was asked to gracefully shutdown.
     ///
     /// (Semantically equivalent to exit status code 130, triggered by SIGINT aka Ctrl-C, or
     /// SIGQUIT)
-    #[error("Quit")]
+    #[error("quit")]
     Quit,
 
     /// The actor tried to send a message to a dowstream actor and failed.
     /// The logic ruled that the actor should be killed.
     ///
     /// (Semantically equivalent to exit status code 141, triggered by SIGPIPE)
-    #[error("Downstream actor exited.")]
+    #[error("downstream actor exited")]
     DownstreamClosed,
 
     /// The actor was killed.
@@ -68,15 +68,15 @@ pub enum ActorExitStatus {
     /// - its kill switch was activated.
     ///
     /// (Semantically equivalent to exit status code 137, triggered by SIGKILL)
-    #[error("Killed")]
+    #[error("killed")]
     Killed,
 
     /// An unexpected error happened while processing a message.
-    #[error("Failure(cause={0:?})")]
+    #[error("failure(cause={0:?})")]
     Failure(Arc<anyhow::Error>),
 
     /// The thread or the task executing the actor loop panicked.
-    #[error("Panicked")]
+    #[error("panicked")]
     Panicked,
 }
 

quickwit/quickwit-actors/src/channel_with_priority.rs

@@ -64,17 +64,17 @@ impl<T> LockedOption<T> {
 
 #[derive(Debug, Error)]
 pub enum SendError {
-    #[error("The channel is closed.")]
+    #[error("the channel is closed")]
     Disconnected,
-    #[error("The channel is full.")]
+    #[error("the channel is full")]
     Full,
 }
 
 #[derive(Debug, Error)]
 pub enum TrySendError<M> {
-    #[error("The channel is closed.")]
+    #[error("the channel is closed")]
     Disconnected,
-    #[error("The channel is full.")]
+    #[error("the channel is full")]
     Full(M),
 }
 
@@ -89,9 +89,9 @@ impl<M> From<flume::TrySendError<M>> for TrySendError<M> {
 
 #[derive(Clone, Copy, Debug, Error, Eq, PartialEq)]
 pub enum RecvError {
-    #[error("No message are currently available.")]
+    #[error("no message are currently available")]
     NoMessageAvailable,
-    #[error("All sender were dropped and no pending messages are in the channel.")]
+    #[error("all senders were dropped and no pending messages are in the channel")]
     Disconnected,
 }
 

quickwit/quickwit-actors/src/lib.rs

@@ -121,10 +121,10 @@ const OBSERVE_TIMEOUT: Duration = Duration::from_secs(3);
 /// Error that occurred while calling `ActorContext::ask(..)` or `Universe::ask`
 #[derive(Error, Debug)]
 pub enum AskError<E: fmt::Debug> {
-    #[error("Message could not be delivered")]
+    #[error("message could not be delivered")]
     MessageNotDelivered,
-    #[error("Error while the message was being processed.")]
+    #[error("error while the message was being processed")]
     ProcessMessageError,
-    #[error("The handler returned an error: `{0:?}`.")]
+    #[error("the handler returned an error: `{0:?}`")]
     ErrorReply(#[from] E),
 }

quickwit/quickwit-actors/src/spawn_builder.rs

@@ -306,9 +306,9 @@ impl<A: Actor> ActorExecutionEnv<A> {
             .get_mut()
             .finalize(&exit_status, &self.ctx)
             .await
-            .with_context(|| format!("Finalization of actor {}", self.actor.get_mut().name()))
+            .with_context(|| format!("finalization of actor {}", self.actor.get_mut().name()))
         {
-            error!(error=?finalize_error, "Finalizing failed, set exit status to panicked.");
+            error!(error=?finalize_error, "finalizing failed, set exit status to panicked");
             return ActorExitStatus::Panicked;
         }
         exit_status

quickwit/quickwit-actors/src/supervisor.rs

@@ -246,7 +246,7 @@ mod tests {
                 }
                 FailingActorMessage::ReturnError => {
                     return Err(ActorExitStatus::from(anyhow::anyhow!(
-                        "Failing actor error"
+                        "failing actor error"
                     )));
                 }
                 FailingActorMessage::Increment => {

quickwit/quickwit-actors/src/tests.rs

@@ -497,7 +497,7 @@ impl Actor for BuggyFinalizeActor {
         _exit_status: &ActorExitStatus,
         _: &ActorContext<Self>,
     ) -> anyhow::Result<()> {
-        anyhow::bail!("Finalize error")
+        anyhow::bail!("finalize error")
     }
 }
 

quickwit/quickwit-cli/src/checklist.rs

@@ -123,7 +123,7 @@ impl Display for ChecklistError {
                     check_item,
                     check_item_err
                         .as_ref()
-                        .expect_err("ChecklistError can't contain success results.")
+                        .expect_err("ChecklistError can't contain success results")
                 )
             })
             .join("");

quickwit/quickwit-cli/src/cli.rs

@@ -77,14 +77,14 @@ impl CliCommand {
     pub fn parse_cli_args(mut matches: ArgMatches) -> anyhow::Result<Self> {
         let (subcommand, submatches) = matches
             .remove_subcommand()
-            .context("Failed to parse command.")?;
+            .context("failed to parse command")?;
         match subcommand.as_str() {
             "index" => IndexCliCommand::parse_cli_args(submatches).map(CliCommand::Index),
             "run" => RunCliCommand::parse_cli_args(submatches).map(CliCommand::Run),
             "source" => SourceCliCommand::parse_cli_args(submatches).map(CliCommand::Source),
             "split" => SplitCliCommand::parse_cli_args(submatches).map(CliCommand::Split),
             "tool" => ToolCliCommand::parse_cli_args(submatches).map(CliCommand::Tool),
-            _ => bail!("Unknown command `{subcommand}`."),
+            _ => bail!("unknown command `{subcommand}`"),
         }
     }
 

quickwit/quickwit-cli/src/index.rs

@@ -259,7 +259,7 @@ impl IndexCliCommand {
     pub fn parse_cli_args(mut matches: ArgMatches) -> anyhow::Result<Self> {
         let (subcommand, submatches) = matches
             .remove_subcommand()
-            .context("Failed to parse index subcommand.")?;
+            .context("failed to parse index subcommand")?;
         match subcommand.as_str() {
             "clear" => Self::parse_clear_args(submatches),
             "create" => Self::parse_create_args(submatches),
@@ -268,7 +268,7 @@ impl IndexCliCommand {
             "ingest" => Self::parse_ingest_args(submatches),
             "list" => Self::parse_list_args(submatches),
             "search" => Self::parse_search_args(submatches),
-            _ => bail!("Unknown index subcommand `{subcommand}`."),
+            _ => bail!("unknown index subcommand `{subcommand}`"),
         }
     }
 
@@ -339,7 +339,7 @@ impl IndexCliCommand {
             (false, false) => CommitType::Auto,
             (false, true) => CommitType::Force,
             (true, false) => CommitType::WaitFor,
-            (true, true) => bail!("`--wait` and `--force` are mutually exclusive options."),
+            (true, true) => bail!("`--wait` and `--force` are mutually exclusive options"),
         };
 
         if commit_type == CommitType::Auto && client_args.commit_timeout.is_some() {
@@ -361,7 +361,7 @@ impl IndexCliCommand {
             .expect("`index` should be a required arg.");
         let query = matches
             .remove_one::<String>("query")
-            .context("`query` should be a required arg.")?;
+            .context("`query` should be a required arg")?;
         let aggregation = matches.remove_one::<String>("aggregation");
 
         let max_hits = matches
@@ -779,7 +779,7 @@ pub async fn ingest_docs_cli(args: IngestDocsArgs) -> anyhow::Result<()> {
     }
     let progress_bar = match &args.input_path_opt {
         Some(filepath) => {
-            let file_len = std::fs::metadata(filepath).context("File not found")?.len();
+            let file_len = std::fs::metadata(filepath).context("file not found")?.len();
             ProgressBar::new(file_len)
         }
         None => ProgressBar::new_spinner(),
@@ -834,7 +834,7 @@ pub async fn search_index(args: SearchIndexArgs) -> anyhow::Result<SearchRespons
     let aggs: Option<serde_json::Value> = args
         .aggregation
         .map(|aggs_string| {
-            serde_json::from_str(&aggs_string).context("Failed to deserialize aggregations.")
+            serde_json::from_str(&aggs_string).context("failed to deserialize aggregations")
         })
         .transpose()?;
     let sort_by = args

quickwit/quickwit-cli/src/lib.rs

@@ -202,7 +202,7 @@ pub fn parse_duration_or_none(duration_with_unit_str: &str) -> anyhow::Result<Ti
     } else {
         humantime::parse_duration(duration_with_unit_str)
             .map(Timeout::new)
-            .context("Failed to parse timeout.")
+            .context("failed to parse timeout")
     }
 }
 
@@ -215,7 +215,7 @@ pub fn start_actor_runtimes(
         || services.contains(&QuickwitService::ControlPlane)
     {
         quickwit_common::runtimes::initialize_runtimes(runtimes_config)
-            .context("Failed to start actor runtimes.")?;
+            .context("failed to start actor runtimes")?;
     }
     Ok(())
 }
@@ -224,11 +224,11 @@ pub fn start_actor_runtimes(
 async fn load_node_config(config_uri: &Uri) -> anyhow::Result<NodeConfig> {
     let config_content = load_file(&StorageResolver::unconfigured(), config_uri)
         .await
-        .context("Failed to load node config.")?;
+        .context("failed to load node config")?;
     let config_format = ConfigFormat::sniff_from_uri(config_uri)?;
     let config = NodeConfig::load(config_format, config_content.as_slice())
         .await
-        .with_context(|| format!("Failed to parse node config `{config_uri}`."))?;
+        .with_context(|| format!("failed to parse node config `{config_uri}`"))?;
     info!(config_uri=%config_uri, config=?config, "Loaded node config.");
     Ok(config)
 }

quickwit/quickwit-cli/src/main.rs

@@ -52,7 +52,7 @@ fn setup_logging_and_tracing(
     let env_filter = env::var("RUST_LOG")
         .map(|_| EnvFilter::from_default_env())
         .or_else(|_| EnvFilter::try_new(format!("quickwit={level}")))
-        .context("Failed to set up tracing env filter.")?;
+        .context("failed to set up tracing env filter")?;
     global::set_text_map_propagator(TraceContextPropagator::new());
     let registry = tracing_subscriber::registry().with(env_filter);
     let event_format = tracing_subscriber::fmt::format()
@@ -64,7 +64,7 @@ fn setup_logging_and_tracing(
                 time::format_description::parse(
                     "[year]-[month]-[day]T[hour]:[minute]:[second].[subsecond digits:3]Z",
                 )
-                .expect("Time format invalid."),
+                .expect("time format invalid"),
             ),
         );
     // Note on disabling ANSI characters: setting the ansi boolean on event format is insufficient.
@@ -84,7 +84,7 @@ fn setup_logging_and_tracing(
             .with_trace_config(trace_config)
             .with_batch_config(batch_config)
             .install_batch(opentelemetry::runtime::Tokio)
-            .context("Failed to initialize OpenTelemetry OTLP exporter.")?;
+            .context("failed to initialize OpenTelemetry OTLP exporter")?;
         registry
             .with(tracing_opentelemetry::layer().with_tracer(tracer))
             .with(
@@ -93,7 +93,7 @@ fn setup_logging_and_tracing(
                     .with_ansi(ansi),
             )
             .try_init()
-            .context("Failed to set up tracing.")?;
+            .context("failed to set up tracing")?;
     } else {
         registry
             .with(
@@ -102,7 +102,7 @@ fn setup_logging_and_tracing(
                     .with_ansi(ansi),
             )
             .try_init()
-            .context("Failed to set up tracing.")?;
+            .context("failed to set up tracing")?;
     }
     Ok(())
 }

quickwit/quickwit-cli/src/source.rs

@@ -209,7 +209,7 @@ impl SourceCliCommand {
     pub fn parse_cli_args(mut matches: ArgMatches) -> anyhow::Result<Self> {
         let (subcommand, submatches) = matches
             .remove_subcommand()
-            .context("Failed to parse source subcommand.")?;
+            .context("failed to parse source subcommand")?;
         match subcommand.as_str() {
             "create" => Self::parse_create_args(submatches).map(Self::CreateSource),
             "enable" => {
@@ -224,7 +224,7 @@ impl SourceCliCommand {
             "reset-checkpoint" => {
                 Self::parse_reset_checkpoint_args(submatches).map(Self::ResetCheckpoint)
             }
-            _ => bail!("Unknown source subcommand `{subcommand}`."),
+            _ => bail!("unknown source subcommand `{subcommand}`"),
         }
     }
 
@@ -348,7 +348,7 @@ async fn toggle_source_cli(args: ToggleSourceArgs) -> anyhow::Result<()> {
         .sources(&args.index_id)
         .toggle(&args.source_id, args.enable)
         .await
-        .context("Failed to update source")?;
+        .context("failed to update source")?;
 
     let toggled_state_name = if args.enable { "enabled" } else { "disabled" };
     println!(
@@ -376,7 +376,7 @@ async fn delete_source_cli(args: DeleteSourceArgs) -> anyhow::Result<()> {
         .sources(&args.index_id)
         .delete(&args.source_id)
         .await
-        .context("Failed to delete source.")?;
+        .context("failed to delete source")?;
     println!("{} Source successfully deleted.", "✔".color(GREEN_COLOR));
     Ok(())
 }
@@ -388,7 +388,7 @@ async fn describe_source_cli(args: DescribeSourceArgs) -> anyhow::Result<()> {
         .indexes()
         .get(&args.index_id)
         .await
-        .context("Failed to fetch index metadata.")?;
+        .context("failed to fetch index metadata")?;
     let source_checkpoint = index_metadata
         .checkpoint
         .source_checkpoint(&args.source_id)
@@ -414,7 +414,7 @@ where
     let source = sources
         .into_iter()
         .find(|source| source.source_id == source_id)
-        .with_context(|| format!("Source `{source_id}` does not exist."))?;
+        .with_context(|| format!("source `{source_id}` does not exist"))?;
 
     let source_rows = vec![SourceRow {
         source_id: source.source_id.clone(),
@@ -446,7 +446,7 @@ async fn list_sources_cli(args: ListSourcesArgs) -> anyhow::Result<()> {
         .indexes()
         .get(&args.index_id)
         .await
-        .context("Failed to fetch indexes metadatas.")?;
+        .context("failed to fetch indexes metadatas")?;
     let table = make_list_sources_table(index_metadata.sources.into_values());
     display_tables(&[table]);
     Ok(())