diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5e0f17827..0766f2025 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,6 +43,10 @@ We are using [prek](https://prek.j178.dev/) (`pre-commit` Rust alternative) hook Pre-commit still works for this project because `prek` and `pre-commit` share the same configuration file. However, the project may switch to a `prek`-specific configuration in the future. +Error messages should begin with a lowercase letter and should not end with +punctuation unless they contain multiple sentences. This follows the convention +used by Rust's standard library and keeps chained errors readable. + ### Tests that use database, cache, or other external resources Some tests use a database, cache, or other external resources. All these tests diff --git a/cot-cli/src/migration_generator.rs b/cot-cli/src/migration_generator.rs index 5d5c4ebe6..8159a30cd 100644 --- a/cot-cli/src/migration_generator.rs +++ b/cot-cli/src/migration_generator.rs @@ -23,15 +23,15 @@ use crate::utils::{CargoTomlManager, PackageManager}; pub fn make_migrations(path: &Path, options: MigrationGeneratorOptions) -> anyhow::Result<()> { let Some(manager) = CargoTomlManager::from_path(path)? else { - bail!("Cargo.toml not found in the specified directory or any parent directory.") + bail!("no Cargo.toml found in the specified directory or any parent directory") }; match manager { CargoTomlManager::Workspace(workspace) => { let Some(package) = workspace.get_current_package_manager() else { bail!( - "Generating migrations for workspaces is not supported yet. \ - Please generate migrations for each package separately." + "generating migrations for workspaces is not supported yet. \ + Please generate migrations for each package separately" ); }; make_package_migrations(package, options) @@ -75,15 +75,15 @@ pub fn create_new_migration( options: MigrationGeneratorOptions, ) -> anyhow::Result<()> { let Some(manager) = CargoTomlManager::from_path(path)? else { - bail!("Cargo.toml not found in the specified directory or any parent directory.") + bail!("no Cargo.toml found in the specified directory or any parent directory") }; match manager { CargoTomlManager::Workspace(workspace) => { let Some(package) = workspace.get_current_package_manager() else { bail!( - "Generating migrations for workspaces is not supported yet. \ - Please generate migrations for each package separately." + "generating migrations for workspaces is not supported yet. \ + Please generate migrations for each package separately" ); }; create_package_new_migration(package, name, options) @@ -137,7 +137,7 @@ pub fn list_migrations(path: &Path) -> anyhow::Result { - bail!("Cargo.toml is not a valid workspace or package manifest"); + bail!("invalid Cargo.toml: expected a workspace or package manifest"); } }; diff --git a/cot-core/src/error/error_impl.rs b/cot-core/src/error/error_impl.rs index 80a2f0f3a..d18eb2a4d 100644 --- a/cot-core/src/error/error_impl.rs +++ b/cot-core/src/error/error_impl.rs @@ -51,10 +51,10 @@ impl Error { /// ``` /// use cot::Error; /// - /// let error = Error::internal("An error occurred"); + /// let error = Error::internal("an error occurred"); /// let error = Error::internal(std::io::Error::new( /// std::io::ErrorKind::Other, - /// "An error occurred", + /// "an error occurred", /// )); /// ``` #[must_use] @@ -79,10 +79,10 @@ impl Error { /// use cot::{Error, StatusCode}; /// /// // Create a 400 Bad Request error - /// let error = Error::with_status("Invalid input", StatusCode::BAD_REQUEST); + /// let error = Error::with_status("invalid input", StatusCode::BAD_REQUEST); /// /// // Create a 403 Forbidden error - /// let error = Error::with_status("Access denied", StatusCode::FORBIDDEN); + /// let error = Error::with_status("access denied", StatusCode::FORBIDDEN); /// ``` #[must_use] pub fn with_status(error: E, status_code: StatusCode) -> Self @@ -109,10 +109,10 @@ impl Error { /// ``` /// use cot::{Error, StatusCode}; /// - /// let error = Error::internal("Something went wrong"); + /// let error = Error::internal("something went wrong"); /// assert_eq!(error.status_code(), StatusCode::INTERNAL_SERVER_ERROR); /// - /// let error = Error::with_status("Bad request", StatusCode::BAD_REQUEST); + /// let error = Error::with_status("bad request", StatusCode::BAD_REQUEST); /// assert_eq!(error.status_code(), StatusCode::BAD_REQUEST); /// ``` #[must_use] diff --git a/cot-core/src/request/extractors.rs b/cot-core/src/request/extractors.rs index 13efb87b8..c7709fc56 100644 --- a/cot-core/src/request/extractors.rs +++ b/cot-core/src/request/extractors.rs @@ -287,7 +287,7 @@ impl FromRequest for Json { #[cfg(feature = "json")] #[derive(Debug, thiserror::Error)] -#[error("JSON deserialization error: {0}")] +#[error("failed to deserialize JSON: {0}")] struct JsonDeserializeError(serde_path_to_error::Error); #[cfg(feature = "json")] impl_into_cot_error!(JsonDeserializeError, BAD_REQUEST); diff --git a/cot-core/src/response/into_response.rs b/cot-core/src/response/into_response.rs index 13a7769f4..2a5c29511 100644 --- a/cot-core/src/response/into_response.rs +++ b/cot-core/src/response/into_response.rs @@ -372,7 +372,7 @@ impl IntoResponse for crate::json::Json { #[cfg(feature = "json")] #[derive(Debug, thiserror::Error)] -#[error("JSON serialization error: {0}")] +#[error("failed to serialize JSON: {0}")] struct JsonSerializeError(serde_path_to_error::Error); #[cfg(feature = "json")] impl_into_cot_error!(JsonSerializeError, INTERNAL_SERVER_ERROR); diff --git a/cot/src/admin.rs b/cot/src/admin.rs index e35e1ce0b..a62fb60fc 100644 --- a/cot/src/admin.rs +++ b/cot/src/admin.rs @@ -109,7 +109,7 @@ async fn login( let mut context = LoginForm::build_context(&mut request).await?; context.add_error( FormErrorTarget::Form, - FormFieldValidationError::from_static("Invalid username or password"), + FormFieldValidationError::from_static("invalid username or password"), ); context } diff --git a/cot/src/db/migrations.rs b/cot/src/db/migrations.rs index 8c011ecbf..0be5518a6 100644 --- a/cot/src/db/migrations.rs +++ b/cot/src/db/migrations.rs @@ -50,7 +50,7 @@ pub enum MigrationEngineError { #[error("error running migration: {0}")] Custom(String), /// An I/O error occurred while writing output (e.g. during dry-run). - #[error("I/O error while writing migration output: {0}")] + #[error("failed to write migration output: {0}")] Io(#[from] io::Error), } diff --git a/cot/src/email.rs b/cot/src/email.rs index ae7640753..14a090335 100644 --- a/cot/src/email.rs +++ b/cot/src/email.rs @@ -178,7 +178,7 @@ pub enum EmailMessageError { #[error("{ERROR_PREFIX} failed to build email message: {0}")] BuildError(Box), /// A required field is missing in the email message. - #[error("{ERROR_PREFIX} The `{0}` field is required but was not set")] + #[error("{ERROR_PREFIX} the `{0}` field is required but was not set")] MissingField(String), } @@ -370,7 +370,7 @@ mod tests { let err = res.err().unwrap(); assert_eq!( err.to_string(), - "email message build error: The `from` field is required but was not set" + "email message build error: the `from` field is required but was not set" ); } diff --git a/cot/src/error/handler.rs b/cot/src/error/handler.rs index 66e0e1708..83052c2c3 100644 --- a/cot/src/error/handler.rs +++ b/cot/src/error/handler.rs @@ -214,7 +214,7 @@ impl FromRequestHead for RequestOuterError { let error = head.extensions.get::(); error .ok_or_else(|| { - Error::internal("No error found in request head. Make sure you use this extractor in an error handler.") + Error::internal("no error found in request head. Make sure you use this extractor in an error handler.") }).cloned() } } diff --git a/cot/src/error/not_found.rs b/cot/src/error/not_found.rs index f62c7057e..3f6bd81e9 100644 --- a/cot/src/error/not_found.rs +++ b/cot/src/error/not_found.rs @@ -19,7 +19,7 @@ use thiserror::Error; /// let error = NotFound::new(); /// /// // Create a 404 error with a custom message -/// let error = NotFound::with_message("User not found"); +/// let error = NotFound::with_message("user not found"); /// ``` /// /// ["404 Not Found"]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/404 @@ -33,7 +33,7 @@ pub struct NotFound { impl_into_cot_error!(NotFound, NOT_FOUND); impl NotFound { - /// Creates a new `NotFound` error with a generic "Not Found" message. + /// Creates a new `NotFound` error with a generic "not found" message. /// /// This is the most common way to create a 404 error when you don't need /// to provide additional context about what was not found. @@ -61,9 +61,9 @@ impl NotFound { /// ``` /// use cot::error::NotFound; /// - /// let error = NotFound::with_message("User with ID 123 not found"); + /// let error = NotFound::with_message("user with ID 123 not found"); /// let page_name = "home"; - /// let error = NotFound::with_message(format!("Page '{}' not found", page_name)); + /// let error = NotFound::with_message(format!("page '{}' not found", page_name)); /// ``` #[must_use] pub fn with_message>(message: T) -> Self { @@ -99,14 +99,14 @@ pub enum Kind { /// /// This variant is used when the router cannot find a route that matches /// the request's path and method. - #[error("Not Found")] + #[error("not found")] #[non_exhaustive] FromRouter, /// A generic 404 error without additional context. /// /// This variant is used for basic "not found" errors where no specific /// message or context is needed. - #[error("Not Found")] + #[error("not found")] #[non_exhaustive] Custom, /// A 404 error with a custom message providing additional context. @@ -114,7 +114,22 @@ pub enum Kind { /// This variant includes a custom message that describes what specifically /// was not found, which can be useful for debugging or providing more /// informative error responses. - #[error("Not Found: {0}")] + #[error("not found: {0}")] #[non_exhaustive] WithMessage(String), } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn messages_are_idiomatic() { + assert_eq!(NotFound::new().to_string(), "not found"); + assert_eq!( + NotFound::with_message("resource unavailable").to_string(), + "not found: resource unavailable" + ); + assert_eq!(NotFound::router().to_string(), "not found"); + } +} diff --git a/cot/src/form.rs b/cot/src/form.rs index 499045e88..0952f07df 100644 --- a/cot/src/form.rs +++ b/cot/src/form.rs @@ -130,43 +130,43 @@ impl FormResult { #[error("{message}")] pub enum FormFieldValidationError { /// The field is required. - #[error("This field is required.")] + #[error("this field is required")] Required, /// The field value is too long. - #[error("This exceeds the maximum length of {max_length}.")] + #[error("this exceeds the maximum length of {max_length}")] MaximumLengthExceeded { /// The maximum length of the field. max_length: u32, }, /// The field value is too short. - #[error("This is below the minimum length of {min_length}.")] + #[error("this is below the minimum length of {min_length}")] MinimumLengthNotMet { /// The minimum length of the field. min_length: u32, }, /// The field value is below the permitted minimum. - #[error("This is below the minimum value of {min_value}.")] + #[error("this is below the minimum value of {min_value}")] MinimumValueNotMet { /// The minimum permitted value. min_value: String, }, /// The field value exceeds the permitted maximum. - #[error("This exceeds the maximum value of {max_value}.")] + #[error("this exceeds the maximum value of {max_value}")] MaximumValueExceeded { /// The maximum permitted value. max_value: String, }, /// The field value is an ambiguous datetime. - #[error("The datetime value `{datetime}` is ambiguous.")] + #[error("the datetime value `{datetime}` is ambiguous")] AmbiguousDateTime { /// The ambiguous datetime value. datetime: NaiveDateTime, }, /// The field value is a non-existent local datetime. - #[error("Local datetime {datetime} does not exist for the specified timezone {timezone}.")] + #[error("local datetime {datetime} does not exist for the specified timezone {timezone}")] NonExistentLocalDateTime { /// The non-existent local datetime value. datetime: NaiveDateTime, @@ -174,13 +174,13 @@ pub enum FormFieldValidationError { timezone: Tz, }, /// The field value is required to be true. - #[error("This field must be checked.")] + #[error("this field must be checked")] BooleanRequiredToBeTrue, /// The field value is invalid. - #[error("Value is not valid for this field.")] + #[error("value is not valid for this field")] InvalidValue(String), /// An error occurred while getting the field value. - #[error("Error getting field value: {0}")] + #[error("error getting field value: {0}")] FormFieldValueError(#[from] FormFieldValueError), /// Custom error with a given message. #[error("{0}")] @@ -868,4 +868,23 @@ mod tests { panic!("Expected RequestError"); } } + #[test] + fn built_in_validation_errors_have_idiomatic_messages() { + assert_eq!( + FormFieldValidationError::Required.to_string(), + "this field is required" + ); + assert_eq!( + FormFieldValidationError::maximum_length_exceeded(10).to_string(), + "this exceeds the maximum length of 10" + ); + assert_eq!( + FormFieldValidationError::minimum_length_not_met(2).to_string(), + "this is below the minimum length of 2" + ); + assert_eq!( + FormFieldValidationError::invalid_value("invalid").to_string(), + "value is not valid for this field" + ); + } } diff --git a/cot/src/form/fields.rs b/cot/src/form/fields.rs index b30c73c0e..66851d3b4 100644 --- a/cot/src/form/fields.rs +++ b/cot/src/form/fields.rs @@ -1066,7 +1066,7 @@ macro_rules! impl_float_as_form_field { if parsed.is_nan() || parsed.is_infinite() { return Err(FormFieldValidationError::from_static( - "Cannot have NaN or inf as form input values", + "cannot have NaN or inf as form input values", )); } @@ -1801,7 +1801,7 @@ mod tests { assert_eq!( value, Err(FormFieldValidationError::from_static( - "Cannot have NaN or inf as form input values" + "cannot have NaN or inf as form input values" )) ); } diff --git a/cot/src/test.rs b/cot/src/test.rs index 24c2d61fb..61456508d 100644 --- a/cot/src/test.rs +++ b/cot/src/test.rs @@ -1670,7 +1670,7 @@ async fn set_current_db(conn: &mut Connection, db_num: usize) { enum RedisDbAllocatorError { #[error(transparent)] Io(#[from] std::io::Error), - #[error("Redis error: {0}")] + #[error("redis error: {0}")] Redis(String), } diff --git a/docs/error-pages.md b/docs/error-pages.md index 582ac2b0e..10421b44c 100644 --- a/docs/error-pages.md +++ b/docs/error-pages.md @@ -94,15 +94,15 @@ async fn view(request: Request) -> cot::Result { // 404 with custom message return Err(NotFound::with_message( - "The article you're looking for doesn't exist".to_string() + "the article you're looking for doesn't exist".to_string() ))?; // 500 Internal Server Error - return Err(Error::internal("Something went wrong")); + return Err(Error::internal("something went wrong")); // or, by re-raising a custom error: return Err(Error::internal(std::io::Error::other("oh no!"))); // or, by panicking: - panic!("Something went wrong"); + panic!("something went wrong"); } ``` diff --git a/docs/forms.md b/docs/forms.md index dec3f075f..1f4ef2032 100644 --- a/docs/forms.md +++ b/docs/forms.md @@ -133,7 +133,7 @@ async fn handle_form(mut request: Request) -> cot::Result { let mut context = ArticleForm::build_context(&mut request).await?; context.add_error( FormErrorTarget::Field("title"), - FormFieldValidationError::from_static("Title contains spam") + FormFieldValidationError::from_static("title contains spam") ); // Re-render form with error