Inconsistent Error Responses for Missing Fields in API Request

[FajarFE]

#![allow(clippy::missing_errors_doc)]
#![allow(clippy::unnecessary_struct_initialization)]
#![allow(clippy::unused_async)]
use chrono::NaiveDate;
use loco_rs::prelude::*;
use axum::{debug_handler, extract::Query, http::StatusCode};
use serde::{Deserialize, Serialize};
use validator::{Validate, ValidationErrors};
use std::collections::HashMap;
use crate::models::_entities::todolists::{ActiveModel, Entity, Model};

#[derive(Clone, Debug, Serialize, Deserialize, Validate)]
pub struct Params {
    
    #[validate(length(min = 1, message = "Invalid email address"))]
    pub status: String,

    #[validate(length(min = 1, message = "Title must not be empty"))]
    pub title: String,

    #[validate(length(min = 1, message = "Description must not be empty"))]
    pub desc: String,

    #[validate(length(min = 1, message = "Due date must not be empty"))]
    pub due_date: String,
}

impl Params {
    fn update(&self, item: &mut ActiveModel) -> Result<(), String> {
        if self.status.trim().is_empty() {
            return Err("Status must not be empty".to_string());
        }
        if self.title.trim().is_empty() {
            return Err("Title must not be empty".to_string());
        }
        if self.desc.trim().is_empty() {
            return Err("Description must not be empty".to_string());
        }
        if self.due_date.trim().is_empty() {
            return Err("Due date must not be empty".to_string());
        }

        item.status = Set(Some(self.status.clone()));
        item.title = Set(Some(self.title.clone()));
        item.desc = Set(Some(self.desc.clone()));
        
        // Parse and validate the date format
        match NaiveDate::parse_from_str(&self.due_date, "%Y-%m-%d") {
            Ok(date) => {
                item.due_date = Set(Some(date));
                Ok(())
            },
            Err(_) => Err("Invalid date format, please use YYYY-MM-DD".to_string())
        }
    }
}

fn validation_errors_to_structured(errors: &ValidationErrors) -> Vec<HashMap<String, String>> {
    errors
        .field_errors()
        .iter()
        .map(|(field, field_errors)| {
            let mut error_map = HashMap::new();
            let error_message = field_errors
                .iter()
                .map(|err| format!("{}", err))
                .collect::<Vec<String>>()
                .join(", ");
            error_map.insert(field.to_string(), error_message);
            error_map
        })
        .collect()
}

async fn load_item(ctx: &AppContext, id: i32) -> Result<Model> {
    let item = Entity::find_by_id(id).one(&ctx.db).await?;
    item.ok_or_else(|| Error::NotFound)
}

#[debug_handler]
pub async fn list(State(ctx): State<AppContext>) -> Result<Response> {
    format::json(Entity::find().all(&ctx.db).await?)
}

#[derive(Deserialize)]
pub struct ListFilter {
    pub status: Option<String>,
    pub search: Option<String>,
    pub due_date: Option<Date>,
}

#[debug_handler]

pub async fn list_by_filter(
    auth: auth::JWT,
    query: Query<ListFilter>,
    State(ctx): State<AppContext>,
) -> Result<Response> {
    match crate::models::users::Model::find_by_pid(&ctx.db, &auth.claims.pid).await {
        Ok(_current_user) => {
            let items = Entity::find_by_filter::<Model>(
                query.due_date,
                query.search.clone(),
                query.status.clone(),
                &ctx.db,
            )
            .await?;
            format::json(serde_json::json!({
                "code": 200,
                "success":true,
                "data": items
            }))
        }
        Err(_) => format::json(serde_json::json!({
            "code": 500,
            "success": false,
            "data": []
        })),
    }
}

#[debug_handler]
pub async fn add(
    auth: auth::JWT,
    State(ctx): State<AppContext>,
    Json(params): Json<Params>,
) -> Result<Response> {
    if let Err(validation_errors) = params.validate() {
        // Return a 400 error with validation details
        return Ok((
            StatusCode::BAD_REQUEST,
            axum::Json(serde_json::json!({
                "code": 400,
                "success": false,
                "error": validation_errors_to_structured(&validation_errors)
            })),
        )
            .into_response());
    }

    match crate::models::users::Model::find_by_pid(&ctx.db, &auth.claims.pid).await {
        Ok(_current_user) => {
            let mut item = ActiveModel {
                user_id: Set(_current_user.id),
                ..Default::default()
            };

            match params.update(&mut item) {
                Ok(_) => {
                    match item.insert(&ctx.db).await {
                        Ok(item) => format::json(serde_json::json!({
                            "code": 200,
                            "success": true,
                            "data": item
                        })),
                        Err(e) => format::json(serde_json::json!({
                            "code": 500,
                            "success": false,
                            "error": format!("Database error: {}", e)
                        }))
                    }
                },
                Err(e) => Ok((
                    StatusCode::BAD_REQUEST,
                    axum::Json(serde_json::json!({
                        "code": 400,
                        "success": false,
                        "error": [{
                            "validation": e
                        }]
                    })),
                ).into_response())
            }
        }
        Err(_) => format::json(serde_json::json!({
            "code": 500,
            "success": false,
            "data": []
        }))
    }
}
#[debug_handler]
pub async fn update(
    auth: auth::JWT,
    Path(id): Path<i32>,
    State(ctx): State<AppContext>,
    Json(params): Json<Params>,
) -> Result<Response> {
    let _current_user = crate::models::users::Model::find_by_pid(&ctx.db, &auth.claims.pid).await?;
    let item = load_item(&ctx, id).await?;
    let mut item = item.into_active_model();
    params.update(&mut item);
    let item = item.update(&ctx.db).await?;
    format::json(item)
}

#[debug_handler]
pub async fn remove(Path(id): Path<i32>, State(ctx): State<AppContext>) -> Result<Response> {
    load_item(&ctx, id).await?.delete(&ctx.db).await?;
    format::empty()
}

#[debug_handler]
pub async fn get_one(Path(id): Path<i32>, State(ctx): State<AppContext>) -> Result<Response> {
    format::json(load_item(&ctx, id).await?)
}

pub fn routes() -> Routes {
    Routes::new()
        .prefix("api/todolists/")
        .add("/", get(list_by_filter))
        .add("/", post(add))
        .add("{id}", get(get_one))
        .add("{id}", delete(remove))
        .add("{id}", put(update))
        .add("{id}", patch(update))
}

Description: We are encountering an issue where the error responses returned by the API are inconsistent when required fields are missing in the request body. This inconsistency makes it harder for users to identify which specific fields are missing or invalid, leading to confusion. In some cases, the API returns detailed error messages indicating which field is missing, while in other cases, it simply returns a generic "Bad Request" error without specifying the problematic field.
Steps to Reproduce:

1. Request with status field empty: When we send a request with an empty status field (but other fields are populated), we receive a detailed error response specifically pointing out the missing field.

    **Request**
    `{
      "status": "",
      "title": "",
      "desc": "This task needs to be finished by the end of the week.",
      "due_date": "2025-02-01"
    }
    `
   
    **Response**:
    `{{
        "code": 400,
        "error": [
            {
                "status": "Invalid email address"
            },
            {
                "title": "Title must not be empty"
            }
        ],
        "success": false
    }
    `
    In this case, the error message clearly indicates that the status field is required.
   

2. Request with title field empty: In a different scenario where the status field is omitted, but other fields like title, desc, and due_date are included (with title being empty), the response returned is not as detailed. Instead of pointing out the specific missing or invalid fields, the API simply returns a "Bad Request" error.

    **Request:**
    `{
      "title": "",
      "desc": "This task needs to be finished by the end of the week.",
      "due_date": "2025-02-01"
    }
    `
    
    **Response:**
    `{
      "error": "Bad Request"
    }
    `
        Here, the response lacks details on which field or fields are causing the issue, and it only provides a generic error message, which is not helpful for debugging the problem.
   

3. Expected Behavior:
   The API should return consistent, clear, and detailed error messages for all missing or invalid fields in the request. When a required field is missing, the API should always return a structured error response that identifies exactly which fields are missing or invalid. This would help developers and users quickly identify and resolve issues with their API requests.
   For instance, in the case where the title field is missing or empty, the error message should look similar to the one for the status field, indicating which field needs to be filled in.

4. Actual Behavior:
   The API returns inconsistent error messages based on which fields are missing:

• When the status field is empty, the API returns a detailed message pointing out that the status field is required.
• When the title field is empty (and status is omitted), the API simply returns a generic "Bad Request" error, without specifying which field is causing the issue.
  This inconsistency makes it difficult to understand what went wrong and what needs to be corrected in the request.

[kaplanelad]

see this pr: #1174

in master and should be included in the next release

[FajarFE]

see this pr: #1174

in master and should be included in the next release

The release will be awaited, and I hope the Loco framework continues to grow and improve in the future.

[FajarFE]

Hello sir, success!

However, if I remove the status and title inputs, an error occurs with code 422, and the error details in the response only show this, even though it should require status and title.