From 9e69a3201f416dcc0f55661c0ea593df8da50882 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Mon, 6 Jan 2025 12:16:47 +0100 Subject: [PATCH] Pin async function evaluation in expressions and reduce trc::Result size --- CHANGELOG.md | 3 +- UPGRADING.md | 2 +- crates/common/src/auth/oauth/introspect.rs | 2 +- crates/common/src/expr/eval.rs | 116 +++++++++++++------- crates/common/src/telemetry/tracers/otel.rs | 4 +- crates/main/Cargo.toml | 4 +- crates/smtp/src/inbound/data.rs | 10 +- crates/smtp/src/inbound/ehlo.rs | 2 +- crates/smtp/src/inbound/mail.rs | 4 +- crates/smtp/src/outbound/delivery.rs | 8 +- crates/smtp/src/reporting/mod.rs | 2 +- crates/smtp/src/scripts/event_loop.rs | 2 +- crates/trc/event-macro/src/lib.rs | 2 +- crates/trc/src/event/conv.rs | 56 +++++----- crates/trc/src/event/mod.rs | 76 ++++++++++--- crates/trc/src/ipc/channel.rs | 8 +- crates/trc/src/lib.rs | 7 +- crates/trc/src/macros.rs | 2 +- crates/trc/src/serializers/binary.rs | 10 +- crates/trc/src/serializers/json.rs | 12 +- crates/trc/src/serializers/text.rs | 20 ++-- tests/Cargo.toml | 4 +- tests/src/jmap/mod.rs | 4 +- tests/src/smtp/config.rs | 23 ++-- 24 files changed, 241 insertions(+), 142 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bda9584f..c141738b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ All notable changes to this project will be documented in this file. This projec ## [0.11.0] - 2025-01-06 -This version includes breaking changes to the configuration file. Please read [UPGRADING.md](UPGRADING.md) for details. +This version includes breaking changes to the configuration file, please read [UPGRADING.md](UPGRADING.md) for details. +To upgrade replace the `stalwart-mail` binary and then upgrade to the latest web-admin. ### Added - Spam filter rewritten in Rust for a significant performance improvement. diff --git a/UPGRADING.md b/UPGRADING.md index afc4560b..cb6f574c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,7 +1,7 @@ Upgrading from `v0.10.x` to `v0.11.0` ------------------------------------ -Version `0.11.0` introduces breaking changes to the spam filter configuration. No data migration is required but, if changes were made to the previous spam filter, the configuration of the new spam filter should be reviewed. In particular: +Version `0.11.0` introduces breaking changes to the spam filter configuration. Although no data migration is required, if changes were made to the previous spam filter, the configuration of the new spam filter should be reviewed. In particular: - `lookup.spam-*` settings are no longer used, these have been replaced by `spam-filter.*` settings. Review the [updated documentation](http://stalw.art/docs/spamfilter/overview). - Previous `spam-filter` and `track-replies` Sieve scripts cannot be used with the new version. They have been replaced by a built-in spam filter written in Rust. diff --git a/crates/common/src/auth/oauth/introspect.rs b/crates/common/src/auth/oauth/introspect.rs index f4e931b5..34dd878e 100644 --- a/crates/common/src/auth/oauth/introspect.rs +++ b/crates/common/src/auth/oauth/introspect.rs @@ -85,7 +85,7 @@ impl Server { }), Err(err) if matches!( - err.inner, + err.event_type(), EventType::Auth(AuthEvent::Error) | EventType::Auth(AuthEvent::TokenExpired) ) => { diff --git a/crates/common/src/expr/eval.rs b/crates/common/src/expr/eval.rs index 43795cb1..8b9b5063 100644 --- a/crates/common/src/expr/eval.rs +++ b/crates/common/src/expr/eval.rs @@ -35,7 +35,16 @@ impl Server { return None; } - match if_block.eval(resolver, self, session_id).await { + match (EvalContext { + resolver, + core: self, + expr: if_block, + captures: Vec::new(), + session_id, + }) + .eval() + .await + { Ok(result) => { trc::event!( Eval(EvalEvent::Result), @@ -82,7 +91,16 @@ impl Server { return None; } - match expr.eval(resolver, self, &mut Vec::new(), session_id).await { + match (EvalContext { + resolver, + core: self, + expr, + captures: &mut Vec::new(), + session_id, + }) + .eval() + .await + { Ok(result) => { trc::event!( Eval(EvalEvent::Result), @@ -119,60 +137,71 @@ impl Server { } } -impl IfBlock { - pub async fn eval<'x, V: ResolveVariable>( - &'x self, - resolver: &'x V, - core: &Server, - session_id: u64, - ) -> trc::Result> { - let mut captures = Vec::new(); +struct EvalContext<'x, 'y, V: ResolveVariable, T, C> { + resolver: &'x V, + core: &'y Server, + expr: &'x T, + captures: C, + session_id: u64, +} - for if_then in &self.if_then { - if if_then - .expr - .eval(resolver, core, &mut captures, session_id) - .await? - .to_bool() +impl<'x, 'y, V: ResolveVariable> EvalContext<'x, 'y, V, IfBlock, Vec> { + async fn eval(&mut self) -> trc::Result> { + for if_then in &self.expr.if_then { + if (EvalContext { + resolver: self.resolver, + core: self.core, + expr: &if_then.expr, + captures: &mut self.captures, + session_id: self.session_id, + }) + .eval() + .await? + .to_bool() { - return if_then - .then - .eval(resolver, core, &mut captures, session_id) - .await; + return (EvalContext { + resolver: self.resolver, + core: self.core, + expr: &if_then.then, + captures: &mut self.captures, + session_id: self.session_id, + }) + .eval() + .await; } } - self.default - .eval(resolver, core, &mut captures, session_id) - .await + (EvalContext { + resolver: self.resolver, + core: self.core, + expr: &self.expr.default, + captures: &mut self.captures, + session_id: self.session_id, + }) + .eval() + .await } } -impl Expression { - async fn eval<'x, 'y, V: ResolveVariable>( - &'x self, - resolver: &'x V, - core: &Server, - captures: &'y mut Vec, - session_id: u64, - ) -> trc::Result> { +impl<'x, 'y, V: ResolveVariable> EvalContext<'x, 'y, V, Expression, &mut Vec> { + async fn eval(&mut self) -> trc::Result> { let mut stack = Vec::new(); - let mut exprs = self.items.iter(); + let mut exprs = self.expr.items.iter(); while let Some(expr) = exprs.next() { match expr { ExpressionItem::Variable(v) => { - stack.push(resolver.resolve_variable(*v)); + stack.push(self.resolver.resolve_variable(*v)); } ExpressionItem::Global(v) => { - stack.push(resolver.resolve_global(v)); + stack.push(self.resolver.resolve_global(v)); } ExpressionItem::Constant(val) => { stack.push(Variable::from(val)); } ExpressionItem::Capture(v) => { stack.push(Variable::String(Cow::Owned( - captures + self.captures .get(*v as usize) .map(|v| v.as_str()) .unwrap_or_default() @@ -216,8 +245,12 @@ impl Expression { let result = if let Some((_, fnc, _)) = FUNCTIONS.get(*id as usize) { (fnc)(arguments) } else { - core.eval_fnc(*id - FUNCTIONS.len() as u32, arguments, session_id) - .await? + Box::pin(self.core.eval_fnc( + *id - FUNCTIONS.len() as u32, + arguments, + self.session_id, + )) + .await? }; stack.push(result); @@ -247,23 +280,26 @@ impl Expression { stack.push(Variable::Array(items)); } ExpressionItem::Regex(regex) => { - captures.clear(); + self.captures.clear(); let value = stack.pop().unwrap_or_default().into_string(); if let Some(captures_) = regex.captures(value.as_ref()) { for capture in captures_.iter() { - captures.push(capture.map_or("", |m| m.as_str()).to_string()); + self.captures + .push(capture.map_or("", |m| m.as_str()).to_string()); } } - stack.push(Variable::Integer(!captures.is_empty() as i64)); + stack.push(Variable::Integer(!self.captures.is_empty() as i64)); } } } Ok(stack.pop().unwrap_or_default()) } +} +impl Expression { pub fn is_empty(&self) -> bool { self.items.is_empty() } diff --git a/crates/common/src/telemetry/tracers/otel.rs b/crates/common/src/telemetry/tracers/otel.rs index 7591350e..8dad9d3f 100644 --- a/crates/common/src/telemetry/tracers/otel.rs +++ b/crates/common/src/telemetry/tracers/otel.rs @@ -257,11 +257,11 @@ fn build_any_value(value: &trc::Value) -> AnyValue { trc::Value::Event(v) => AnyValue::Map(Box::new( [( Key::from_static_str("eventName"), - AnyValue::String(v.inner.name().into()), + AnyValue::String(v.event_type().name().into()), )] .into_iter() .chain( - v.keys + v.keys() .iter() .map(|(k, v)| (build_key(k), build_any_value(v))), ) diff --git a/crates/main/Cargo.toml b/crates/main/Cargo.toml index 9b3965da..264d902d 100644 --- a/crates/main/Cargo.toml +++ b/crates/main/Cargo.toml @@ -34,8 +34,8 @@ tokio = { version = "1.23", features = ["full"] } jemallocator = "0.5.0" [features] -default = ["sqlite", "postgres", "mysql", "rocks", "elastic", "s3", "redis", "azure", "enterprise"] -#default = ["rocks", "enterprise"] +#default = ["sqlite", "postgres", "mysql", "rocks", "elastic", "s3", "redis", "azure", "enterprise"] +default = ["rocks", "enterprise"] sqlite = ["store/sqlite"] foundationdb = ["store/foundation", "common/foundation"] postgres = ["store/postgres"] diff --git a/crates/smtp/src/inbound/data.rs b/crates/smtp/src/inbound/data.rs index 080b440b..dc2ffd08 100644 --- a/crates/smtp/src/inbound/data.rs +++ b/crates/smtp/src/inbound/data.rs @@ -140,7 +140,7 @@ impl Session { }), SpanId = self.data.session_id, Strict = strict, - Result = dkim_output.iter().map(trc::Event::from).collect::>(), + Result = dkim_output.iter().map(trc::Error::from).collect::>(), Elapsed = time.elapsed(), ); @@ -194,7 +194,7 @@ impl Session { }), SpanId = self.data.session_id, Strict = strict, - Result = trc::Event::from(arc_output.result()), + Result = trc::Error::from(arc_output.result()), Elapsed = time.elapsed(), ); @@ -295,7 +295,7 @@ impl Session { Strict = strict, Domain = dmarc_output.domain().to_string(), Policy = dmarc_policy.to_string(), - Result = trc::Event::from(&dmarc_result), + Result = trc::Error::from(&dmarc_result), Elapsed = time.elapsed(), ); @@ -417,7 +417,7 @@ impl Session { set.write_header(&mut headers); } Err(err) => { - trc::error!(trc::Event::from(err) + trc::error!(trc::Error::from(err) .span_id(self.data.session_id) .details("Failed to ARC seal message")); } @@ -649,7 +649,7 @@ impl Session { signature.write_header(&mut headers); } Err(err) => { - trc::error!(trc::Event::from(err) + trc::error!(trc::Error::from(err) .span_id(self.data.session_id) .details("Failed to DKIM sign message")); } diff --git a/crates/smtp/src/inbound/ehlo.rs b/crates/smtp/src/inbound/ehlo.rs index 1f36b1c0..91a674c6 100644 --- a/crates/smtp/src/inbound/ehlo.rs +++ b/crates/smtp/src/inbound/ehlo.rs @@ -67,7 +67,7 @@ impl Session { }), SpanId = self.data.session_id, Domain = self.data.helo_domain.clone(), - Result = trc::Event::from(&spf_output), + Result = trc::Error::from(&spf_output), Elapsed = time.elapsed(), ); diff --git a/crates/smtp/src/inbound/mail.rs b/crates/smtp/src/inbound/mail.rs index 396a9ef9..61f85e64 100644 --- a/crates/smtp/src/inbound/mail.rs +++ b/crates/smtp/src/inbound/mail.rs @@ -75,7 +75,7 @@ impl Session { }), SpanId = self.data.session_id, Domain = self.data.helo_domain.clone(), - Result = trc::Event::from(&iprev), + Result = trc::Error::from(&iprev), Elapsed = time.elapsed(), ); @@ -482,7 +482,7 @@ impl Session { "<>" } .to_string(), - Result = trc::Event::from(&spf_output), + Result = trc::Error::from(&spf_output), Elapsed = time.elapsed(), ); diff --git a/crates/smtp/src/outbound/delivery.rs b/crates/smtp/src/outbound/delivery.rs index 15c48175..fb0c871c 100644 --- a/crates/smtp/src/outbound/delivery.rs +++ b/crates/smtp/src/outbound/delivery.rs @@ -374,7 +374,7 @@ impl DeliveryAttempt { TlsRpt(TlsRptEvent::RecordFetchError), SpanId = message.span_id, Domain = domain.domain.clone(), - CausedBy = trc::Event::from(err), + CausedBy = trc::Error::from(err), Elapsed = time.elapsed(), ); None @@ -466,7 +466,7 @@ impl DeliveryAttempt { MtaSts(MtaStsEvent::PolicyFetchError), SpanId = message.span_id, Domain = domain.domain.clone(), - CausedBy = trc::Event::from(err.clone()), + CausedBy = trc::Error::from(err.clone()), Strict = strict, Elapsed = time.elapsed(), ); @@ -532,7 +532,7 @@ impl DeliveryAttempt { Delivery(DeliveryEvent::MxLookupFailed), SpanId = message.span_id, Domain = domain.domain.clone(), - CausedBy = trc::Event::from(err.clone()), + CausedBy = trc::Error::from(err.clone()), Elapsed = time.elapsed(), ); @@ -820,7 +820,7 @@ impl DeliveryAttempt { SpanId = message.span_id, Domain = domain.domain.clone(), Hostname = envelope.mx.to_string(), - CausedBy = trc::Event::from(err.clone()), + CausedBy = trc::Error::from(err.clone()), Strict = strict, Elapsed = time.elapsed(), ); diff --git a/crates/smtp/src/reporting/mod.rs b/crates/smtp/src/reporting/mod.rs index f4e68b55..674e13af 100644 --- a/crates/smtp/src/reporting/mod.rs +++ b/crates/smtp/src/reporting/mod.rs @@ -230,7 +230,7 @@ impl SmtpReporting for Server { signature.write_header(&mut headers); } Err(err) => { - trc::error!(trc::Event::from(err) + trc::error!(trc::Error::from(err) .span_id(message.span_id) .details("Failed to sign message") .caused_by(trc::location!())); diff --git a/crates/smtp/src/scripts/event_loop.rs b/crates/smtp/src/scripts/event_loop.rs index e7c0a61b..c8e900e6 100644 --- a/crates/smtp/src/scripts/event_loop.rs +++ b/crates/smtp/src/scripts/event_loop.rs @@ -288,7 +288,7 @@ impl RunScript for Server { signature.write_header(&mut headers); } Err(err) => { - trc::error!(trc::Event::from(err) + trc::error!(trc::Error::from(err) .span_id(session_id) .caused_by(trc::location!()) .details("DKIM sign failed")); diff --git a/crates/trc/event-macro/src/lib.rs b/crates/trc/event-macro/src/lib.rs index 1a07bb47..5157f1fc 100644 --- a/crates/trc/event-macro/src/lib.rs +++ b/crates/trc/event-macro/src/lib.rs @@ -303,7 +303,7 @@ pub fn event(input: TokenStream) -> TokenStream { (trc::Key::#key, trc::Value::from(#value)) } }); - // This avoid having to evaluate expensive values when we know we are not interested in the event + // This avoids having to evaluate expensive values when we know we are not interested in the event let key_value_metric_tokens = key_values.iter().filter_map(|(key, value)| { if key.is_metric_key() { Some(quote! { diff --git a/crates/trc/src/event/conv.rs b/crates/trc/src/event/conv.rs index f831ecb2..b37fcb69 100644 --- a/crates/trc/src/event/conv.rs +++ b/crates/trc/src/event/conv.rs @@ -12,7 +12,7 @@ use crate::*; impl AsRef for Error { fn as_ref(&self) -> &EventType { - &self.inner + &self.0.inner } } @@ -100,8 +100,8 @@ impl From for Value { } } -impl From> for Value { - fn from(value: Event) -> Self { +impl From for Value { + fn from(value: Error) -> Self { Self::Event(value) } } @@ -152,7 +152,7 @@ where fn from(value: &crate::Result) -> Self { match value { Ok(value) => format!("{:?}", value).into(), - Err(err) => err.clone().into(), + Err(err) => Value::Event(err.clone()), } } } @@ -205,7 +205,7 @@ impl EventType { } } -impl From for Event { +impl From for Error { fn from(err: mail_auth::Error) -> Self { match err { mail_auth::Error::ParseError => { @@ -282,68 +282,68 @@ impl From for Event { } } -impl From<&mail_auth::DkimResult> for Event { +impl From<&mail_auth::DkimResult> for Error { fn from(value: &mail_auth::DkimResult) -> Self { match value.clone() { - mail_auth::DkimResult::Pass => Event::new(EventType::Dkim(DkimEvent::Pass)), + mail_auth::DkimResult::Pass => Error::new(EventType::Dkim(DkimEvent::Pass)), mail_auth::DkimResult::Neutral(err) => { - Event::new(EventType::Dkim(DkimEvent::Neutral)).caused_by(Event::from(err)) + Error::new(EventType::Dkim(DkimEvent::Neutral)).caused_by(Error::from(err)) } mail_auth::DkimResult::Fail(err) => { - Event::new(EventType::Dkim(DkimEvent::Fail)).caused_by(Event::from(err)) + Error::new(EventType::Dkim(DkimEvent::Fail)).caused_by(Error::from(err)) } mail_auth::DkimResult::PermError(err) => { - Event::new(EventType::Dkim(DkimEvent::PermError)).caused_by(Event::from(err)) + Error::new(EventType::Dkim(DkimEvent::PermError)).caused_by(Error::from(err)) } mail_auth::DkimResult::TempError(err) => { - Event::new(EventType::Dkim(DkimEvent::TempError)).caused_by(Event::from(err)) + Error::new(EventType::Dkim(DkimEvent::TempError)).caused_by(Error::from(err)) } - mail_auth::DkimResult::None => Event::new(EventType::Dkim(DkimEvent::None)), + mail_auth::DkimResult::None => Error::new(EventType::Dkim(DkimEvent::None)), } } } -impl From<&mail_auth::DmarcResult> for Event { +impl From<&mail_auth::DmarcResult> for Error { fn from(value: &mail_auth::DmarcResult) -> Self { match value.clone() { - mail_auth::DmarcResult::Pass => Event::new(EventType::Dmarc(DmarcEvent::Pass)), + mail_auth::DmarcResult::Pass => Error::new(EventType::Dmarc(DmarcEvent::Pass)), mail_auth::DmarcResult::Fail(err) => { - Event::new(EventType::Dmarc(DmarcEvent::Fail)).caused_by(Event::from(err)) + Error::new(EventType::Dmarc(DmarcEvent::Fail)).caused_by(Error::from(err)) } mail_auth::DmarcResult::PermError(err) => { - Event::new(EventType::Dmarc(DmarcEvent::PermError)).caused_by(Event::from(err)) + Error::new(EventType::Dmarc(DmarcEvent::PermError)).caused_by(Error::from(err)) } mail_auth::DmarcResult::TempError(err) => { - Event::new(EventType::Dmarc(DmarcEvent::TempError)).caused_by(Event::from(err)) + Error::new(EventType::Dmarc(DmarcEvent::TempError)).caused_by(Error::from(err)) } - mail_auth::DmarcResult::None => Event::new(EventType::Dmarc(DmarcEvent::None)), + mail_auth::DmarcResult::None => Error::new(EventType::Dmarc(DmarcEvent::None)), } } } -impl From<&mail_auth::DkimOutput<'_>> for Event { +impl From<&mail_auth::DkimOutput<'_>> for Error { fn from(value: &mail_auth::DkimOutput<'_>) -> Self { - Event::from(value.result()).ctx_opt( + Error::from(value.result()).ctx_opt( Key::Domain, value.signature().map(|s| s.domain().to_string()), ) } } -impl From<&mail_auth::IprevOutput> for Event { +impl From<&mail_auth::IprevOutput> for Error { fn from(value: &mail_auth::IprevOutput) -> Self { match value.result().clone() { - mail_auth::IprevResult::Pass => Event::new(EventType::Iprev(IprevEvent::Pass)), + mail_auth::IprevResult::Pass => Error::new(EventType::Iprev(IprevEvent::Pass)), mail_auth::IprevResult::Fail(err) => { - Event::new(EventType::Iprev(IprevEvent::Fail)).caused_by(Event::from(err)) + Error::new(EventType::Iprev(IprevEvent::Fail)).caused_by(Error::from(err)) } mail_auth::IprevResult::PermError(err) => { - Event::new(EventType::Iprev(IprevEvent::PermError)).caused_by(Event::from(err)) + Error::new(EventType::Iprev(IprevEvent::PermError)).caused_by(Error::from(err)) } mail_auth::IprevResult::TempError(err) => { - Event::new(EventType::Iprev(IprevEvent::TempError)).caused_by(Event::from(err)) + Error::new(EventType::Iprev(IprevEvent::TempError)).caused_by(Error::from(err)) } - mail_auth::IprevResult::None => Event::new(EventType::Iprev(IprevEvent::None)), + mail_auth::IprevResult::None => Error::new(EventType::Iprev(IprevEvent::None)), } .ctx_opt( Key::Details, @@ -356,9 +356,9 @@ impl From<&mail_auth::IprevOutput> for Event { } } -impl From<&mail_auth::SpfOutput> for Event { +impl From<&mail_auth::SpfOutput> for Error { fn from(value: &mail_auth::SpfOutput) -> Self { - Event::new(EventType::Spf(match value.result() { + Error::new(EventType::Spf(match value.result() { mail_auth::SpfResult::Pass => SpfEvent::Pass, mail_auth::SpfResult::Fail => SpfEvent::Fail, mail_auth::SpfResult::SoftFail => SpfEvent::SoftFail, diff --git a/crates/trc/src/event/mod.rs b/crates/trc/src/event/mod.rs index 61a1a0d4..b9a4462f 100644 --- a/crates/trc/src/event/mod.rs +++ b/crates/trc/src/event/mod.rs @@ -55,19 +55,33 @@ impl Event { } }) } + + pub fn into_boxed(self) -> Box { + Box::new(self) + } } -impl Event { +impl Error { + #[inline(always)] + pub fn new(inner: EventType) -> Self { + Error(Box::new(Event::new(inner))) + } + + #[inline(always)] + pub fn set_ctx(&mut self, key: Key, value: impl Into) { + self.0.keys.push((key, value.into())); + } + #[inline(always)] pub fn ctx(mut self, key: Key, value: impl Into) -> Self { - self.keys.push((key, value.into())); + self.0.keys.push((key, value.into())); self } #[inline(always)] pub fn ctx_unique(mut self, key: Key, value: impl Into) -> Self { - if self.keys.iter().all(|(k, _)| *k != key) { - self.keys.push((key, value.into())); + if self.0.keys.iter().all(|(k, _)| *k != key) { + self.0.keys.push((key, value.into())); } self } @@ -82,7 +96,12 @@ impl Event { #[inline(always)] pub fn matches(&self, inner: EventType) -> bool { - self.inner == inner + self.0.inner == inner + } + + #[inline(always)] + pub fn event_type(&self) -> EventType { + self.0.inner } #[inline(always)] @@ -135,13 +154,39 @@ impl Event { Error::new(cause).caused_by(self) } + #[inline(always)] + pub fn keys(&self) -> &[(Key, Value)] { + &self.0.keys + } + + #[inline(always)] + pub fn value(&self, key: Key) -> Option<&Value> { + self.0.value(key) + } + + #[inline(always)] + pub fn value_as_str(&self, key: Key) -> Option<&str> { + self.0.value_as_str(key) + } + + #[inline(always)] + pub fn value_as_uint(&self, key: Key) -> Option { + self.0.value_as_uint(key) + } + + #[inline(always)] + pub fn take_value(&mut self, key: Key) -> Option { + self.0.take_value(key) + } + #[inline(always)] pub fn is_assertion_failure(&self) -> bool { - self.inner == EventType::Store(StoreEvent::AssertValueFailed) + self.0.inner == EventType::Store(StoreEvent::AssertValueFailed) } pub fn key(&self, key: Key) -> Option<&Value> { - self.keys + self.0 + .keys .iter() .find_map(|(k, v)| if *k == key { Some(v) } else { None }) } @@ -149,7 +194,7 @@ impl Event { #[inline(always)] pub fn is_jmap_method_error(&self) -> bool { !matches!( - self.inner, + self.0.inner, EventType::Jmap( JmapEvent::UnknownCapability | JmapEvent::NotJson | JmapEvent::NotRequest ) @@ -159,7 +204,7 @@ impl Event { #[inline(always)] pub fn must_disconnect(&self) -> bool { matches!( - self.inner, + self.0.inner, EventType::Network(_) | EventType::Auth(AuthEvent::TooManyAttempts) | EventType::Limit(LimitEvent::ConcurrentRequest | LimitEvent::TooManyRequests) @@ -169,7 +214,7 @@ impl Event { #[inline(always)] pub fn should_write_err(&self) -> bool { - !matches!(self.inner, EventType::Network(_) | EventType::Security(_)) + !matches!(self.0.inner, EventType::Network(_) | EventType::Security(_)) } pub fn corrupted_key(key: &[u8], value: Option<&[u8]>, caused_by: &'static str) -> Error { @@ -644,7 +689,10 @@ impl AddContext for Result { fn caused_by(self, location: &'static str) -> Result { match self { Ok(value) => Ok(value), - Err(err) => Err(err.ctx(Key::CausedBy, location)), + Err(mut err) => { + err.set_ctx(Key::CausedBy, location); + Err(err) + } } } @@ -664,9 +712,9 @@ impl std::error::Error for Error {} impl Eq for Error {} impl PartialEq for Error { fn eq(&self, other: &Self) -> bool { - if self.inner == other.inner && self.keys.len() == other.keys.len() { - for kv in self.keys.iter() { - if !other.keys.iter().any(|okv| kv == okv) { + if self.0.inner == other.0.inner && self.0.keys.len() == other.0.keys.len() { + for kv in self.0.keys.iter() { + if !other.0.keys.iter().any(|okv| kv == okv) { return false; } } diff --git a/crates/trc/src/ipc/channel.rs b/crates/trc/src/ipc/channel.rs index 34aa2358..4eb01678 100644 --- a/crates/trc/src/ipc/channel.rs +++ b/crates/trc/src/ipc/channel.rs @@ -16,7 +16,7 @@ use rtrb::{Consumer, Producer, PushError, RingBuffer}; use crate::{ ipc::collector::{Update, COLLECTOR_THREAD, COLLECTOR_UPDATES}, - Event, EventType, + Error, Event, EventType, }; use super::collector::{Collector, CollectorThread}; @@ -113,3 +113,9 @@ impl Event { self.send(); } } + +impl Error { + pub fn send(self) { + self.0.send(); + } +} diff --git a/crates/trc/src/lib.rs b/crates/trc/src/lib.rs index d2e7d9fc..688a119d 100644 --- a/crates/trc/src/lib.rs +++ b/crates/trc/src/lib.rs @@ -21,7 +21,10 @@ pub use event_macro::event; use event_macro::{event_family, event_type, key_names, total_event_count}; pub type Result = std::result::Result; -pub type Error = Event; + +#[derive(Debug, Clone)] +#[repr(transparent)] +pub struct Error(Box>); #[derive(Debug, Clone)] pub struct Event { @@ -61,7 +64,7 @@ pub enum Value { Bool(bool), Ipv4(Ipv4Addr), Ipv6(Ipv6Addr), - Event(Event), + Event(Error), Array(Vec), #[default] None, diff --git a/crates/trc/src/macros.rs b/crates/trc/src/macros.rs index ce15ce91..a9352206 100644 --- a/crates/trc/src/macros.rs +++ b/crates/trc/src/macros.rs @@ -25,7 +25,7 @@ macro_rules! error { let event_id = err.as_ref().id(); if $crate::Collector::is_metric(event_id) { - $crate::Collector::record_metric(*err.as_ref(), event_id, &err.keys); + $crate::Collector::record_metric(*err.as_ref(), event_id, err.keys()); } if $crate::Collector::has_interest(event_id) { err.send(); diff --git a/crates/trc/src/serializers/binary.rs b/crates/trc/src/serializers/binary.rs index a2c987f4..de6ab99c 100644 --- a/crates/trc/src/serializers/binary.rs +++ b/crates/trc/src/serializers/binary.rs @@ -174,9 +174,9 @@ impl Value { } Value::Event(v) => { buf.push(11u8); - leb128_write(buf, v.inner.code()); - leb128_write(buf, v.keys.len() as u64); - for (k, v) in &v.keys { + leb128_write(buf, v.0.inner.code()); + leb128_write(buf, v.0.keys.len() as u64); + for (k, v) in &v.0.keys { leb128_write(buf, k.code()); v.serialize(buf); } @@ -258,7 +258,9 @@ impl Value { let value = Value::deserialize(iter)?; keys.push((key, value)); } - Some(Value::Event(Event::with_keys(code, keys))) + Some(Value::Event(Error( + Event::with_keys(code, keys).into_boxed(), + ))) } 12 => { let len = leb128_read(iter)?; diff --git a/crates/trc/src/serializers/json.rs b/crates/trc/src/serializers/json.rs index 215a0477..56b42637 100644 --- a/crates/trc/src/serializers/json.rs +++ b/crates/trc/src/serializers/json.rs @@ -4,7 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL */ -use crate::{Event, EventDetails, EventType, Key, Value}; +use crate::{Error, Event, EventDetails, Key, Value}; use ahash::AHashSet; use base64::{engine::general_purpose::STANDARD, Engine}; use mail_parser::DateTime; @@ -156,24 +156,24 @@ impl Serialize for JsonEventSerializer> { } } -impl Serialize for JsonEventSerializer<&Event> { +impl Serialize for JsonEventSerializer<&Error> { fn serialize(&self, serializer: S) -> Result where S: Serializer, { let mut map = serializer.serialize_map(None)?; - map.serialize_entry("type", self.inner.inner.name())?; + map.serialize_entry("type", self.inner.0.inner.name())?; if self.with_description { - map.serialize_entry("text", self.inner.inner.description())?; + map.serialize_entry("text", self.inner.0.inner.description())?; } if self.with_explanation { - map.serialize_entry("details", self.inner.inner.explain())?; + map.serialize_entry("details", self.inner.0.inner.explain())?; } map.serialize_entry( "data", &JsonEventSerializer { inner: Keys { - keys: self.inner.keys.as_slice(), + keys: self.inner.0.keys.as_slice(), span_keys: &[], }, with_spans: self.with_spans, diff --git a/crates/trc/src/serializers/text.rs b/crates/trc/src/serializers/text.rs index 6e816e75..e7375d07 100644 --- a/crates/trc/src/serializers/text.rs +++ b/crates/trc/src/serializers/text.rs @@ -9,7 +9,7 @@ use std::fmt::Display; use mail_parser::DateTime; use tokio::io::{AsyncWrite, AsyncWriteExt}; -use crate::{Event, EventDetails, EventType, Key, Level, Value}; +use crate::{Error, Event, EventDetails, Key, Level, Value}; use base64::{engine::general_purpose::STANDARD, Engine}; pub struct FmtWriter { @@ -240,17 +240,17 @@ impl FmtWriter { } Value::Event(e) => { self.writer - .write_all(e.inner.description().as_bytes()) + .write_all(e.0.inner.description().as_bytes()) .await?; self.writer.write_all(" (".as_bytes()).await?; - self.writer.write_all(e.inner.name().as_bytes()).await?; + self.writer.write_all(e.0.inner.name().as_bytes()).await?; self.writer.write_all(")".as_bytes()).await?; - if !e.keys.is_empty() { + if !e.0.keys.is_empty() { self.writer .write_all(if self.multiline { "\n" } else { " { " }.as_bytes()) .await?; - self.write_keys(&e.keys, &[], indent + 1).await?; + self.write_keys(&e.0.keys, &[], indent + 1).await?; if !self.multiline { self.writer.write_all(" }".as_bytes()).await?; @@ -354,16 +354,16 @@ impl Display for Value { } } -impl Display for Event { +impl Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.inner.description().fmt(f)?; + self.0.inner.description().fmt(f)?; " (".fmt(f)?; - self.inner.name().fmt(f)?; + self.0.inner.name().fmt(f)?; ")".fmt(f)?; - if !self.keys.is_empty() { + if !self.0.keys.is_empty() { f.write_str(": ")?; - for (i, (key, value)) in self.keys.iter().enumerate() { + for (i, (key, value)) in self.0.keys.iter().enumerate() { if i > 0 { f.write_str(", ")?; } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 24b8a8d0..a3053a98 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -5,9 +5,9 @@ edition = "2021" resolver = "2" [features] -default = ["sqlite", "postgres", "mysql", "rocks", "elastic", "s3", "redis", "azure", "foundationdb"] +#default = ["sqlite", "postgres", "mysql", "rocks", "elastic", "s3", "redis", "azure", "foundationdb"] #default = ["sqlite", "postgres", "mysql", "rocks", "s3", "redis", "foundationdb"] -#default = ["rocks"] +default = ["rocks"] sqlite = ["store/sqlite"] foundationdb = ["store/foundation", "common/foundation"] postgres = ["store/postgres"] diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index 1abed89d..c283654a 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -372,7 +372,7 @@ pub async fn jmap_tests() { .await; webhooks::test(&mut params).await; - /*email_query::test(&mut params, delete).await; + email_query::test(&mut params, delete).await; email_get::test(&mut params).await; email_set::test(&mut params).await; email_parse::test(&mut params).await; @@ -385,7 +385,7 @@ pub async fn jmap_tests() { mailbox::test(&mut params).await; delivery::test(&mut params).await; auth_acl::test(&mut params).await; - auth_limits::test(&mut params).await;*/ + auth_limits::test(&mut params).await; auth_oauth::test(&mut params).await; event_source::test(&mut params).await; push_subscription::test(&mut params).await; diff --git a/tests/src/smtp/config.rs b/tests/src/smtp/config.rs index 1a9d17ea..23490c0e 100644 --- a/tests/src/smtp/config.rs +++ b/tests/src/smtp/config.rs @@ -426,15 +426,18 @@ async fn eval_if() { //println!("============= Testing {:?} ==================", key); let (_, expected_result) = key.rsplit_once('-').unwrap(); assert_eq!( - IfBlock { - key: key.to_string(), - if_then: vec![IfThen { - expr: Expression::try_parse(&mut config, key.as_str(), &token_map).unwrap(), - then: Expression::from(true), - }], - default: Expression::from(false), - } - .eval(&envelope, &core, 0) + core.eval_if::( + &IfBlock { + key: key.to_string(), + if_then: vec![IfThen { + expr: Expression::try_parse(&mut config, key.as_str(), &token_map).unwrap(), + then: Expression::from(true), + }], + default: Expression::from(false), + }, + &envelope, + 0 + ) .await .unwrap() .to_bool(), @@ -485,7 +488,7 @@ async fn eval_dynvalue() { .unwrap_or_else(|| panic!("Missing expect for test {test_name:?}")); assert_eq!( - String::try_from(if_block.eval(&envelope, &core, 0).await.unwrap()).ok(), + core.eval_if::(&if_block, &envelope, 0).await, expected, "failed for test {test_name:?}" );