From 53e2b4841d8102497de563936abd683cec034a9b Mon Sep 17 00:00:00 2001 From: leo Date: Wed, 23 Oct 2024 21:55:45 +0800 Subject: [PATCH] feat: refine error message in dev server (#183) Co-authored-by: Hu Yueh-Wei --- .vscode/settings.json | 6 - .../src/cmd/cmd_check/cmd_check_graph.rs | 14 ++- .../src/dev_server/addons/extensions.rs | 7 +- .../src/dev_server/graphs/connections.rs | 7 +- .../ten_manager/src/dev_server/graphs/mod.rs | 9 +- .../src/dev_server/graphs/nodes.rs | 39 +++--- .../src/dev_server/graphs/update.rs | 16 +-- .../src/dev_server/manifest/check.rs | 19 +-- .../src/dev_server/manifest/dump.rs | 17 +-- .../src/dev_server/messages/compatible.rs | 117 +++++++++--------- core/src/ten_manager/src/dev_server/mod.rs | 2 +- .../src/dev_server/packages/reload.rs | 8 +- .../src/dev_server/property/check.rs | 19 +-- .../src/dev_server/property/dump.rs | 19 +-- .../ten_manager/src/dev_server/response.rs | 28 ++++- core/src/ten_manager/src/lib.rs | 2 +- core/src/ten_manager/tests/cmd_check_graph.rs | 3 +- core/src/ten_manager/tests/cmd_dev_server.rs | 91 ++++++++++++++ .../src/ten_manager/tests/integration_test.rs | 1 + .../manifest.json | 22 ++++ .../extension/addon_a/manifest.json | 18 +++ .../extension/addon_b/manifest.json | 18 +++ .../manifest.json | 22 ++++ .../property.json | 42 +++++++ .../extension/addon_a/manifest.json | 18 +++ .../extension/addon_b/manifest.json | 18 +++ core/src/ten_rust/src/pkg_info/binding.rs | 2 +- .../src/ten_rust/src/pkg_info/manifest/mod.rs | 20 ++- core/src/ten_rust/src/pkg_info/mod.rs | 21 +--- .../src/ten_rust/src/pkg_info/property/mod.rs | 29 ++--- 30 files changed, 437 insertions(+), 217 deletions(-) create mode 100644 core/src/ten_manager/tests/cmd_dev_server.rs create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/manifest.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_a/manifest.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_b/manifest.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/manifest.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/property.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_a/manifest.json create mode 100644 core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_b/manifest.json diff --git a/.vscode/settings.json b/.vscode/settings.json index fa28d7473..79f0ccfd1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -30,12 +30,6 @@ "**/out/*/**": true, "**/third_party/*/**": true }, - "go.formatFlags": [ - "-m", - "80", - "--shorten-comments", - "-w" - ], "go.inlayHints.constantValues": true, "go.inlayHints.parameterNames": true, "go.inlayHints.rangeVariableTypes": true, diff --git a/core/src/ten_manager/src/cmd/cmd_check/cmd_check_graph.rs b/core/src/ten_manager/src/cmd/cmd_check/cmd_check_graph.rs index 7715dc7bf..216efe417 100644 --- a/core/src/ten_manager/src/cmd/cmd_check/cmd_check_graph.rs +++ b/core/src/ten_manager/src/cmd/cmd_check/cmd_check_graph.rs @@ -105,10 +105,15 @@ fn get_existed_pkgs_of_all_apps( for app in &command.app { let app_path = path::Path::new(app); - let app_property = parse_property_in_folder(app_path)?; let app_existed_pkgs = get_all_existed_pkgs_info_of_app(app_path)?; - let app_uri = app_property.get_app_uri(); + let app_property = parse_property_in_folder(app_path)?; + let app_uri = if let Some(property) = app_property { + property.get_app_uri() + } else { + localhost() + }; + if !single_app && app_uri.as_str() == localhost() { return Err(anyhow::anyhow!( "The app uri should be some string other than 'localhost' when @@ -139,7 +144,12 @@ fn get_graphs_to_be_checked(command: &CheckGraphCommand) -> Result> { } else { let first_app_path = path::Path::new(&command.app[0]); let first_app_property = parse_property_in_folder(first_app_path)?; + if first_app_property.is_none() { + return Err(anyhow::anyhow!("The property.json is not found in the first app, which is required to retrieve the predefined graphs.")); + } + let predefined_graphs = first_app_property + .unwrap() ._ten .and_then(|p| p.predefined_graphs) .ok_or_else(|| { diff --git a/core/src/ten_manager/src/dev_server/addons/extensions.rs b/core/src/ten_manager/src/dev_server/addons/extensions.rs index ab2df62e3..6994620ef 100644 --- a/core/src/ten_manager/src/dev_server/addons/extensions.rs +++ b/core/src/ten_manager/src/dev_server/addons/extensions.rs @@ -38,11 +38,8 @@ pub async fn get_extension_addons( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } diff --git a/core/src/ten_manager/src/dev_server/graphs/connections.rs b/core/src/ten_manager/src/dev_server/graphs/connections.rs index 179042992..07b457d8c 100644 --- a/core/src/ten_manager/src/dev_server/graphs/connections.rs +++ b/core/src/ten_manager/src/dev_server/graphs/connections.rs @@ -116,11 +116,8 @@ pub async fn get_graph_connections( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } diff --git a/core/src/ten_manager/src/dev_server/graphs/mod.rs b/core/src/ten_manager/src/dev_server/graphs/mod.rs index 4ff54fd8c..65250fa56 100644 --- a/core/src/ten_manager/src/dev_server/graphs/mod.rs +++ b/core/src/ten_manager/src/dev_server/graphs/mod.rs @@ -21,7 +21,7 @@ use super::{ use ten_rust::pkg_info::pkg_type::PkgType; #[derive(Serialize, Deserialize, Debug, PartialEq)] -struct RespGraph { +pub struct RespGraph { name: String, auto_start: bool, } @@ -33,11 +33,8 @@ pub async fn get_graphs( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return Ok(HttpResponse::NotFound().json(error_response)); } diff --git a/core/src/ten_manager/src/dev_server/graphs/nodes.rs b/core/src/ten_manager/src/dev_server/graphs/nodes.rs index 33966f208..14515604f 100644 --- a/core/src/ten_manager/src/dev_server/graphs/nodes.rs +++ b/core/src/ten_manager/src/dev_server/graphs/nodes.rs @@ -214,11 +214,8 @@ pub async fn get_graph_nodes( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -227,14 +224,14 @@ pub async fn get_graph_nodes( match get_extension_nodes_in_graph(&graph_name, all_pkgs) { Ok(exts) => exts, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Error fetching runtime extensions for graph '{}': {}", - graph_name, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Error fetching runtime extensions for graph '{}'", + graph_name + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } }; @@ -243,14 +240,14 @@ pub async fn get_graph_nodes( for extension in &extensions { let pkg_info = get_pkg_info_for_extension(extension, all_pkgs); if let Err(err) = pkg_info { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Error fetching runtime extensions for graph '{}': {}", - graph_name, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Error fetching runtime extensions for graph '{}'", + graph_name + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } diff --git a/core/src/ten_manager/src/dev_server/graphs/update.rs b/core/src/ten_manager/src/dev_server/graphs/update.rs index 9f7368f6b..6b1d9bdab 100644 --- a/core/src/ten_manager/src/dev_server/graphs/update.rs +++ b/core/src/ten_manager/src/dev_server/graphs/update.rs @@ -37,11 +37,8 @@ pub async fn update_graph( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -64,11 +61,10 @@ pub async fn update_graph( ) { Ok(graph) => graph, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Invalid input data: {}", err), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + "Invalid input data:", + ); return HttpResponse::NotFound().json(error_response); } }; diff --git a/core/src/ten_manager/src/dev_server/manifest/check.rs b/core/src/ten_manager/src/dev_server/manifest/check.rs index b31830b83..985a9f779 100644 --- a/core/src/ten_manager/src/dev_server/manifest/check.rs +++ b/core/src/ten_manager/src/dev_server/manifest/check.rs @@ -30,11 +30,8 @@ pub async fn check_manifest( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -55,14 +52,10 @@ pub async fn check_manifest( HttpResponse::Ok().json(response) } Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to check manifest: {}", - err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + "Failed to check manifest:", + ); HttpResponse::NotFound().json(error_response) } }, diff --git a/core/src/ten_manager/src/dev_server/manifest/dump.rs b/core/src/ten_manager/src/dev_server/manifest/dump.rs index afd0b7df5..38ca948e0 100644 --- a/core/src/ten_manager/src/dev_server/manifest/dump.rs +++ b/core/src/ten_manager/src/dev_server/manifest/dump.rs @@ -35,11 +35,8 @@ pub async fn dump_manifest( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -59,12 +56,10 @@ pub async fn dump_manifest( if let Err(err) = dump_manifest_str_to_file(&new_manifest_str, manifest_file_path) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Failed to dump new manifest content to manifest file: {}", - err), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + "Failed to dump new manifest content to manifest file:", + ); return HttpResponse::NotFound().json(error_response); } diff --git a/core/src/ten_manager/src/dev_server/messages/compatible.rs b/core/src/ten_manager/src/dev_server/messages/compatible.rs index e97124e87..9565dd90b 100644 --- a/core/src/ten_manager/src/dev_server/messages/compatible.rs +++ b/core/src/ten_manager/src/dev_server/messages/compatible.rs @@ -94,11 +94,8 @@ pub async fn get_compatible_messages( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -107,14 +104,14 @@ pub async fn get_compatible_messages( match get_extension_nodes_in_graph(&input.graph, all_pkgs) { Ok(exts) => exts, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Error fetching runtime extensions for graph: {}: {}", - input.graph, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Error fetching runtime extensions for graph '{}'", + input.graph + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } }; @@ -127,14 +124,15 @@ pub async fn get_compatible_messages( ) { Ok(ext) => ext, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to find the extension: {}: {}", - input.extension, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Failed to find the extension: {}", + input.extension + ) + .as_str(), + ); + return HttpResponse::NotFound().json(error_response); } }; @@ -142,14 +140,11 @@ pub async fn get_compatible_messages( let msg_ty = match MsgType::from_str(&input.msg_type) { Ok(msg_ty) => msg_ty, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Unsupported message type: {}: {}", - input.msg_type, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!("Unsupported message type: {}", input.msg_type) + .as_str(), + ); return HttpResponse::InternalServerError() .json(error_response); } @@ -158,14 +153,14 @@ pub async fn get_compatible_messages( let msg_dir = match MsgDirection::from_str(&input.msg_direction) { Ok(msg_dir) => msg_dir, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Unsupported message direction: {}: {}", - input.msg_direction, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Unsupported message direction: {}", + input.msg_direction + ) + .as_str(), + ); return HttpResponse::InternalServerError() .json(error_response); } @@ -176,14 +171,14 @@ pub async fn get_compatible_messages( let pkg_info = get_pkg_info_for_extension(extension, all_pkgs); if let Err(err) = pkg_info { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Error fetching runtime extensions for graph: {}: {}", - input.graph, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Error fetching runtime extensions for graph '{}'", + input.graph + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } @@ -212,14 +207,14 @@ pub async fn get_compatible_messages( ) { Ok(results) => results, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to find compatible cmd/{}: {}", - input.msg_name, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Failed to find compatible cmd/{}:", + input.msg_name + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } }; @@ -268,14 +263,14 @@ pub async fn get_compatible_messages( ) { Ok(results) => results, Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to find compatible cmd/{}: {}", - input.msg_name, err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + format!( + "Failed to find compatible cmd/{}:", + input.msg_name + ) + .as_str(), + ); return HttpResponse::NotFound().json(error_response); } }; diff --git a/core/src/ten_manager/src/dev_server/mod.rs b/core/src/ten_manager/src/dev_server/mod.rs index 42f776006..f5672bb2a 100644 --- a/core/src/ten_manager/src/dev_server/mod.rs +++ b/core/src/ten_manager/src/dev_server/mod.rs @@ -13,7 +13,7 @@ mod messages; mod mock; mod packages; mod property; -mod response; +pub mod response; mod version; use std::sync::{Arc, RwLock}; diff --git a/core/src/ten_manager/src/dev_server/packages/reload.rs b/core/src/ten_manager/src/dev_server/packages/reload.rs index 288c76585..58294998f 100644 --- a/core/src/ten_manager/src/dev_server/packages/reload.rs +++ b/core/src/ten_manager/src/dev_server/packages/reload.rs @@ -29,10 +29,8 @@ pub async fn clear_and_reload_pkgs( data: "Packages reloaded successfully", meta: None, }), - Err(err) => HttpResponse::InternalServerError().json(ErrorResponse { - status: Status::Fail, - message: format!("Failed to reload packages: {}", err), - error: None, - }), + Err(err) => HttpResponse::InternalServerError().json( + ErrorResponse::from_error(&err, "Failed to reload packages:"), + ), } } diff --git a/core/src/ten_manager/src/dev_server/property/check.rs b/core/src/ten_manager/src/dev_server/property/check.rs index b07da2316..4b93a42d1 100644 --- a/core/src/ten_manager/src/dev_server/property/check.rs +++ b/core/src/ten_manager/src/dev_server/property/check.rs @@ -30,11 +30,8 @@ pub async fn check_property( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -55,14 +52,10 @@ pub async fn check_property( HttpResponse::Ok().json(response) } Err(err) => { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to check property: {}", - err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + "Failed to check property:", + ); HttpResponse::NotFound().json(error_response) } }, diff --git a/core/src/ten_manager/src/dev_server/property/dump.rs b/core/src/ten_manager/src/dev_server/property/dump.rs index dc2b3b980..68863db00 100644 --- a/core/src/ten_manager/src/dev_server/property/dump.rs +++ b/core/src/ten_manager/src/dev_server/property/dump.rs @@ -34,11 +34,8 @@ pub async fn dump_property( // Fetch all packages if not already done. if let Err(err) = get_all_pkgs(&mut state) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!("Error fetching packages: {}", err), - error: None, - }; + let error_response = + ErrorResponse::from_error(&err, "Error fetching packages:"); return HttpResponse::NotFound().json(error_response); } @@ -66,14 +63,10 @@ pub async fn dump_property( if let Err(err) = app_prop.dump_property_to_file(&property_file_path) { - let error_response = ErrorResponse { - status: Status::Fail, - message: format!( - "Failed to dump new content to property.json: {}", - err - ), - error: None, - }; + let error_response = ErrorResponse::from_error( + &err, + "Failed to dump new content to property.json:", + ); return HttpResponse::NotFound().json(error_response); } diff --git a/core/src/ten_manager/src/dev_server/response.rs b/core/src/ten_manager/src/dev_server/response.rs index 6bd0a9ef3..97d0eeccb 100644 --- a/core/src/ten_manager/src/dev_server/response.rs +++ b/core/src/ten_manager/src/dev_server/response.rs @@ -4,7 +4,7 @@ // Licensed under the Apache License, Version 2.0, with certain conditions. // Refer to the "LICENSE" file in the root directory for more information. // -use anyhow::Result; +use anyhow::{Error, Result}; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug, Clone)] @@ -68,3 +68,29 @@ pub struct InnerError { pub code: Option, pub message: String, } + +impl ErrorResponse { + pub fn from_error(err: &Error, message_header: &str) -> Self { + let mut this = ErrorResponse { + status: Status::Fail, + message: format!("{} {}", message_header, err), + error: None, + }; + + let mut errors = vec![]; + + err.chain().skip(1).for_each(|cause| { + errors.push(cause.to_string()); + }); + + if !errors.is_empty() { + this.error = Some(InnerError { + error_type: "root cause".to_string(), + code: None, + message: errors.join("\n"), + }); + } + + this + } +} diff --git a/core/src/ten_manager/src/lib.rs b/core/src/ten_manager/src/lib.rs index db8645d73..f7583eaab 100644 --- a/core/src/ten_manager/src/lib.rs +++ b/core/src/ten_manager/src/lib.rs @@ -26,7 +26,7 @@ pub mod cmd_line; pub mod config; pub mod constants; mod dep_and_candidate; -mod dev_server; +pub mod dev_server; mod error; mod fs; mod install; diff --git a/core/src/ten_manager/tests/cmd_check_graph.rs b/core/src/ten_manager/tests/cmd_check_graph.rs index 5c8dbc38d..80992c29a 100644 --- a/core/src/ten_manager/tests/cmd_check_graph.rs +++ b/core/src/ten_manager/tests/cmd_check_graph.rs @@ -99,10 +99,11 @@ async fn test_cmd_check_app_in_graph_cannot_be_localhost() { ) .await; + // 'localhost' is not allowed in graph definition. assert!(result.is_err()); eprintln!("{:?}", result); - let msg = result.err().unwrap().to_string(); + let msg = format!("{:?}", result.err().unwrap()); assert!(msg.contains("'localhost' is not allowed in graph definition")); } diff --git a/core/src/ten_manager/tests/cmd_dev_server.rs b/core/src/ten_manager/tests/cmd_dev_server.rs new file mode 100644 index 000000000..81ee08423 --- /dev/null +++ b/core/src/ten_manager/tests/cmd_dev_server.rs @@ -0,0 +1,91 @@ +// +// Copyright © 2024 Agora +// This file is part of TEN Framework, an open source project. +// Licensed under the Apache License, Version 2.0, with certain conditions. +// Refer to the "LICENSE" file in the root directory for more information. +// +use std::sync::{Arc, RwLock}; + +use actix_web::{http::StatusCode, test, web, App}; + +use ten_manager::{ + config::TmanConfig, + dev_server::{ + graphs::{get_graphs, RespGraph}, + response::ApiResponse, + DevServerState, + }, +}; + +#[actix_rt::test] +async fn test_cmd_dev_server_graphs_some_property_invalid() { + let dev_server_state = DevServerState { + base_dir: Some( + "tests/test_data/cmd_dev_server_graphs_some_property_invalid" + .to_string(), + ), + all_pkgs: None, + tman_config: TmanConfig::default(), + }; + + let dev_server_state = Arc::new(RwLock::new(dev_server_state)); + let app = test::init_service( + App::new() + .app_data(web::Data::new(dev_server_state)) + .route("/api/dev-server/v1/graphs", web::get().to(get_graphs)), + ) + .await; + + let req = test::TestRequest::get() + .uri("/api/dev-server/v1/graphs") + .to_request(); + let resp = test::call_service(&app, req).await; + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + + let body = test::read_body(resp).await; + let body_str = std::str::from_utf8(&body).unwrap(); + let json: serde_json::Value = serde_json::from_str(body_str).unwrap(); + + let pretty_json = serde_json::to_string_pretty(&json).unwrap(); + println!("Response body: {}", pretty_json); + + let root_cause = json["error"]["message"].as_str().unwrap(); + assert!(root_cause + .contains("the app uri should be some string other than 'localhost'")); +} + +#[actix_rt::test] +async fn test_cmd_dev_server_graphs_app_property_not_exist() { + let dev_server_state = DevServerState { + base_dir: Some( + "tests/test_data/cmd_dev_server_graphs_app_property_not_exist" + .to_string(), + ), + all_pkgs: None, + tman_config: TmanConfig::default(), + }; + + let dev_server_state = Arc::new(RwLock::new(dev_server_state)); + let app = test::init_service( + App::new() + .app_data(web::Data::new(dev_server_state)) + .route("/api/dev-server/v1/graphs", web::get().to(get_graphs)), + ) + .await; + + let req = test::TestRequest::get() + .uri("/api/dev-server/v1/graphs") + .to_request(); + let resp = test::call_service(&app, req).await; + assert_eq!(resp.status(), StatusCode::OK); + + let body = test::read_body(resp).await; + let body_str = std::str::from_utf8(&body).unwrap(); + let json = + serde_json::from_str::>>(body_str).unwrap(); + + let pretty_json = serde_json::to_string_pretty(&json).unwrap(); + println!("Response body: {}", pretty_json); + + assert!(json.data.is_empty()); +} diff --git a/core/src/ten_manager/tests/integration_test.rs b/core/src/ten_manager/tests/integration_test.rs index 42a5f0f6a..4768d0d2a 100644 --- a/core/src/ten_manager/tests/integration_test.rs +++ b/core/src/ten_manager/tests/integration_test.rs @@ -16,3 +16,4 @@ fn main() { // Those following mods will be compiled in one executable. mod cmd_check_graph; +mod cmd_dev_server; diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/manifest.json new file mode 100644 index 000000000..7cb8a2ca0 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/manifest.json @@ -0,0 +1,22 @@ +{ + "type": "app", + "name": "check_predefined_graph_success", + "version": "0.1.0", + "dependencies": [ + { + "type": "extension", + "name": "addon_a", + "version": "0.1.0" + }, + { + "type": "extension", + "name": "addon_b", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + } +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_a/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_a/manifest.json new file mode 100644 index 000000000..05434a564 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_a/manifest.json @@ -0,0 +1,18 @@ +{ + "type": "extension", + "name": "addon_a", + "version": "0.1.0", + "dependencies": [ + { + "type": "system", + "name": "ten_runtime_go", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + }, + "api": {} +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_b/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_b/manifest.json new file mode 100644 index 000000000..9821e29f4 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_app_property_not_exist/ten_packages/extension/addon_b/manifest.json @@ -0,0 +1,18 @@ +{ + "type": "extension", + "name": "addon_b", + "version": "0.1.0", + "dependencies": [ + { + "type": "system", + "name": "ten_runtime_go", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + }, + "api": {} +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/manifest.json new file mode 100644 index 000000000..7cb8a2ca0 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/manifest.json @@ -0,0 +1,22 @@ +{ + "type": "app", + "name": "check_predefined_graph_success", + "version": "0.1.0", + "dependencies": [ + { + "type": "extension", + "name": "addon_a", + "version": "0.1.0" + }, + { + "type": "extension", + "name": "addon_b", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + } +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/property.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/property.json new file mode 100644 index 000000000..b40219aa5 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/property.json @@ -0,0 +1,42 @@ +{ + "_ten": { + "predefined_graphs": [ + { + "name": "default", + "auto_start": false, + "nodes": [ + { + "type": "extension", + "name": "ext_a", + "addon": "addon_a", + "extension_group": "some_group" + }, + { + "type": "extension", + "name": "ext_b", + "addon": "addon_b", + "extension_group": "some_group", + "app": "localhost" + } + ], + "connections": [ + { + "extension_group": "some_group", + "extension": "ext_a", + "cmd": [ + { + "name": "cmd_1", + "dest": [ + { + "extension_group": "some_group", + "extension": "ext_b" + } + ] + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_a/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_a/manifest.json new file mode 100644 index 000000000..05434a564 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_a/manifest.json @@ -0,0 +1,18 @@ +{ + "type": "extension", + "name": "addon_a", + "version": "0.1.0", + "dependencies": [ + { + "type": "system", + "name": "ten_runtime_go", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + }, + "api": {} +} \ No newline at end of file diff --git a/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_b/manifest.json b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_b/manifest.json new file mode 100644 index 000000000..9821e29f4 --- /dev/null +++ b/core/src/ten_manager/tests/test_data/cmd_dev_server_graphs_some_property_invalid/ten_packages/extension/addon_b/manifest.json @@ -0,0 +1,18 @@ +{ + "type": "extension", + "name": "addon_b", + "version": "0.1.0", + "dependencies": [ + { + "type": "system", + "name": "ten_runtime_go", + "version": "0.1.0" + } + ], + "package": { + "include": [ + "**" + ] + }, + "api": {} +} \ No newline at end of file diff --git a/core/src/ten_rust/src/pkg_info/binding.rs b/core/src/ten_rust/src/pkg_info/binding.rs index 089ed73a5..85bd088c7 100644 --- a/core/src/ten_rust/src/pkg_info/binding.rs +++ b/core/src/ten_rust/src/pkg_info/binding.rs @@ -31,7 +31,7 @@ pub extern "C" fn ten_rust_check_graph_for_app( rust_app_uri, ); if ret.is_err() { - let err_msg = ret.err().unwrap().to_string(); + let err_msg = format!("{:?}", ret.err().unwrap()); let c_err_msg = CString::new(err_msg).expect("Failed to allocate memory."); unsafe { diff --git a/core/src/ten_rust/src/pkg_info/manifest/mod.rs b/core/src/ten_rust/src/pkg_info/manifest/mod.rs index 7bcb939b6..08e636ca8 100644 --- a/core/src/ten_rust/src/pkg_info/manifest/mod.rs +++ b/core/src/ten_rust/src/pkg_info/manifest/mod.rs @@ -11,7 +11,7 @@ pub mod support; use std::{fmt, fs, path::Path, str::FromStr}; -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use crate::pkg_info::utils::read_file_to_string; @@ -117,21 +117,17 @@ pub fn parse_manifest_in_folder(folder_path: &Path) -> Result { // Read and parse the manifest.json file. let manifest = crate::pkg_info::manifest::parse_manifest_from_file( manifest_path.clone(), - )?; + ) + .with_context(|| format!("Failed to load {}.", manifest_path.display()))?; // Validate the manifest schema only if it is present. let manifest_path_str = manifest_path.to_owned(); - let validation = json_schema::ten_validate_manifest_json_file( + json_schema::ten_validate_manifest_json_file( manifest_path_str.to_str().unwrap(), - ); - - if let Err(validation_err) = validation { - return Err(anyhow::anyhow!( - "Failed to validate {}, caused by {}", - manifest_path.display(), - validation_err - )); - } + ) + .with_context(|| { + format!("Failed to validate {}.", manifest_path.display()) + })?; Ok(manifest) } diff --git a/core/src/ten_rust/src/pkg_info/mod.rs b/core/src/ten_rust/src/pkg_info/mod.rs index a2ee35cf9..bb87aa04a 100644 --- a/core/src/ten_rust/src/pkg_info/mod.rs +++ b/core/src/ten_rust/src/pkg_info/mod.rs @@ -219,10 +219,7 @@ impl PkgInfo { pub fn get_pkg_info_from_path(pkg_path: &Path) -> Result { let manifest = parse_manifest_in_folder(pkg_path)?; - let property = match parse_property_in_folder(pkg_path) { - Ok(prop) => Some(prop), - Err(_) => None, - }; + let property = parse_property_in_folder(pkg_path)?; let mut pkg_info: PkgInfo = PkgInfo::from_metadata(&manifest, &property)?; @@ -259,17 +256,11 @@ pub fn get_all_existed_pkgs_info_of_app_to_hashset( let mut pkgs_info: HashSet = HashSet::new(); // Process the manifest.json file in the root path. - match collect_pkg_info_from_path(app_path, &mut pkgs_info) { - Ok(manifest) => { - if manifest.pkg_type != APP_PKG_TYPE { - return Err(anyhow::anyhow!(ERR_STR_NOT_APP_DIR)); - } - manifest - } - Err(_) => { - return Err(anyhow::anyhow!(ERR_STR_NOT_APP_DIR)); - } - }; + let app_pkg_manifest = + collect_pkg_info_from_path(app_path, &mut pkgs_info)?; + if app_pkg_manifest.pkg_type != APP_PKG_TYPE { + return Err(anyhow::anyhow!(ERR_STR_NOT_APP_DIR)); + } // Define paths to include manifest.json files from. let addon_types = diff --git a/core/src/ten_rust/src/pkg_info/property/mod.rs b/core/src/ten_rust/src/pkg_info/property/mod.rs index 2e2fccb38..fe43b5f9f 100644 --- a/core/src/ten_rust/src/pkg_info/property/mod.rs +++ b/core/src/ten_rust/src/pkg_info/property/mod.rs @@ -13,7 +13,7 @@ use std::{ str::FromStr, }; -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -135,26 +135,27 @@ pub fn parse_property_from_file>( Property::from_str(&content) } -pub fn parse_property_in_folder(folder_path: &Path) -> Result { +pub fn parse_property_in_folder( + folder_path: &Path, +) -> Result> { // Path to the property.json file. let property_path = folder_path.join(PROPERTY_JSON_FILENAME); + if !property_path.exists() { + return Ok(None); + } // Read and parse the property.json file. - let property = parse_property_from_file(&property_path)?; + let property = + parse_property_from_file(&property_path).with_context(|| { + format!("Failed to load {}.", property_path.display()) + })?; // Validate the property schema only if it is present. - let validation = - json_schema::ten_validate_property_json_file(&property_path); - - if let Err(validation_err) = validation { - return Err(anyhow::anyhow!( - "Failed to validate {}, caused by {}", - property_path.display(), - validation_err - )); - } + json_schema::ten_validate_property_json_file(&property_path).with_context( + || format!("Failed to validate {}.", property_path.display()), + )?; - Ok(property) + Ok(Some(property)) } #[cfg(test)]