Merge pull request #134 from chrivers/chrivers/apiv1-error-handling-rework

Rework api V1 error handling.

Previously, any non-successful api operation on the legacy api would yield incorrect error responses. Yes, even the errors were in error!

This did not help application compatibility, but the problem is now fixed consistently.

Author: chrivers

crates/hue/src/error.rs

@@ -38,9 +38,6 @@ pub enum HueError {
     #[error("Failed to decode Hue Zigbee Update: Unknown flags {0:04x}")]
     HueZigbeeUnknownFlags(u16),
 
-    #[error("State changes not supported for: {0:?}")]
-    UpdateUnsupported(RType),
-
     #[error("Resource {0} not found")]
     NotFound(uuid::Uuid),
 
@@ -57,4 +54,59 @@ pub enum HueError {
     Undiffable,
 }
 
+/// Error types for Hue Bridge v1 API
+#[derive(Error, Debug, Clone, Copy)]
+pub enum HueApiV1Error {
+    /// Type 1
+    #[error("Unauthorized")]
+    UnauthorizedUser = 1,
+
+    /// Type 2
+    #[error("Body contains invalid JSON")]
+    BodyContainsInvalidJson = 2,
+
+    /// Type 3
+    #[error("Resource not found")]
+    ResourceNotfound = 3,
+
+    /// Type 4
+    #[error("Method not available for resource")]
+    MethodNotAvailableForResource = 4,
+
+    /// Type 5
+    #[error("Missing parameters in body")]
+    MissingParametersInBody = 5,
+
+    /// Type 6
+    #[error("Parameter not available")]
+    ParameterNotAvailable = 6,
+
+    /// Type 7
+    #[error("Invalid value for parameter")]
+    InvalidValueForParameter = 7,
+
+    /// Type 8
+    #[error("Parameter not modifiable")]
+    ParameterNotModifiable = 8,
+
+    /// Type 11
+    #[error("Too many items in list")]
+    TooManyItemsInList = 11,
+
+    /// Type 12
+    #[error("Portal connection is required")]
+    PortalConnectionIsRequired = 12,
+
+    /// Type 901
+    #[error("Internal bridge error")]
+    BridgeInternalError = 901,
+}
+
+impl HueApiV1Error {
+    #[must_use]
+    pub const fn error_code(&self) -> u32 {
+        *self as u32
+    }
+}
+
 pub type HueResult<T> = Result<T, HueError>;

crates/hue/src/legacy_api.rs

@@ -117,6 +117,7 @@ pub struct NewUser {
 #[derive(Debug, Serialize, Deserialize)]
 pub struct NewUserReply {
     pub username: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub clientkey: Option<String>,
 }
 

src/error.rs

@@ -8,7 +8,6 @@ use tokio::task::JoinError;
 
 use bifrost_api::backend::BackendRequest;
 use hue::event::EventBlock;
-use hue::legacy_api::ApiResourceType;
 use svc::error::SvcError;
 
 #[derive(Error, Debug)]
@@ -120,10 +119,6 @@ pub enum ApiError {
     #[error("Unexpected z2m message: {0:?}")]
     UnexpectedZ2mReply(tokio_tungstenite::tungstenite::Message),
 
-    /* hue api v1 errors */
-    #[error("Cannot create resources of type: {0:?}")]
-    V1CreateUnsupported(ApiResourceType),
-
     /* hue api v2 errors */
     #[error("Failed to get firmware version reply from update server")]
     NoUpdateInformation,

src/routes/api.rs

@@ -2,11 +2,11 @@ use std::collections::HashMap;
 
 use axum::Router;
 use axum::extract::{Path, State};
-use axum::response::IntoResponse;
 use axum::routing::{get, post, put};
 use bytes::Bytes;
 use chrono::Utc;
 use log::{info, warn};
+use serde::Serialize;
 use serde_json::{Value, json};
 use tokio::sync::MutexGuard;
 use uuid::Uuid;
@@ -17,7 +17,7 @@ use hue::api::{
     GroupedLightUpdate, Light, LightUpdate, RType, Resource, ResourceLink, Room, Scene,
     SceneActive, SceneStatus, SceneUpdate, V1Reply,
 };
-use hue::error::{HueError, HueResult};
+use hue::error::{HueApiV1Error, HueError, HueResult};
 use hue::legacy_api::{
     ApiGroup, ApiGroupActionUpdate, ApiGroupUpdate2, ApiLight, ApiLightStateUpdate,
     ApiResourceType, ApiScene, ApiSceneAppData, ApiSceneType, ApiSceneVersion, ApiSensor,
@@ -28,13 +28,14 @@ use crate::error::{ApiError, ApiResult};
 use crate::resource::Resources;
 use crate::routes::auth::STANDARD_CLIENT_KEY;
 use crate::routes::extractor::Json;
+use crate::routes::{ApiV1Error, ApiV1Result};
 use crate::server::appstate::AppState;
 
-async fn get_api_config(State(state): State<AppState>) -> impl IntoResponse {
+async fn get_api_config(State(state): State<AppState>) -> Json<impl Serialize> {
     Json(state.api_short_config().await)
 }
 
-async fn post_api(bytes: Bytes) -> ApiResult<impl IntoResponse> {
+async fn post_api(bytes: Bytes) -> ApiV1Result<Json<impl Serialize>> {
     info!("post: {bytes:?}");
     let json: NewUser = serde_json::from_slice(&bytes)?;
 
@@ -105,7 +106,7 @@ fn get_groups(res: &MutexGuard<Resources>, group_0: bool) -> ApiResult<HashMap<S
     Ok(rooms)
 }
 
-pub fn get_scene(res: &Resources, owner: String, scene: &Scene) -> ApiResult<ApiScene> {
+pub fn get_scene(res: &Resources, owner: String, scene: &Scene) -> ApiV1Result<ApiScene> {
     let lights = scene
         .actions
         .iter()
@@ -121,7 +122,7 @@ pub fn get_scene(res: &Resources, owner: String, scene: &Scene) -> ApiResult<Api
                 ApiLightStateUpdate::from(sae.action.clone()),
             ))
         })
-        .collect::<ApiResult<_>>()?;
+        .collect::<ApiV1Result<_>>()?;
 
     let room_id = res.get_id_v1_index(scene.group.rid)?;
 
@@ -146,7 +147,7 @@ pub fn get_scene(res: &Resources, owner: String, scene: &Scene) -> ApiResult<Api
     })
 }
 
-fn get_scenes(owner: &str, res: &MutexGuard<Resources>) -> ApiResult<HashMap<String, ApiScene>> {
+fn get_scenes(owner: &str, res: &MutexGuard<Resources>) -> ApiV1Result<HashMap<String, ApiScene>> {
     let mut scenes = HashMap::new();
 
     for rr in res.get_resources_by_type(RType::Scene) {
@@ -165,7 +166,7 @@ fn get_scenes(owner: &str, res: &MutexGuard<Resources>) -> ApiResult<HashMap<Str
 async fn get_api_user(
     state: State<AppState>,
     Path(username): Path<String>,
-) -> ApiResult<impl IntoResponse> {
+) -> ApiV1Result<Json<impl Serialize>> {
     let lock = state.res.lock().await;
 
     Ok(Json(ApiUserConfig {
@@ -183,7 +184,7 @@ async fn get_api_user(
 async fn get_api_user_resource(
     State(state): State<AppState>,
     Path((username, artype)): Path<(String, ApiResourceType)>,
-) -> ApiResult<Json<Value>> {
+) -> ApiV1Result<Json<Value>> {
     let lock = &state.res.lock().await;
     match artype {
         ApiResourceType::Config => Ok(Json(json!(state.api_config(username).await?))),
@@ -201,26 +202,26 @@ async fn get_api_user_resource(
 async fn post_api_user_resource(
     Path((_username, resource)): Path<(String, ApiResourceType)>,
     Json(req): Json<Value>,
-) -> ApiResult<Json<Value>> {
+) -> ApiV1Result<Json<Value>> {
     warn!("POST v1 user resource unsupported");
     warn!("Request: {req:?}");
-    Err(ApiError::V1CreateUnsupported(resource))
+    Err(ApiV1Error::V1CreateUnsupported(resource))
 }
 
 async fn put_api_user_resource(
     Path((_username, _resource)): Path<(String, String)>,
     Json(req): Json<Value>,
-) -> impl IntoResponse {
+) -> ApiV1Result<Json<impl Serialize>> {
     warn!("PUT v1 user resource {req:?}");
     //Json(format!("user {username} resource {resource}"))
-    Json(vec![HueApiResult::Success(req)])
+    Ok(Json(vec![HueApiResult::Success(req)]))
 }
 
 #[allow(clippy::significant_drop_tightening)]
 async fn get_api_user_resource_id(
     State(state): State<AppState>,
     Path((username, resource, id)): Path<(String, ApiResourceType, u32)>,
-) -> ApiResult<impl IntoResponse> {
+) -> ApiV1Result<Json<impl Serialize>> {
     log::debug!("GET v1 username={username} resource={resource:?} id={id}");
     let result = match resource {
         ApiResourceType::Lights => {
@@ -260,7 +261,7 @@ async fn put_api_user_resource_id(
     State(state): State<AppState>,
     Path((username, artype, id)): Path<(String, ApiResourceType, u32)>,
     Json(req): Json<Value>,
-) -> ApiResult<impl IntoResponse> {
+) -> ApiV1Result<Json<Value>> {
     log::debug!("PUT v1 username={username} resource={artype:?} id={id}");
     log::debug!("JSON: {req:?}");
     match artype {
@@ -292,15 +293,15 @@ async fn put_api_user_resource_id(
         | ApiResourceType::Scenes
         | ApiResourceType::Schedules
         | ApiResourceType::Sensors
-        | ApiResourceType::Capabilities => Err(ApiError::V1CreateUnsupported(artype)),
+        | ApiResourceType::Capabilities => Err(ApiV1Error::V1CreateUnsupported(artype)),
     }
 }
 
 async fn put_api_user_resource_id_path(
     State(state): State<AppState>,
     Path((_username, artype, id, path)): Path<(String, ApiResourceType, u32, String)>,
     Json(req): Json<Value>,
-) -> ApiResult<Json<Value>> {
+) -> ApiV1Result<Json<Value>> {
     match artype {
         ApiResourceType::Lights => {
             log::debug!("req: {}", serde_json::to_string_pretty(&req)?);
@@ -349,7 +350,7 @@ async fn put_api_user_resource_id_path(
                     V1Reply::for_group_path(id, &path).with_light_state_update(&upd)?
                 }
                 ApiGroupActionUpdate::GroupUpdate(upd) => {
-                    let scene_id = upd.scene.parse()?;
+                    let scene_id = upd.scene.parse().map_err(ApiError::ParseIntError)?;
                     let scene_uuid = lock.from_id_v1(scene_id)?;
                     let rlink = RType::Scene.link_to(scene_uuid);
                     let updv2 = SceneUpdate::new().with_recall_action(Some(SceneStatus {
@@ -404,7 +405,7 @@ async fn put_api_user_resource_id_path(
         | ApiResourceType::Scenes
         | ApiResourceType::Schedules
         | ApiResourceType::Sensors
-        | ApiResourceType::Capabilities => Err(ApiError::V1CreateUnsupported(artype)),
+        | ApiResourceType::Capabilities => Err(ApiV1Error::V1CreateUnsupported(artype)),
     }
 }
 
@@ -414,8 +415,8 @@ async fn put_api_user_resource_id_path(
 /// even though this does not seem to ever have been a valid hue endpoint.
 ///
 /// 2025-01-24: This response has been confirmed to work by Alexa and Peter Miller on discord.
-pub async fn workaround_iconnect_hue() -> impl IntoResponse {
-    Json(json!([{"error":{"type":1,"address":"/","description":"unauthorized user"}}]))
+pub async fn workaround_iconnect_hue() -> ApiV1Result<()> {
+    Err(HueApiV1Error::UnauthorizedUser)?
 }
 
 pub fn router() -> Router<AppState> {

src/routes/clip/mod.rs

@@ -9,7 +9,6 @@ use entertainment_configuration as ent_conf;
 
 use axum::Router;
 use axum::extract::{Path, State};
-use axum::response::IntoResponse;
 use axum::routing::{delete, get, post, put};
 use hue::api::{RType, ResourceLink};
 use serde::{Deserialize, Serialize};
@@ -51,7 +50,7 @@ impl<T: Serialize> V2Reply<T> {
     }
 }
 
-async fn get_all_resources(State(state): State<AppState>) -> impl IntoResponse {
+async fn get_all_resources(State(state): State<AppState>) -> ApiV2Result {
     let lock = state.res.lock().await;
     let res = lock.get_resources();
     drop(lock);
@@ -69,7 +68,7 @@ async fn post_resource(
     State(state): State<AppState>,
     Path(rtype): Path<RType>,
     Json(req): Json<Value>,
-) -> impl IntoResponse {
+) -> ApiV2Result {
     log::info!("POST {rtype:?}");
     log::debug!("Json data:\n{}", serde_json::to_string_pretty(&req)?);