Skip to content

Commit 54aaa6b

Browse files
refactor(Mountain/Vine): Centralize effect mapping and standardize gRPC error handling
- Introduced `erase_and_map_effect` helper in `EffectCreation` to uniformly handle capability requirements and JSON serialization for all ActionEffects, replacing manual mapping and simplifying effect creation - Renamed `RPCError` to `RpcError` in `Vine.proto` to match Rust naming conventions and reflect recent standardization efforts - Simplified error handling in `TreeViewProvider` by removing redundant error type specification - Refactored `VineError` variants to be more concise: - Replaced detailed error variants with generalized `ClientNotConnected` and `RpcError` - Added automatic conversion from `tonic::Status` to `RpcError` - Updated gRPC service methods to use snake_case per Rust conventions - Added `transport` feature to tonic-build in Cargo.toml for future IPC enhancements This refactoring aligns with Land's final architecture by: 1) Strengthening the `Track` dispatcher's ability to handle `Common`-defined ActionEffects with proper capability resolution 2) Ensuring consistent error handling across the `Vine` gRPC layer 3) Reducing boilerplate in effect-to-RPC mapping while maintaining type safety 4) Preparing for enhanced transport capabilities in the IPC layer between Mountain and Cocoon
1 parent 719b173 commit 54aaa6b

7 files changed

Lines changed: 68 additions & 53 deletions

File tree

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ serde = { workspace = true, features = ["derive"] }
99
serde_json = { workspace = true }
1010
tauri-build = { workspace = true, features = [] }
1111
toml = { workspace = true }
12-
tonic-build = { workspace = true }
12+
tonic-build = { workspace = true, features = ["transport"] }
1313

1414
[dependencies]
15-
tauri = { version = "*", features = [
15+
tauri = { workspace = true, features = [
1616
"wry",
1717
"tray-icon",
1818
"devtools",

Proto/Vine.proto

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ message GenericRequest {
3636
message GenericResponse {
3737
uint64 request_id = 1;
3838
bytes result = 2; // The successful result, JSON-serialized.
39-
optional RPCError error = 3;
39+
optional RpcError error = 3;
4040
}
4141

4242
// A generic notification message, which is fire-and-forget and does not have an ID.
@@ -46,7 +46,7 @@ message GenericNotification {
4646
}
4747

4848
// A structured error payload, compliant with JSON-RPC error objects.
49-
message RPCError {
49+
message RpcError {
5050
int32 code = 1;
5151
string message = 2;
5252
bytes data = 3; // Optional, additional error data, JSON-serialized.

Source/Environment/TreeViewProvider.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@ impl TreeViewProvider for MountainEnvironment {
1818
/// Registers a new tree data provider from Cocoon.
1919
async fn RegisterTreeDataProvider(&self, ViewIdentifier:String, Options:Value) -> Result<(), CommonError> {
2020
info!("[TreeViewProvider] Registering data provider for view: {}", ViewIdentifier);
21-
let OptionsDTO:Common::TreeView::DTO::TreeViewOptionsDTO = serde_json::from_value(Options).map_err(|e| {
22-
CommonError::CommonError::InvalidArgument { ArgumentName:"Options".into(), Reason:e.to_string() }
23-
})?;
21+
let OptionsDTO:Common::TreeView::DTO::TreeViewOptionsDTO::TreeViewOptionsDTO = serde_json::from_value(Options)
22+
.map_err(|e| CommonError::InvalidArgument { ArgumentName:"Options".into(), Reason:e.to_string() })?;
2423

2524
let NewState = TreeViewStateDTO {
2625
ViewIdentifier:ViewIdentifier.clone(),

Source/Track/EffectCreation.rs

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
use std::sync::Arc;
1010

1111
use Common::{
12-
self,
13-
Effect::{ActionEffect::ActionEffect, ApplicationRunTime::ApplicationRunTime},
12+
Effect::{ActionEffect::ActionEffect, ApplicationRunTime::ApplicationRunTime as ApplicationRunTimeTrait},
1413
Error::CommonError::CommonError,
1514
};
1615
use serde_json::{Value, from_value};
@@ -34,6 +33,22 @@ macro_rules! get_param {
3433
};
3534
}
3635

36+
/// A helper function to erase the specific capability type of an effect,
37+
/// mapping it to a generic effect that can be run by the dispatcher.
38+
fn erase_and_map_effect<C, O, E>(effect:ActionEffect<Arc<C>, E, O>) -> MappedEffect
39+
where
40+
C: ?Sized + Send + Sync + 'static,
41+
O: serde::Serialize + Send + 'static,
42+
E: Into<CommonError> + Send + Sync + 'static,
43+
MountainRunTime: Common::Environment::Requires::Requires<C>, {
44+
let mapped_effect = effect.map(|output| serde_json::to_value(output).unwrap_or(Value::Null));
45+
46+
ActionEffect::New(Arc::new(move |runtime:Arc<MountainRunTime>| {
47+
let effect_clone = mapped_effect.clone();
48+
Box::pin(async move { runtime.Run(effect_clone).await.map_err(|e| e.into()) })
49+
}))
50+
}
51+
3752
/// Creates an `ActionEffect` for a request from any source (frontend or
3853
/// sidecar).
3954
pub fn CreateEffectForRequest<R:Runtime>(
@@ -47,31 +62,36 @@ pub fn CreateEffectForRequest<R:Runtime>(
4762

4863
// This is the main RPC-to-Effect mapping. It deserializes the `params`
4964
// into the arguments required by the corresponding effect constructor.
50-
let Effect:MappedEffect = match Method {
65+
let effect:MappedEffect = match Method {
5166
// --- Command Effects ---
5267
"Command.Execute" => {
5368
let ID = get_param!(ParametersArray, 0, String)?;
5469
let Args = ParametersArray.get(1).cloned().unwrap_or(Value::Null);
55-
Common::Command::ExecuteCommand::ExecuteCommand(ID, Args).map(to_value)
70+
let specific_effect = Common::Command::ExecuteCommand::ExecuteCommand(ID, Args);
71+
erase_and_map_effect(specific_effect)
5672
},
5773
"Command.Register" => {
5874
let SidecarID = get_param!(ParametersArray, 0, String)?;
5975
let CommandID = get_param!(ParametersArray, 1, String)?;
60-
Common::Command::RegisterCommand::RegisterCommand(SidecarID, CommandID).map(to_value)
76+
let specific_effect = Common::Command::RegisterCommand::RegisterCommand(SidecarID, CommandID);
77+
erase_and_map_effect(specific_effect)
6178
},
6279

6380
// --- FileSystem Read Effects ---
6481
"FileSystem.ReadFile" => {
6582
let Path = get_param!(ParametersArray, 0, _)?;
66-
Common::FileSystem::ReadFile::ReadFile(Path).map(to_value)
83+
let specific_effect = Common::FileSystem::ReadFile::ReadFile(Path);
84+
erase_and_map_effect(specific_effect)
6785
},
6886
"FileSystem.StatFile" => {
6987
let Path = get_param!(ParametersArray, 0, _)?;
70-
Common::FileSystem::StatFile::StatFile(Path).map(to_value)
88+
let specific_effect = Common::FileSystem::StatFile::StatFile(Path);
89+
erase_and_map_effect(specific_effect)
7190
},
7291
"FileSystem.ReadDirectory" => {
7392
let Path = get_param!(ParametersArray, 0, _)?;
74-
Common::FileSystem::ReadDirectory::ReadDirectory(Path).map(to_value)
93+
let specific_effect = Common::FileSystem::ReadDirectory::ReadDirectory(Path);
94+
erase_and_map_effect(specific_effect)
7595
},
7696

7797
// --- FileSystem Write Effects ---
@@ -80,42 +100,34 @@ pub fn CreateEffectForRequest<R:Runtime>(
80100
let Content = get_param!(ParametersArray, 1, Vec<u8>)?;
81101
let Create = get_param!(ParametersArray, 2, bool)?;
82102
let Overwrite = get_param!(ParametersArray, 3, bool)?;
83-
Common::FileSystem::WriteFileBytes::WriteFileBytes(Path, Content, Create, Overwrite).map(to_value)
103+
let specific_effect = Common::FileSystem::WriteFileBytes::WriteFileBytes(Path, Content, Create, Overwrite);
104+
erase_and_map_effect(specific_effect)
84105
},
85106
"FileSystem.Delete" => {
86107
let Path = get_param!(ParametersArray, 0, _)?;
87108
let Recursive = get_param!(ParametersArray, 1, bool)?;
88109
let UseTrash = get_param!(ParametersArray, 2, bool)?;
89-
Common::FileSystem::Delete::Delete(Path, Recursive, UseTrash).map(to_value)
110+
let specific_effect = Common::FileSystem::Delete::Delete(Path, Recursive, UseTrash);
111+
erase_and_map_effect(specific_effect)
90112
},
91113

92114
// --- UserInterface Effects ---
93115
"UserInterface.ShowMessage" => {
94116
let Severity = get_param!(ParametersArray, 0, _)?;
95117
let Message = get_param!(ParametersArray, 1, String)?;
96118
let Options = ParametersArray.get(2).cloned().unwrap_or(Value::Null);
97-
Common::UserInterface::ShowMessage::ShowMessage(Severity, Message, Options).map(to_value)
119+
let specific_effect = Common::UserInterface::ShowMessage::ShowMessage(Severity, Message, Options);
120+
erase_and_map_effect(specific_effect)
98121
},
99122
"UserInterface.ShowOpenDialog" => {
100123
let Options = get_param!(ParametersArray, 0, _)?;
101-
Common::UserInterface::ShowOpenDialog::ShowOpenDialog(Options).map(to_value)
124+
let specific_effect = Common::UserInterface::ShowOpenDialog::ShowOpenDialog(Options);
125+
erase_and_map_effect(specific_effect)
102126
},
103127

104128
// ... Add mappings for all other effects here ...
105129
_ => return Err(format!("No ActionEffect mapping found for method: {}", Method)),
106-
}?;
107-
108-
Ok(Effect)
109-
}
130+
};
110131

111-
/// A helper function to map the output of any effect to a `serde_json::Value`.
112-
fn to_value<TOutput:serde::Serialize, TError, TRunTime>(
113-
effect:ActionEffect<Arc<TRunTime>, TError, TOutput>,
114-
) -> Result<MappedEffect, String>
115-
where
116-
TRunTime: ApplicationRunTime<EnvironmentType = crate::Environment::MountainEnvironment::MountainEnvironment>
117-
+ Send
118-
+ Sync
119-
+ 'static, {
120-
Ok(effect.map(|output| serde_json::to_value(output).unwrap_or(Value::Null)))
132+
Ok(effect)
121133
}

Source/Vine/Client.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ pub async fn SendRequest(
7171
match timeout(Duration::from_millis(TimeoutMilliseconds), Future).await {
7272
Ok(Ok(Response)) => {
7373
let ResponseData = Response.into_inner();
74-
if let Some(RPCError) = ResponseData.error {
74+
if let Some(RpcError) = ResponseData.error {
7575
error!(
7676
"[VineClient] Received RPC error from sidecar '{}': {}",
77-
SidecarIdentifier, RPCError.message
77+
SidecarIdentifier, RpcError.message
7878
);
79-
Err(VineError::RPCError(RPCError.message))
79+
Err(VineError::RpcError(RpcError.message))
8080
} else {
8181
let DeserializedValue = from_slice(&ResponseData.result)?;
8282
Ok(DeserializedValue)

Source/Vine/Error.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,12 @@ use thiserror::Error;
1212
pub enum VineError {
1313
/// A gRPC client channel for the specified sidecar could not be found or
1414
/// is not ready.
15-
#[error("Sidecar '{SidecarIdentifier}' not found or its gRPC client channel is not ready: {Details}")]
16-
ClientChannelError { SidecarIdentifier:String, Details:String },
15+
#[error("Sidecar '{0}' not found or its gRPC client channel is not ready.")]
16+
ClientNotConnected(String),
1717

1818
/// An RPC call to a sidecar failed with a specific gRPC status.
19-
#[error(
20-
"gRPC call to sidecar '{SidecarIdentifier}' (method: '{MethodName}') failed: {StatusCode} - {StatusMessage}"
21-
)]
22-
gRPCRequestFailed {
23-
SidecarIdentifier:String,
24-
MethodName:String,
25-
StatusCode:String,
26-
StatusMessage:String,
27-
},
19+
#[error("gRPC call failed: {0}")]
20+
RpcError(String),
2821

2922
/// An error occurred while serializing or deserializing a JSON payload.
3023
#[error("JSON serialization error for gRPC payload: {0}")]
@@ -43,10 +36,18 @@ pub enum VineError {
4336
/// A shared state mutex was "poisoned," indicating a panic.
4437
#[error("Internal state lock poisoned: {0}")]
4538
InternalLockError(String),
39+
40+
/// An error occurred from an invalid URI.
41+
#[error("Invalid URI: {0}")]
42+
InvalidUri(#[from] tonic::transport::uri::InvalidUri),
4643
}
4744

4845
impl<T> From<PoisonError<MutexGuard<'_, T>>> for VineError {
4946
fn from(Error:PoisonError<MutexGuard<'_, T>>) -> Self {
5047
VineError::InternalLockError(format!("Shared state lock poisoned: {}", Error))
5148
}
5249
}
50+
51+
impl From<tonic::Status> for VineError {
52+
fn from(status:tonic::Status) -> Self { VineError::RpcError(status.to_string()) }
53+
}

Source/Vine/Server/MountainVinegRPCService.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::{
2020
GenericNotification,
2121
GenericRequest,
2222
GenericResponse,
23-
RPCError,
23+
RpcError,
2424
mountain_service_server::MountainService,
2525
},
2626
};
@@ -42,7 +42,10 @@ impl MountainVinegRPCService {
4242
#[tonic::async_trait]
4343
impl MountainService for MountainVinegRPCService {
4444
/// Handles generic request-response RPCs from Cocoon.
45-
async fn ProcessCocoonRequest(&self, request:Request<GenericRequest>) -> Result<Response<GenericResponse>, Status> {
45+
async fn process_cocoon_request(
46+
&self,
47+
request:Request<GenericRequest>,
48+
) -> Result<Response<GenericResponse>, Status> {
4649
let RequestData = request.into_inner();
4750
let MethodName = RequestData.method;
4851
let RequestIdentifier = RequestData.request_id;
@@ -60,7 +63,7 @@ impl MountainService for MountainVinegRPCService {
6063
return Ok(Response::new(GenericResponse {
6164
request_id:RequestIdentifier,
6265
result:vec![],
63-
error:Some(RPCError { message:msg, code:-32700, data:vec![] }),
66+
error:Some(RpcError { message:msg, code:-32700, data:vec![] }),
6467
}));
6568
},
6669
};
@@ -91,7 +94,7 @@ impl MountainService for MountainVinegRPCService {
9194
Ok(Response::new(GenericResponse {
9295
request_id:RequestIdentifier,
9396
result:vec![],
94-
error:Some(RPCError {
97+
error:Some(RpcError {
9598
message:ErrorString,
9699
code:-32000, // JSON-RPC Generic Server Error
97100
data:vec![],
@@ -102,7 +105,7 @@ impl MountainService for MountainVinegRPCService {
102105
}
103106

104107
/// Handles generic fire-and-forget notifications from Cocoon.
105-
async fn SendCocoonNotification(&self, request:Request<GenericNotification>) -> Result<Response<Empty>, Status> {
108+
async fn send_cocoon_notification(&self, request:Request<GenericNotification>) -> Result<Response<Empty>, Status> {
106109
let NotificationData = request.into_inner();
107110
let MethodName = NotificationData.method;
108111
info!("[VineServer] Received gRPC Notification: Method='{}'", MethodName);
@@ -118,7 +121,7 @@ impl MountainService for MountainVinegRPCService {
118121
}
119122

120123
/// Handles a request from Cocoon to cancel a long-running operation.
121-
async fn CancelOperation(&self, _request:Request<CancelOperationRequest>) -> Result<Response<Empty>, Status> {
124+
async fn cancel_operation(&self, _request:Request<CancelOperationRequest>) -> Result<Response<Empty>, Status> {
122125
info!("[VineServer] Received CancelOperation request, but cancellation is not yet implemented.");
123126
// A full implementation would map the request_id_to_cancel to a
124127
// CancellationToken and trigger it.

0 commit comments

Comments
 (0)