Wrong error status in function delete API.

[ShreyBana]

We're sending a 500 status code in case there is a foreign key ref constraint violation while deleting the function. Basically a case where the user tried to delete a function which was being referenced by either dimension or default-config. This should result in a 400 instead of a 500.

[kanusowi]

can I contribute?

crates/context_aware_config/src/api/functions/handlers.rs:

Err(e) => match e {
    diesel::result::Error::DatabaseError(
        diesel::result::DatabaseErrorKind::ForeignKeyViolation,
        _,
    ) => {
        log::error!("Attempted to delete function {f_name} which is referenced by another entity: {e}");
        Err(bad_argument!(
            "Function {} cannot be deleted as it is currently referenced by a dimension or default config.",
            f_name
        ))
    }
    _ => {
        log::error!("function delete query failed with error: {e}");
        Err(unexpected_error!(
            "Something went wrong, failed to delete the function"
        ))
    }
}

[ayushjain17]

@kanusowi you can, but the solution here is to simply un-handle the error, the Result type which we are using auto handles these cases, by handling the error case separately here, we overrode the default behaviour, which was wrong

[kanusowi]

@ayushjain17 thanks for the clarification! I've removed the explicit match for FKValidation. Using the ? operator to let the error propagate.

    .execute(&mut conn)?; <<----
    let deleted_row =
    delete(functions::functions.filter(functions::function_name.eq(&f_name)))
        .schema_name(&schema_name)
        .execute(&mut conn)?;

match deleted_row {
    0 => Err(not_found!("Function {} doesn't exists", f_name)),
    _ => {
        log::info!("{f_name} function deleted by {}", user.get_email());
        Ok(HttpResponse::NoContent().finish())
    }
}

It should now be handled by AppError by default