Skip to content

Commit d754df5

Browse files
committed
fix(rivetkit): preserve internal bridge errors
1 parent 4e070ea commit d754df5

51 files changed

Lines changed: 2460 additions & 594 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,10 @@ When the user asks to track something in a note, store it in `.agent/notes/` by
222222

223223
## Memory Leaks
224224

225-
- Never call `Box::leak` inside a per-request, per-error, or per-call code path. If the leak is for a `'static` reference required by an upstream API (e.g. `RivetErrorSchema`), intern the leaked value through a process-global `LazyLock<scc::HashMap<Key, &'static T>>` keyed on its identity so each unique value is leaked at most once. Examples: `BRIDGE_RIVET_ERROR_SCHEMAS` in `rivetkit-typescript/packages/rivetkit-napi/src/actor_factory.rs`.
226-
- If every field in a leaked struct is a compile-time constant, use a `static`/`const` instead of `Box::leak(Box::new(...))`.
227-
- `std::mem::forget` is only acceptable when an FFI handle cannot be dropped in the current context (e.g. napi `Ref::unref` requires an `Env`). Document the constraint inline and ensure the leak is bounded per actor/connection lifetime, not per call. Prefer routing the drop through an Env-bearing thread when possible.
225+
- Do not introduce intentional leaks (`Box::leak`, `std::mem::forget`, `*_into_raw` without matching cleanup) unless an upstream API makes ownership impossible to express safely.
226+
- Never call `Box::leak` inside a per-request, per-error, or per-call code path; if a `'static` reference is required, use a compile-time `static`/`const` or intern it through a process-global map keyed by identity.
227+
- Interned leaks must be bounded by unique schema/config identity and must not include unbounded user input such as raw error messages, SQL, actor keys, request paths, or headers.
228+
- `std::mem::forget` is only acceptable when an FFI handle cannot be dropped in the current context; document the constraint inline, prove the leak is bounded, and prefer routing cleanup through an Env-bearing owner.
228229
- Spawned futures that capture JS callbacks or other heavy resources must have a guaranteed completion path (e.g. a `CancellationToken` whose clones are guaranteed to drop). A `spawn_local(async move { token.cancelled().await; ... })` only drains if every clone of the token is dropped or cancelled.
229230

230231
## Async Rust Locks

engine/packages/api-builder/src/error_response.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ impl IntoResponse for ApiError {
3939
(
4040
StatusCode::INTERNAL_SERVER_ERROR,
4141
ErrorResponse::from(&RivetError {
42-
schema: &rivet_error::INTERNAL_ERROR,
42+
kind: rivet_error::RivetErrorKind::Static(&rivet_error::INTERNAL_ERROR),
4343
meta: None,
4444
message: None,
45+
actor: None,
4546
}),
4647
)
4748
};
@@ -84,8 +85,8 @@ pub struct ErrorResponse {
8485
impl From<&RivetError> for ErrorResponse {
8586
fn from(value: &RivetError) -> Self {
8687
ErrorResponse {
87-
group: value.group().into(),
88-
code: value.code().into(),
88+
group: value.group().to_owned().into(),
89+
code: value.code().to_owned().into(),
8990
message: value.message().into(),
9091
metadata: value.metadata(),
9192
}

engine/packages/error-macros/src/lib.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ fn derive_struct_error(input: DeriveInput, data_struct: &syn::DataStruct) -> Tok
9292
.and_then(|v| ::serde_json::value::to_raw_value(&v).ok());
9393

9494
let error = RivetError {
95-
schema: &SCHEMA,
95+
kind: rivet_error::RivetErrorKind::Static(&SCHEMA),
9696
meta: meta_json,
9797
message: None,
98+
actor: None,
9899
};
99100
::anyhow::Error::new(error)
100101
}
@@ -193,9 +194,10 @@ fn derive_struct_error(input: DeriveInput, data_struct: &syn::DataStruct) -> Tok
193194
let meta_json = ::serde_json::value::to_raw_value(&meta_value).ok();
194195

195196
let error = RivetError {
196-
schema: &SCHEMA,
197+
kind: rivet_error::RivetErrorKind::Static(&SCHEMA),
197198
meta: meta_json,
198199
message: None,
200+
actor: None,
199201
};
200202
::anyhow::Error::new(error)
201203
}
@@ -365,9 +367,10 @@ fn derive_enum_error(input: DeriveInput, data_enum: &syn::DataEnum) -> TokenStre
365367
let meta_json = ::serde_json::value::to_raw_value(&meta_value).ok();
366368

367369
let error = RivetError {
368-
schema: &SCHEMA,
370+
kind: rivet_error::RivetErrorKind::Static(&SCHEMA),
369371
meta: meta_json,
370372
message: None,
373+
actor: None,
371374
};
372375
::anyhow::Error::new(error)
373376
}
@@ -454,9 +457,10 @@ fn derive_enum_error(input: DeriveInput, data_enum: &syn::DataEnum) -> TokenStre
454457
let meta_json = ::serde_json::value::to_raw_value(&meta_value).ok();
455458

456459
let error = RivetError {
457-
schema: &SCHEMA,
460+
kind: rivet_error::RivetErrorKind::Static(&SCHEMA),
458461
meta: meta_json,
459462
message: None,
463+
actor: None,
460464
};
461465
::anyhow::Error::new(error)
462466
}

engine/packages/error/src/error.rs

Lines changed: 122 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::INTERNAL_ERROR;
22
use crate::schema::RivetErrorSchema;
3-
use serde::Serialize;
3+
use serde::{Deserialize, Serialize};
44
use std::{fmt, sync::OnceLock};
55

66
static EXPOSE_INTERNAL_ERRORS: OnceLock<bool> = OnceLock::new();
@@ -10,20 +10,111 @@ fn expose_internal_errors() -> bool {
1010
.get_or_init(|| matches!(std::env::var("RIVET_EXPOSE_ERRORS").as_deref(), Ok("1")))
1111
}
1212

13+
/// Identifies the actor that was handling work when an error was produced.
14+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
15+
#[serde(rename_all = "camelCase")]
16+
pub struct ActorSpecifier {
17+
pub actor_id: String,
18+
pub generation: u64,
19+
#[serde(skip_serializing_if = "Option::is_none")]
20+
pub key: Option<String>,
21+
}
22+
23+
impl ActorSpecifier {
24+
pub fn new(actor_id: impl Into<String>, generation: u64) -> Self {
25+
Self {
26+
actor_id: actor_id.into(),
27+
generation,
28+
key: None,
29+
}
30+
}
31+
32+
pub fn with_key(mut self, key: impl Into<String>) -> Self {
33+
self.key = Some(key.into());
34+
self
35+
}
36+
}
37+
38+
impl fmt::Display for ActorSpecifier {
39+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
40+
match &self.key {
41+
Some(key) => write!(
42+
f,
43+
"actor {} generation {} key {}",
44+
self.actor_id, self.generation, key
45+
),
46+
None => write!(f, "actor {} generation {}", self.actor_id, self.generation),
47+
}
48+
}
49+
}
50+
51+
impl std::error::Error for ActorSpecifier {}
52+
53+
#[derive(Debug, Clone)]
54+
pub enum RivetErrorKind {
55+
Static(&'static RivetErrorSchema),
56+
Dynamic {
57+
group: String,
58+
code: String,
59+
default_message: String,
60+
},
61+
}
62+
63+
impl RivetErrorKind {
64+
pub fn group(&self) -> &str {
65+
match self {
66+
Self::Static(schema) => schema.group,
67+
Self::Dynamic { group, .. } => group,
68+
}
69+
}
70+
71+
pub fn code(&self) -> &str {
72+
match self {
73+
Self::Static(schema) => schema.code,
74+
Self::Dynamic { code, .. } => code,
75+
}
76+
}
77+
78+
pub fn default_message(&self) -> &str {
79+
match self {
80+
Self::Static(schema) => schema.default_message,
81+
Self::Dynamic {
82+
default_message, ..
83+
} => default_message,
84+
}
85+
}
86+
87+
pub fn schema(&self) -> Option<&'static RivetErrorSchema> {
88+
match self {
89+
Self::Static(schema) => Some(schema),
90+
Self::Dynamic { .. } => None,
91+
}
92+
}
93+
}
94+
1395
#[derive(Debug, Clone)]
1496
pub struct RivetError {
15-
pub schema: &'static RivetErrorSchema,
97+
pub kind: RivetErrorKind,
1698
pub meta: Option<Box<serde_json::value::RawValue>>,
1799
pub message: Option<String>,
100+
pub actor: Option<ActorSpecifier>,
18101
}
19102

20103
impl RivetError {
21104
pub fn extract(error: &anyhow::Error) -> Self {
22-
error
105+
// `anyhow::Error::downcast_ref` walks both the chain and any
106+
// `.context(...)` wrappers, so this finds an `ActorSpecifier` no matter
107+
// where it was attached.
108+
let actor = error.downcast_ref::<ActorSpecifier>().cloned();
109+
let mut extracted = error
23110
.chain()
24111
.find_map(|x| x.downcast_ref::<Self>())
25112
.cloned()
26-
.unwrap_or_else(|| INTERNAL_ERROR.build_internal(error))
113+
.unwrap_or_else(|| INTERNAL_ERROR.build_internal(error));
114+
if extracted.actor.is_none() {
115+
extracted.actor = actor;
116+
}
117+
extracted
27118
}
28119

29120
pub(crate) fn build_internal(error: &anyhow::Error) -> Self {
@@ -34,31 +125,45 @@ impl RivetError {
34125
let meta = serde_json::value::to_raw_value(&meta_json).ok();
35126

36127
Self {
37-
schema: &INTERNAL_ERROR,
128+
kind: RivetErrorKind::Static(&INTERNAL_ERROR),
38129
meta,
39130
message: expose_internal_errors().then(|| format!("Internal error: {}", error)),
131+
actor: None,
40132
}
41133
}
42134

43-
pub fn group(&self) -> &'static str {
44-
self.schema.group
135+
pub fn group(&self) -> &str {
136+
self.kind.group()
45137
}
46138

47-
pub fn code(&self) -> &'static str {
48-
self.schema.code
139+
pub fn code(&self) -> &str {
140+
self.kind.code()
49141
}
50142

51143
pub fn message(&self) -> &str {
52144
self.message
53145
.as_deref()
54-
.unwrap_or(self.schema.default_message)
146+
.unwrap_or_else(|| self.kind.default_message())
55147
}
56148

57149
pub fn metadata(&self) -> Option<serde_json::Value> {
58150
self.meta
59151
.as_ref()
60152
.and_then(|raw| serde_json::from_str(raw.get()).ok())
61153
}
154+
155+
pub fn actor(&self) -> Option<&ActorSpecifier> {
156+
self.actor.as_ref()
157+
}
158+
159+
pub fn with_actor(mut self, actor: ActorSpecifier) -> Self {
160+
self.actor = Some(actor);
161+
self
162+
}
163+
164+
pub fn schema(&self) -> Option<&'static RivetErrorSchema> {
165+
self.kind.schema()
166+
}
62167
}
63168

64169
impl fmt::Display for RivetError {
@@ -76,11 +181,9 @@ impl Serialize for RivetError {
76181
{
77182
use serde::ser::SerializeStruct;
78183

79-
let mut state = if self.meta.is_some() {
80-
serializer.serialize_struct("RivetError", 4)?
81-
} else {
82-
serializer.serialize_struct("RivetError", 3)?
83-
};
184+
let field_count =
185+
3 + usize::from(self.meta.is_some()) + usize::from(self.actor.is_some());
186+
let mut state = serializer.serialize_struct("RivetError", field_count)?;
84187

85188
state.serialize_field("group", self.group())?;
86189
state.serialize_field("code", self.code())?;
@@ -90,6 +193,10 @@ impl Serialize for RivetError {
90193
state.serialize_field("meta", meta)?;
91194
}
92195

196+
if let Some(actor) = &self.actor {
197+
state.serialize_field("actor", actor)?;
198+
}
199+
93200
state.end()
94201
}
95202
}

engine/packages/error/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
mod error;
22
mod schema;
33

4-
pub use error::RivetError;
4+
pub use error::{ActorSpecifier, RivetError, RivetErrorKind};
55
pub use schema::{MacroMarker, RivetErrorSchema, RivetErrorSchemaWithMeta};
66

77
pub use rivet_error_macros::RivetError;

engine/packages/error/src/schema.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::error::RivetError;
1+
use crate::error::{RivetError, RivetErrorKind};
22
use serde::Serialize;
33
use std::marker::PhantomData;
44

@@ -48,9 +48,10 @@ impl RivetErrorSchema {
4848
/// Builds an anyhow::Error from this schema
4949
pub fn build(&'static self) -> anyhow::Error {
5050
let error = RivetError {
51-
schema: self,
51+
kind: RivetErrorKind::Static(self),
5252
meta: None,
5353
message: None,
54+
actor: None,
5455
};
5556
anyhow::Error::new(error)
5657
}
@@ -67,9 +68,10 @@ impl<T: Serialize> RivetErrorSchemaWithMeta<T> {
6768
let meta_json = serde_json::value::to_raw_value(&meta).ok();
6869

6970
let error = RivetError {
70-
schema: &self.schema,
71+
kind: RivetErrorKind::Static(&self.schema),
7172
meta: meta_json,
7273
message: Some(message),
74+
actor: None,
7375
};
7476
anyhow::Error::new(error)
7577
}
@@ -78,9 +80,10 @@ impl<T: Serialize> RivetErrorSchemaWithMeta<T> {
7880
impl From<&'static RivetErrorSchema> for RivetError {
7981
fn from(value: &'static RivetErrorSchema) -> Self {
8082
RivetError {
81-
schema: value,
83+
kind: RivetErrorKind::Static(value),
8284
meta: None,
8385
message: None,
86+
actor: None,
8487
}
8588
}
8689
}

engine/packages/error/tests/basic.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,32 @@ fn test_internal_error_extraction() {
6868
assert!(rivet_error.meta.is_some());
6969
}
7070

71+
#[test]
72+
fn test_internal_error_preserves_actor_specifier() {
73+
let regular_error =
74+
anyhow::anyhow!("Some random error").context(ActorSpecifier::new("actor-2", 8));
75+
let rivet_error = RivetError::extract(&regular_error);
76+
77+
assert_eq!(rivet_error.group(), "core");
78+
assert_eq!(
79+
rivet_error.actor(),
80+
Some(&ActorSpecifier::new("actor-2", 8))
81+
);
82+
}
83+
84+
#[test]
85+
fn test_actor_specifier_extracted_from_middle_of_chain() {
86+
let error = anyhow::anyhow!("Some random error")
87+
.context(ActorSpecifier::new("actor-3", 9))
88+
.context("dispatching action");
89+
let rivet_error = RivetError::extract(&error);
90+
91+
assert_eq!(
92+
rivet_error.actor(),
93+
Some(&ActorSpecifier::new("actor-3", 9))
94+
);
95+
}
96+
7197
#[test]
7298
fn test_error_serialization() {
7399
init_test();
@@ -83,6 +109,26 @@ fn test_error_serialization() {
83109
assert!(value.get("meta").is_none());
84110
}
85111

112+
#[test]
113+
fn test_actor_specifier_serialization() {
114+
init_test();
115+
let error = TestError
116+
.build()
117+
.context(ActorSpecifier::new("actor-1", 7).with_key("user:1"));
118+
let rivet_error = RivetError::extract(&error);
119+
120+
assert_eq!(
121+
rivet_error.actor(),
122+
Some(&ActorSpecifier::new("actor-1", 7).with_key("user:1"))
123+
);
124+
125+
let value: serde_json::Value =
126+
serde_json::from_str(&serde_json::to_string(&rivet_error).unwrap()).unwrap();
127+
assert_eq!(value["actor"]["actorId"], "actor-1");
128+
assert_eq!(value["actor"]["generation"], 7);
129+
assert_eq!(value["actor"]["key"], "user:1");
130+
}
131+
86132
#[test]
87133
fn test_meta_error_serialization() {
88134
init_test();

0 commit comments

Comments
 (0)