-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: reorganize crate by responsibility into amqp/service/client modules #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| //! Stable path for symbols that the `#[girolle]` attribute macro | ||
| //! emits into user code. The expanded code references | ||
| //! `::girolle::__macro_support::*`; nothing else should depend on | ||
| //! this module. | ||
|
|
||
| pub use crate::service::args::build_inputs_fn_service; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| use lapin::{types::FieldTable, Connection, ConnectionProperties}; | ||
| use tracing::{error, info}; | ||
|
|
||
| /// Open an AMQP connection wired up to the Tokio executor and reactor, | ||
| /// with a custom heartbeat advertised in the client properties. | ||
| pub(crate) async fn get_connection( | ||
| amqp_uri: String, | ||
| heartbeat_value: u16, | ||
| ) -> Result<Connection, lapin::Error> { | ||
| let mut connection_options = ConnectionProperties::default() | ||
| .with_executor(tokio_executor_trait::Tokio::current()) | ||
| .with_reactor(tokio_reactor_trait::Tokio); | ||
| let mut client_properties_custom = FieldTable::default(); | ||
| client_properties_custom.insert("heartbeat".into(), heartbeat_value.into()); | ||
| connection_options.client_properties = client_properties_custom; | ||
| match Connection::connect(&amqp_uri, connection_options).await { | ||
| Ok(connection) => { | ||
| info!("Connected to RabbitMQ"); | ||
| Ok(connection) | ||
| } | ||
| Err(e) => { | ||
| error!("Failed to connect to RabbitMQ with error:{}", e); | ||
| Err(e) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| //! Manipulation of Nameko-specific AMQP headers. | ||
| //! | ||
| //! In particular `nameko.call_id_stack`: every hop appends | ||
| //! `<routing_key>.<id>`, and the stack is truncated to a configurable | ||
| //! window so it doesn't grow unbounded across long call chains. | ||
|
|
||
| use crate::error::GirolleError; | ||
| use crate::protocol::NAMEKO_CALL_ID_STACK; | ||
| use lapin::message::Delivery; | ||
| use lapin::types::{AMQPValue, FieldTable, LongString, ShortString}; | ||
| use lapin::BasicProperties; | ||
| use tracing::error; | ||
| use uuid::Uuid; | ||
|
|
||
| fn set_current_call_id(function_name: &str, id: &str) -> AMQPValue { | ||
| let rpc_id = format!("{}.{}", function_name, id); | ||
| AMQPValue::LongString(LongString::from(rpc_id.as_bytes())) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_set_current_call_id() { | ||
| let function_name = "package_cg_asset.get_filepaths_from_tags"; | ||
| let id = "4c5615e2-9367-46aa-8f90-b87e89723fa0"; | ||
| let rpc_id = set_current_call_id(function_name, id); | ||
| assert_eq!( | ||
| rpc_id, | ||
| AMQPValue::LongString(LongString::from( | ||
| "package_cg_asset.get_filepaths_from_tags.4c5615e2-9367-46aa-8f90-b87e89723fa0" | ||
| .as_bytes() | ||
| )) | ||
| ); | ||
| } | ||
|
|
||
| /// Append a fresh call id to `nameko.call_id_stack` and truncate the | ||
| /// stack to `parent_calls_tracked` entries (Nameko semantics). | ||
| pub(crate) fn insert_new_id_to_call_id( | ||
| mut headers: FieldTable, | ||
| function_name: &str, | ||
| id: &str, | ||
| parent_calls_tracked: usize, | ||
| ) -> FieldTable { | ||
| let inner_headers = headers.inner(); | ||
| let call_id_stack_slice = inner_headers | ||
| .get(NAMEKO_CALL_ID_STACK) | ||
| .unwrap() | ||
| .as_array() | ||
| .unwrap() | ||
| .as_slice(); | ||
| let mut call_id_stack = call_id_stack_slice.to_vec(); | ||
| call_id_stack.push(set_current_call_id(function_name, id)); | ||
|
|
||
| if call_id_stack.len() > parent_calls_tracked { | ||
| call_id_stack = call_id_stack[call_id_stack.len() - parent_calls_tracked..].to_vec(); | ||
| } | ||
|
|
||
| let to_amqp = AMQPValue::FieldArray(call_id_stack.into()); | ||
| let key_field = ShortString::from(NAMEKO_CALL_ID_STACK); | ||
| headers.insert(key_field, to_amqp); | ||
| headers | ||
| } | ||
|
|
||
| /// Read a required `ShortString` header into an owned `String`, | ||
| /// panicking if the header is absent. | ||
| pub(crate) fn get_id(opt_id: &Option<ShortString>, id_name: &str) -> String { | ||
| match opt_id { | ||
| Some(id) => id.to_string(), | ||
| None => { | ||
| error!("{}: None", id_name); | ||
| panic!("{}: None", id_name) | ||
| } | ||
|
Comment on lines
+66
to
+70
|
||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_get_id() { | ||
| let id = get_id(&Some(ShortString::from("id")), "id"); | ||
| assert_eq!(id, "id".to_string()); | ||
| } | ||
|
|
||
| /// Build the `BasicProperties` of a reply message from an inbound | ||
| /// delivery: copies the correlation id, sets reply-to to the caller's | ||
| /// reply queue, and updates `nameko.call_id_stack` for the next hop. | ||
| pub(crate) fn delivery_to_message_properties( | ||
| delivery: &Delivery, | ||
| id: &Uuid, | ||
| rpc_queue: &str, | ||
| parent_calls_tracked: usize, | ||
| ) -> Result<BasicProperties, GirolleError> { | ||
| let opt_routing_key = delivery.routing_key.to_string(); | ||
| let correlation_id = get_id(delivery.properties.correlation_id(), "correlation_id"); | ||
| let opt_headers = delivery.properties.headers(); | ||
| let headers = match opt_headers { | ||
| Some(h) => insert_new_id_to_call_id( | ||
| h.clone(), | ||
| &opt_routing_key, | ||
| &id.to_string(), | ||
| parent_calls_tracked, | ||
| ), | ||
| None => { | ||
| error!("No headers found in delivery properties"); | ||
| return Err(GirolleError::MissingHeader); | ||
| } | ||
| }; | ||
| Ok(BasicProperties::default() | ||
| .with_correlation_id(correlation_id.into()) | ||
| .with_content_type("application/json".into()) | ||
| .with_reply_to(rpc_queue.into()) | ||
| .with_content_encoding("utf-8".into()) | ||
| .with_headers(headers) | ||
| .with_delivery_mode(2) | ||
| .with_priority(0)) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| //! AMQP transport layer. | ||
| //! | ||
| //! Owns every direct interaction with `lapin`: opening connections, | ||
| //! declaring channels/queues/exchanges, manipulating Nameko-specific | ||
| //! AMQP headers, and publishing reply messages. Higher layers | ||
| //! (`service`, `client`) talk to the broker exclusively through this | ||
| //! module. | ||
|
|
||
| pub(crate) mod channel; | ||
| pub(crate) mod connection; | ||
| pub(crate) mod headers; | ||
| pub(crate) mod publish; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::protocol::PayloadResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use lapin::options::BasicPublishOptions; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use lapin::{BasicProperties, Channel}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Publish an RPC reply on `rpc_exchange` with `reply_to_id` as the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// routing key. Spawned as a fire-and-forget task so the consumer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// loop is not blocked on the broker's confirm. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub(crate) async fn publish( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rpc_channel: &Channel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| payload: PayloadResult, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| properties: BasicProperties, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reply_to_id: String, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rpc_exchange: &str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> lapin::Result<()> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let rpc_channel_clone = rpc_channel.clone(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let rpc_exchange_clone = rpc_exchange.to_string(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tokio::spawn(async move { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rpc_channel_clone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .basic_publish( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| &rpc_exchange_clone, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| &reply_to_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BasicPublishOptions::default(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| payload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .to_string() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .expect("can't serialize payload") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .as_bytes(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| properties, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .unwrap() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+32
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let rpc_channel_clone = rpc_channel.clone(); | |
| let rpc_exchange_clone = rpc_exchange.to_string(); | |
| tokio::spawn(async move { | |
| rpc_channel_clone | |
| .basic_publish( | |
| &rpc_exchange_clone, | |
| &reply_to_id, | |
| BasicPublishOptions::default(), | |
| payload | |
| .to_string() | |
| .expect("can't serialize payload") | |
| .as_bytes(), | |
| properties, | |
| ) | |
| .await | |
| .unwrap() | |
| .await | |
| .unwrap(); | |
| let payload = payload.to_string().map_err(|err| { | |
| lapin::Error::from(std::io::Error::new( | |
| std::io::ErrorKind::InvalidData, | |
| format!("can't serialize payload: {err}"), | |
| )) | |
| })?; | |
| let rpc_channel_clone = rpc_channel.clone(); | |
| let rpc_exchange_clone = rpc_exchange.to_string(); | |
| tokio::spawn(async move { | |
| if let Ok(confirm) = rpc_channel_clone | |
| .basic_publish( | |
| &rpc_exchange_clone, | |
| &reply_to_id, | |
| BasicPublishOptions::default(), | |
| payload.as_bytes(), | |
| properties, | |
| ) | |
| .await | |
| { | |
| let _ = confirm.await; | |
| } |
Copilot
AI
Apr 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spawned publish task uses unwrap() on both the initial publish and the broker confirm future. If the broker closes the channel or the confirm is negative, this will panic in a background task and the caller will still get Ok(()). Consider handling these errors explicitly (log + return error or signal failure) instead of panicking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
insert_new_id_to_call_idusesunwrap()on thenameko.call_id_stackheader and on its type conversions. If a caller sends headers without that key (or with an unexpected type), the service will panic. Consider treating a missing/invalid stack as an empty stack and seeding it instead of panicking.