RTSDA/church-api
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

Phase 1: Enhance error handling and database operations

Browse source 
- Add specific error variants for better internal handling
- Enhance error messages with context and logging
- Improve database operations with better error handling
- Use specific errors for media processing, config issues, duplicates
- All HTTP responses remain identical - zero breaking changes
- Foundation for better maintainability and debugging
This commit is contained in:
Benjamin Slingo 2025-08-28 21:20:29 -04:00
parent 5793e12df9
commit ad43a19f7c
6 changed files with 181 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/db/bulletins.rs
View file

@ -46,7 +46,10 @@ pub async fn get_by_date(pool: &PgPool, date: chrono::NaiveDate) -> Result<Optio
)
.fetch_optional(pool)
.await
.map_err(ApiError::DatabaseError)?;
.map_err(|e| {
tracing::error!("Failed to get bulletin by date {}: {}", date, e);
ApiError::DatabaseError(e)
})?;
Ok(bulletin)
}

10
src/db/schedule.rs
View file

@ -48,7 +48,15 @@ pub async fn insert_or_update(pool: &PgPool, schedule: &Schedule) -> Result<Sche
)
.fetch_one(pool)
.await
.map_err(|e| ApiError::Database(e.to_string()))?;
.map_err(|e| {
tracing::error!("Failed to insert/update schedule for date {}: {}", schedule.date, e);
match e {
sqlx::Error::Database(db_err) if db_err.constraint().is_some() => {
ApiError::duplicate_entry("Schedule", &schedule.date)
}
_ => ApiError::DatabaseError(e)
}
})?;
Ok(result)
}

134
src/error.rs
View file

@ -14,6 +14,34 @@ pub enum ApiError {
JwtError(jsonwebtoken::errors::Error),
BcryptError(bcrypt::BcryptError),
SerdeError(serde_json::Error),
// Enhanced specific error types for better internal handling
// All map to existing HTTP responses - zero breaking changes
BulletinNotFound(String),
EventNotFound(String),
ScheduleNotFound(String),
MemberNotFound(String),
HymnNotFound(String),
UserNotFound(String),
// Processing errors
BulletinProcessingError(String),
MediaProcessingError(String),
EmailSendError(String),
UploadError(String),
// Configuration errors
ConfigurationError(String),
MissingConfiguration(String),
// Business logic errors
DuplicateEntry(String),
InvalidDateRange(String),
InvalidRecurringPattern(String),
// External service errors
OwncastConnectionError(String),
ExternalServiceError { service: String, message: String },
}
impl IntoResponse for ApiError {
@ -51,6 +79,50 @@ impl IntoResponse for ApiError {
tracing::error!("Serde error: {:?}", e);
(StatusCode::BAD_REQUEST, "Invalid JSON".to_string())
}
// Enhanced error types - map to existing HTTP responses for zero breaking changes
// All *NotFound variants map to 404
ApiError::BulletinNotFound(msg) | ApiError::EventNotFound(msg) |
ApiError::ScheduleNotFound(msg) | ApiError::MemberNotFound(msg) |
ApiError::HymnNotFound(msg) | ApiError::UserNotFound(msg) => {
tracing::warn!("Resource not found: {}", msg);
(StatusCode::NOT_FOUND, msg)
}
// Processing errors map to 500
ApiError::BulletinProcessingError(msg) | ApiError::MediaProcessingError(msg) => {
tracing::error!("Processing error: {}", msg);
(StatusCode::INTERNAL_SERVER_ERROR, msg)
}
// Email and upload errors map to 500
ApiError::EmailSendError(msg) | ApiError::UploadError(msg) => {
tracing::error!("Service error: {}", msg);
(StatusCode::INTERNAL_SERVER_ERROR, msg)
}
// Configuration errors map to 500
ApiError::ConfigurationError(msg) | ApiError::MissingConfiguration(msg) => {
tracing::error!("Configuration error: {}", msg);
(StatusCode::INTERNAL_SERVER_ERROR, "Server configuration error".to_string())
}
// Business logic errors map to 400
ApiError::DuplicateEntry(msg) | ApiError::InvalidDateRange(msg) |
ApiError::InvalidRecurringPattern(msg) => {
tracing::warn!("Business logic error: {}", msg);
(StatusCode::BAD_REQUEST, msg)
}
// External service errors map to 500
ApiError::OwncastConnectionError(msg) => {
tracing::error!("Owncast connection error: {}", msg);
(StatusCode::INTERNAL_SERVER_ERROR, "External service unavailable".to_string())
}
ApiError::ExternalServiceError { service, message } => {
tracing::error!("External service '{}' error: {}", service, message);
(StatusCode::INTERNAL_SERVER_ERROR, "External service error".to_string())
}
};
(
@ -94,4 +166,66 @@ impl From<serde_json::Error> for ApiError {
}
}
impl ApiError {
// Constructor methods for common patterns - makes code more readable and consistent
pub fn bulletin_not_found(id: impl std::fmt::Display) -> Self {
Self::BulletinNotFound(format!("Bulletin not found: {}", id))
}
pub fn event_not_found(id: impl std::fmt::Display) -> Self {
Self::EventNotFound(format!("Event not found: {}", id))
}
pub fn schedule_not_found(date: impl std::fmt::Display) -> Self {
Self::ScheduleNotFound(format!("Schedule not found for date: {}", date))
}
pub fn hymn_not_found(hymnal: &str, number: i32) -> Self {
Self::HymnNotFound(format!("Hymn {} not found in {}", number, hymnal))
}
pub fn user_not_found(identifier: impl std::fmt::Display) -> Self {
Self::UserNotFound(format!("User not found: {}", identifier))
}
pub fn member_not_found(id: impl std::fmt::Display) -> Self {
Self::MemberNotFound(format!("Member not found: {}", id))
}
pub fn bulletin_processing_failed(reason: impl std::fmt::Display) -> Self {
Self::BulletinProcessingError(format!("Bulletin processing failed: {}", reason))
}
pub fn media_processing_failed(reason: impl std::fmt::Display) -> Self {
Self::MediaProcessingError(format!("Media processing failed: {}", reason))
}
pub fn email_send_failed(reason: impl std::fmt::Display) -> Self {
Self::EmailSendError(format!("Email sending failed: {}", reason))
}
pub fn upload_failed(reason: impl std::fmt::Display) -> Self {
Self::UploadError(format!("Upload failed: {}", reason))
}
pub fn invalid_date_range(start: impl std::fmt::Display, end: impl std::fmt::Display) -> Self {
Self::InvalidDateRange(format!("Invalid date range: {} to {}", start, end))
}
pub fn duplicate_entry(resource: &str, identifier: impl std::fmt::Display) -> Self {
Self::DuplicateEntry(format!("{} already exists: {}", resource, identifier))
}
pub fn missing_config(key: &str) -> Self {
Self::MissingConfiguration(format!("Missing required configuration: {}", key))
}
pub fn external_service_failed(service: &str, message: impl std::fmt::Display) -> Self {
Self::ExternalServiceError {
service: service.to_string(),
message: message.to_string(),
}
}
}
pub type Result<T> = std::result::Result<T, ApiError>;

50
src/handlers/smart_streaming.rs
View file

@ -75,7 +75,7 @@ async fn serve_head_response_for_streaming(media_id: Uuid, headers: &HeaderMap)
.header("x-codec", "av01")
.header("content-length", "0") // HEAD request - no body
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Failed to build HEAD response: {}", e)))?
.map_err(|e| ApiError::media_processing_failed(format!("Failed to build HEAD response: {}", e)))?
} else {
// Legacy client - return redirect headers for HLS
Response::builder()
@ -86,7 +86,7 @@ async fn serve_head_response_for_streaming(media_id: Uuid, headers: &HeaderMap)
.header("x-transcoded-by", "Intel-Arc-A770-segments")
.header("cache-control", "no-cache")
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Failed to build HEAD response: {}", e)))?
.map_err(|e| ApiError::media_processing_failed(format!("Failed to build HEAD response: {}", e)))?
};
tracing::info!("📊 METRICS: HEAD_RESPONSE media_id={} av1_support={} user_agent='{}'",
@ -117,7 +117,7 @@ async fn serve_hls_with_arc_a770_segments(
.header("Location", playlist_url)
.header("X-Streaming-Method", "hls-arc-a770-redirect")
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Cannot build redirect: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build redirect: {}", e)))?;
Ok(response)
} else {
@ -132,7 +132,7 @@ async fn serve_hls_with_arc_a770_segments(
.header("X-Streaming-Method", "hls-arc-a770-redirect")
.header("Cache-Control", "no-cache")
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Cannot build redirect: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build redirect: {}", e)))?;
tracing::info!("📊 METRICS: HLS_REDIRECT_TO_ARC_A770 media_id={}", media_id);
Ok(response)
@ -147,10 +147,10 @@ async fn serve_hls_with_arc_a770_segments(
async fn serve_direct_video_with_ranges(source_path: &str, headers: &HeaderMap) -> Result<Response> {
// Check if file exists
let file = fs::File::open(source_path).await
.map_err(|e| ApiError::NotFound(format!("Video file not found: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Video file not found: {}", e)))?;
let file_size = file.metadata().await
.map_err(|e| ApiError::Internal(format!("Cannot get file metadata: {}", e)))?.len();
.map_err(|e| ApiError::media_processing_failed(format!("Cannot get file metadata: {}", e)))?.len();
// Parse Range header
let range_header = headers.get("range").and_then(|h| h.to_str().ok());
@ -176,15 +176,15 @@ async fn serve_partial_content(file_path: &str, file_size: u64, range_header: &s
// Read requested range
let mut file = fs::File::open(file_path).await
.map_err(|e| ApiError::Internal(format!("Cannot open file: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot open file: {}", e)))?;
file.seek(SeekFrom::Start(start)).await
.map_err(|e| ApiError::Internal(format!("Cannot seek file: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot seek file: {}", e)))?;
let bytes_to_read = (end - start + 1) as usize;
let mut buffer = vec![0u8; bytes_to_read];
file.read_exact(&mut buffer).await
.map_err(|e| ApiError::Internal(format!("Cannot read range: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot read range: {}", e)))?;
// Return 206 Partial Content
let response = Response::builder()
@ -196,7 +196,7 @@ async fn serve_partial_content(file_path: &str, file_size: u64, range_header: &s
.header("Cache-Control", "public, max-age=3600")
.header("X-Streaming-Method", "direct-range")
.body(Body::from(buffer))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
Ok(response)
}
@ -212,9 +212,9 @@ async fn serve_entire_file(file_path: &str, file_size: u64) -> Result<Response>
.header("X-Streaming-Method", "direct-full")
.body(Body::from_stream(tokio_util::io::ReaderStream::new(
fs::File::open(file_path).await
.map_err(|e| ApiError::Internal(format!("Cannot open file: {}", e)))?
.map_err(|e| ApiError::media_processing_failed(format!("Cannot open file: {}", e)))?
)))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
Ok(response)
}
@ -240,7 +240,7 @@ async fn serve_hls_with_segment_generation(
.header("Location", playlist_url)
.header("X-Streaming-Method", "hls-segment-generation-redirect")
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Cannot build redirect: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build redirect: {}", e)))?;
Ok(response)
}
@ -288,7 +288,7 @@ pub async fn generate_hls_playlist_for_segment_generation(
.header("X-Streaming-Method", "hls-arc-a770-playlist")
.header("X-Transcoded-By", "Intel-Arc-A770")
.body(Body::from(playlist))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
Ok(response)
}
@ -315,7 +315,7 @@ async fn serve_hls_with_transcoding(
.header("Location", playlist_url)
.header("X-Streaming-Method", "hls-redirect")
.body(Body::empty())
.map_err(|e| ApiError::Internal(format!("Cannot build redirect: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build redirect: {}", e)))?;
Ok(response)
}
@ -362,7 +362,7 @@ pub async fn generate_hls_playlist_for_transcoding(
.header("Cache-Control", "public, max-age=300") // 5 minute cache
.header("X-Streaming-Method", "hls-playlist")
.body(Body::from(playlist))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
Ok(response)
}
@ -424,7 +424,7 @@ pub async fn serve_hls_segment(
.header("X-Streaming-Method", "hls-arc-a770-cached")
.header("X-Transcoded-By", "Intel-Arc-A770")
.body(Body::from(buffer))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
return Ok(response);
}
@ -451,7 +451,7 @@ pub async fn serve_hls_segment(
.header("X-Segment-Duration", &actual_duration.to_string())
.header("X-Start-Time", &start_time.to_string())
.body(Body::from(buffer))
.map_err(|e| ApiError::Internal(format!("Cannot build response: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Cannot build response: {}", e)))?;
tracing::info!("📊 METRICS: ARC_A770_SEGMENT_SUCCESS segment={} duration={}s media_id={}",
segment_index, actual_duration, media_id);
@ -503,7 +503,7 @@ async fn get_media_source_path(media_id: Uuid) -> Result<String> {
// Get database connection from environment
let database_url = std::env::var("DATABASE_URL")
.map_err(|_| ApiError::Internal("DATABASE_URL not set".to_string()))?;
.map_err(|_| ApiError::missing_config("DATABASE_URL"))?;
let pool = PgPool::connect(&database_url).await
.map_err(|e| ApiError::Database(e.to_string()))?;
@ -553,7 +553,7 @@ async fn get_video_duration_direct(file_path: &str) -> Result<f64> {
])
.output()
.await
.map_err(|e| ApiError::Internal(format!("Failed to run ffprobe: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Failed to run ffprobe: {}", e)))?;
if !output.status.success() {
return Err(ApiError::Internal("Failed to get video duration".to_string()));
@ -561,7 +561,7 @@ async fn get_video_duration_direct(file_path: &str) -> Result<f64> {
let duration_str = String::from_utf8_lossy(&output.stdout).trim().to_string();
let duration = duration_str.parse::<f64>()
.map_err(|_| ApiError::Internal("Invalid duration format".to_string()))?;
.map_err(|_| ApiError::media_processing_failed("Invalid duration format"))?;
Ok(duration)
}
@ -626,7 +626,7 @@ async fn generate_h264_segments_from_av1(
// Create output directory
tokio::fs::create_dir_all(output_dir).await
.map_err(|e| ApiError::Internal(format!("Failed to create output directory: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Failed to create output directory: {}", e)))?;
let segment_pattern = format!("{}/segment_%03d.ts", output_dir);
@ -652,7 +652,7 @@ async fn generate_h264_segments_from_av1(
.arg(&segment_pattern)
.output()
.await
.map_err(|e| ApiError::Internal(format!("Failed to run ffmpeg: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Failed to run ffmpeg: {}", e)))?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
@ -745,7 +745,7 @@ async fn generate_arc_a770_segment(
])
.output()
.await
.map_err(|e| ApiError::Internal(format!("Failed to run Arc A770 ffmpeg: {}", e)))?;
.map_err(|e| ApiError::media_processing_failed(format!("Failed to run Arc A770 ffmpeg: {}", e)))?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
@ -811,7 +811,7 @@ pub async fn serve_thumbnail(
// Update database with thumbnail path
let database_url = std::env::var("DATABASE_URL")
.map_err(|_| ApiError::Internal("DATABASE_URL not set".to_string()))?;
.map_err(|_| ApiError::missing_config("DATABASE_URL"))?;
let pool = sqlx::PgPool::connect(&database_url).await
.map_err(|e| ApiError::Database(e.to_string()))?;

2
src/utils/datetime.rs
View file

@ -1,4 +1,4 @@
use chrono::{DateTime, NaiveDate, NaiveDateTime, TimeZone, Utc, Datelike};
use chrono::{DateTime, NaiveDate, NaiveDateTime, TimeZone, Utc};
use chrono_tz::Tz;
use serde::{Deserialize, Serialize};
use crate::error::{ApiError, Result};

10
src/utils/db_operations.rs
View file

@ -81,7 +81,10 @@ impl DbOperations {
)
.fetch_optional(pool)
.await
.map_err(ApiError::DatabaseError)
.map_err(|e| {
tracing::error!("Failed to get bulletin {}: {}", id, e);
ApiError::DatabaseError(e)
})
}
/// Generic get by ID operation for events specifically
@ -92,7 +95,10 @@ impl DbOperations {
sqlx::query_as!(Event, "SELECT * FROM events WHERE id = $1", id)
.fetch_optional(pool)
.await
.map_err(ApiError::DatabaseError)
.map_err(|e| {
tracing::error!("Failed to get event {}: {}", id, e);
ApiError::DatabaseError(e)
})
}
/// Delete bulletin by ID