Skip to content

Commit 123ae7e

Browse files
anonriganonrig
committed
simplify result handling and errors
1 parent 9b96eed commit 123ae7e

File tree

11 files changed

+281
-162
lines changed

11 files changed

+281
-162
lines changed

‎src/rust/api/dns.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ impl From<DnsParserError> for jsg::Error {
2020
fn from(val: DnsParserError) -> Self {
2121
match val {
2222
DnsParserError::InvalidHexString(msg) | DnsParserError::InvalidDnsResponse(msg) => {
23-
Self::new("Error", msg)
23+
Self::new_error(&msg)
2424
}
25-
DnsParserError::ParseIntError(msg) => Self::new("RangeError", msg.to_string()),
26-
DnsParserError::Unknown => Self::new("Error", "Unknown dns parser error".to_owned()),
25+
DnsParserError::ParseIntError(msg) => Self::new_range_error(msg.to_string()),
26+
DnsParserError::Unknown => Self::new_error("Unknown dns parser error"),
2727
}
2828
}
2929
}

‎src/rust/jsg-macros/lib.rs‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,13 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
111111
.map(|(i, ty)| {
112112
let arg = syn::Ident::new(&format!("arg{i}"), fn_name.span());
113113
let unwrap = quote! {
114-
let Ok(#arg) = <#ty as jsg::FromJS>::from_js(&mut lock, args.get(#i)) else { return; };
114+
let #arg = match <#ty as jsg::FromJS>::from_js(&mut lock, args.get(#i)) {
115+
Ok(v) => v,
116+
Err(err) => {
117+
lock.throw_exception(&err);
118+
return;
119+
}
120+
};
115121
};
116122
// For reference types (like &str), FromJS returns an owned type (String),
117123
// so we need to borrow it when passing to the function.
@@ -125,6 +131,22 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
125131
})
126132
.unzip();
127133

134+
// Check if return type is Result<T, E>
135+
let is_result = matches!(&fn_sig.output, syn::ReturnType::Type(_, ty) if is_result_type(ty));
136+
137+
let result_handling = if is_result {
138+
quote! {
139+
match result {
140+
Ok(value) => args.set_return_value(jsg::ToJS::to_js(value, &mut lock)),
141+
Err(err) => lock.throw_exception(&err.into()),
142+
}
143+
}
144+
} else {
145+
quote! {
146+
args.set_return_value(jsg::ToJS::to_js(result, &mut lock));
147+
}
148+
};
149+
128150
quote! {
129151
#fn_vis #fn_sig { #fn_block }
130152

@@ -136,7 +158,7 @@ pub fn jsg_method(_attr: TokenStream, item: TokenStream) -> TokenStream {
136158
let this = args.this();
137159
let self_ = jsg::unwrap_resource::<Self>(&mut lock, this);
138160
let result = self_.#fn_name(#(#arg_exprs),*);
139-
args.set_return_value(jsg::ToJS::to_js(result, &mut lock));
161+
#result_handling
140162
}
141163
}
142164
.into()
@@ -314,3 +336,13 @@ fn snake_to_camel(s: &str) -> String {
314336
}
315337
result
316338
}
339+
340+
/// Checks if a type is `Result<T, E>`.
341+
fn is_result_type(ty: &syn::Type) -> bool {
342+
if let syn::Type::Path(type_path) = ty
343+
&& let Some(segment) = type_path.path.segments.last()
344+
{
345+
return segment.ident == "Result";
346+
}
347+
false
348+
}

‎src/rust/jsg-test/lib.rs‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::pin::Pin;
22

3+
use jsg::FromJS;
34
use jsg::v8;
45
use kj_rs::KjOwn;
56

@@ -47,9 +48,33 @@ pub struct EvalContext<'a> {
4748
isolate: v8::IsolatePtr,
4849
}
4950

51+
#[derive(Debug)]
52+
pub enum EvalError<'a> {
53+
UncoercibleResult {
54+
value: v8::Local<'a, v8::Value>,
55+
message: String,
56+
},
57+
Exception(v8::Local<'a, v8::Value>),
58+
EvalFailed,
59+
}
60+
61+
impl EvalError<'_> {
62+
/// Extracts a `jsg::Error` from an `EvalError::Exception` variant.
63+
///
64+
/// # Panics
65+
///
66+
/// Panics if `self` is not `EvalError::Exception`, or if the value cannot be converted to a
67+
/// `jsg::Error`.
68+
pub fn unwrap_jsg_err(&self, lock: &mut jsg::Lock) -> jsg::Error {
69+
match self {
70+
EvalError::Exception(value) => jsg::Error::from_js(lock, value.clone())
71+
.expect("Failed to convert exception to jsg::Error"),
72+
_ => panic!("Unexpected error"),
73+
}
74+
}
75+
}
5076
impl EvalContext<'_> {
51-
// TODO(soon): There is no way to distinguish a throw versus an error as a return value.
52-
pub fn eval<T>(&self, lock: &mut jsg::Lock, code: &str) -> Result<T, jsg::Error>
77+
pub fn eval<T>(&self, lock: &mut jsg::Lock, code: &str) -> Result<T, EvalError<'_>>
5378
where
5479
T: jsg::FromJS<ResultType = T>,
5580
{
@@ -59,20 +84,24 @@ impl EvalContext<'_> {
5984
if result.success {
6085
match opt_local {
6186
Some(local) => {
62-
T::from_js(lock, unsafe { v8::Local::from_ffi(self.isolate, local) })
87+
let local = unsafe { v8::Local::from_ffi(self.isolate, local) };
88+
match T::from_js(lock, local.clone()) {
89+
Err(e) => Err(EvalError::UncoercibleResult {
90+
value: local,
91+
message: e.to_string(),
92+
}),
93+
Ok(value) => Ok(value),
94+
}
6395
}
64-
None => Err(jsg::Error::new(
65-
"Error",
66-
"eval returned empty result".to_owned(),
67-
)),
96+
None => unreachable!(),
6897
}
6998
} else {
7099
match opt_local {
71100
Some(local) => {
72101
let value = unsafe { v8::Local::from_ffi(self.isolate, local) };
73-
Err(jsg::Error::from_value(lock, value))
102+
Err(EvalError::Exception(value))
74103
}
75-
None => Err(jsg::Error::new("Error", "eval failed".to_owned())),
104+
None => Err(EvalError::EvalFailed),
76105
}
77106
}
78107
}

‎src/rust/jsg-test/tests/eval.rs‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
use crate::EvalError;
2+
13
#[test]
24
fn eval_returns_correct_type() {
35
let harness = crate::Harness::new();
46
harness.run_in_context(|lock, ctx| {
5-
let result: String = ctx.eval(lock, "'Hello, World!'")?;
7+
let result: String = ctx.eval(lock, "'Hello, World!'").unwrap();
68
assert_eq!(result, "Hello, World!");
79
Ok(())
810
});
@@ -12,7 +14,7 @@ fn eval_returns_correct_type() {
1214
fn eval_string_concatenation() {
1315
let harness = crate::Harness::new();
1416
harness.run_in_context(|lock, ctx| {
15-
let result: String = ctx.eval(lock, "'Hello' + ', ' + 'World!'")?;
17+
let result: String = ctx.eval(lock, "'Hello' + ', ' + 'World!'").unwrap();
1618
assert_eq!(result, "Hello, World!");
1719
Ok(())
1820
});
@@ -22,7 +24,7 @@ fn eval_string_concatenation() {
2224
fn eval_number_returns_number_type() {
2325
let harness = crate::Harness::new();
2426
harness.run_in_context(|lock, ctx| {
25-
let result: f64 = ctx.eval(lock, "42")?;
27+
let result: f64 = ctx.eval(lock, "42").unwrap();
2628
assert!((result - 42.0).abs() < f64::EPSILON);
2729
Ok(())
2830
});
@@ -32,7 +34,7 @@ fn eval_number_returns_number_type() {
3234
fn eval_arithmetic_expression() {
3335
let harness = crate::Harness::new();
3436
harness.run_in_context(|lock, ctx| {
35-
let result: f64 = ctx.eval(lock, "1 + 2 + 3")?;
37+
let result: f64 = ctx.eval(lock, "1 + 2 + 3").unwrap();
3638
assert!((result - 6.0).abs() < f64::EPSILON);
3739
Ok(())
3840
});
@@ -42,7 +44,7 @@ fn eval_arithmetic_expression() {
4244
fn eval_boolean_true() {
4345
let harness = crate::Harness::new();
4446
harness.run_in_context(|lock, ctx| {
45-
let result: bool = ctx.eval(lock, "true")?;
47+
let result: bool = ctx.eval(lock, "true").unwrap();
4648
assert!(result);
4749
Ok(())
4850
});
@@ -52,7 +54,7 @@ fn eval_boolean_true() {
5254
fn eval_boolean_false() {
5355
let harness = crate::Harness::new();
5456
harness.run_in_context(|lock, ctx| {
55-
let result: bool = ctx.eval(lock, "false")?;
57+
let result: bool = ctx.eval(lock, "false").unwrap();
5658
assert!(!result);
5759
Ok(())
5860
});
@@ -62,7 +64,7 @@ fn eval_boolean_false() {
6264
fn eval_comparison_expression() {
6365
let harness = crate::Harness::new();
6466
harness.run_in_context(|lock, ctx| {
65-
let result: bool = ctx.eval(lock, "5 > 3")?;
67+
let result: bool = ctx.eval(lock, "5 > 3").unwrap();
6668
assert!(result);
6769
Ok(())
6870
});
@@ -72,7 +74,7 @@ fn eval_comparison_expression() {
7274
fn eval_null() {
7375
let harness = crate::Harness::new();
7476
harness.run_in_context(|lock, ctx| {
75-
let result: jsg::Nullable<bool> = ctx.eval(lock, "null")?;
77+
let result: jsg::Nullable<bool> = ctx.eval(lock, "null").unwrap();
7678
assert!(result.is_null());
7779
Ok(())
7880
});
@@ -82,11 +84,13 @@ fn eval_null() {
8284
fn eval_throws_on_error() {
8385
let harness = crate::Harness::new();
8486
harness.run_in_context(|lock, ctx| {
85-
let result: Result<bool, jsg::Error> = ctx.eval(lock, "throw new Error('test error')");
86-
assert!(result.is_err());
87-
let err = result.unwrap_err();
88-
assert_eq!(err.name, "Error");
89-
assert_eq!(err.message, "test error");
87+
let result = ctx.eval::<bool>(lock, "throw new Error('test error')");
88+
match result.unwrap_err() {
89+
EvalError::Exception(value) => {
90+
assert_eq!(value.to_string(), "Error: test error");
91+
}
92+
_ => panic!("Unexpected error type"),
93+
}
9094
Ok(())
9195
});
9296
}
@@ -95,11 +99,13 @@ fn eval_throws_on_error() {
9599
fn eval_throws_string_preserves_message() {
96100
let harness = crate::Harness::new();
97101
harness.run_in_context(|lock, ctx| {
98-
let result: Result<bool, jsg::Error> = ctx.eval(lock, "throw 'custom string error'");
99-
assert!(result.is_err());
100-
let err = result.unwrap_err();
101-
assert_eq!(err.name, "Error");
102-
assert_eq!(err.message, "custom string error");
102+
let result = ctx.eval::<bool>(lock, "throw 'custom string error'");
103+
match result.unwrap_err() {
104+
EvalError::Exception(value) => {
105+
assert_eq!(value.to_string(), "custom string error");
106+
}
107+
_ => panic!("Unexpected error type"),
108+
}
103109
Ok(())
104110
});
105111
}
@@ -108,7 +114,9 @@ fn eval_throws_string_preserves_message() {
108114
fn eval_function_call() {
109115
let harness = crate::Harness::new();
110116
harness.run_in_context(|lock, ctx| {
111-
let result: String = ctx.eval(lock, "(function() { return 'from function'; })()")?;
117+
let result: String = ctx
118+
.eval(lock, "(function() { return 'from function'; })()")
119+
.unwrap();
112120
assert_eq!(result, "from function");
113121
Ok(())
114122
});
@@ -118,7 +126,7 @@ fn eval_function_call() {
118126
fn eval_typeof_string() {
119127
let harness = crate::Harness::new();
120128
harness.run_in_context(|lock, ctx| {
121-
let result: String = ctx.eval(lock, "typeof 'hello'")?;
129+
let result: String = ctx.eval(lock, "typeof 'hello'").unwrap();
122130
assert_eq!(result, "string");
123131
Ok(())
124132
});
@@ -128,7 +136,7 @@ fn eval_typeof_string() {
128136
fn eval_typeof_number() {
129137
let harness = crate::Harness::new();
130138
harness.run_in_context(|lock, ctx| {
131-
let result: String = ctx.eval(lock, "typeof 42")?;
139+
let result: String = ctx.eval(lock, "typeof 42").unwrap();
132140
assert_eq!(result, "number");
133141
Ok(())
134142
});
@@ -138,7 +146,7 @@ fn eval_typeof_number() {
138146
fn eval_typeof_boolean() {
139147
let harness = crate::Harness::new();
140148
harness.run_in_context(|lock, ctx| {
141-
let result: String = ctx.eval(lock, "typeof true")?;
149+
let result: String = ctx.eval(lock, "typeof true").unwrap();
142150
assert_eq!(result, "boolean");
143151
Ok(())
144152
});
@@ -148,7 +156,7 @@ fn eval_typeof_boolean() {
148156
fn eval_unicode_string() {
149157
let harness = crate::Harness::new();
150158
harness.run_in_context(|lock, ctx| {
151-
let result: String = ctx.eval(lock, "'こんにちは'")?;
159+
let result: String = ctx.eval(lock, "'こんにちは'").unwrap();
152160
assert_eq!(result, "こんにちは");
153161
Ok(())
154162
});
@@ -158,7 +166,7 @@ fn eval_unicode_string() {
158166
fn eval_emoji_string() {
159167
let harness = crate::Harness::new();
160168
harness.run_in_context(|lock, ctx| {
161-
let result: String = ctx.eval(lock, "'😀🎉'")?;
169+
let result: String = ctx.eval(lock, "'😀🎉'").unwrap();
162170
assert_eq!(result, "😀🎉");
163171
Ok(())
164172
});

‎src/rust/jsg-test/tests/jsg_struct.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,8 @@ fn nested_object_properties() {
157157

158158
#[test]
159159
fn error_creation_and_display() {
160-
let error = Error::default();
161-
assert_eq!(error.name, "Error");
162-
assert_eq!(error.message, "An unknown error occurred");
163-
164-
let type_error = Error::new("TypeError", "Invalid type".to_owned());
165-
assert_eq!(type_error.name, "TypeError");
160+
let type_error = Error::new_type_error("Invalid type");
161+
assert_eq!(type_error.name, ExceptionType::TypeError);
166162
assert_eq!(type_error.message, "Invalid type");
167163

168164
// Test Display for all exception types
@@ -178,7 +174,7 @@ fn error_creation_and_display() {
178174
fn error_from_parse_int_error() {
179175
let parse_result: Result<i32, _> = "not_a_number".parse();
180176
let error: Error = parse_result.unwrap_err().into();
181-
assert_eq!(error.name, "TypeError");
177+
assert_eq!(error.name, ExceptionType::RangeError);
182178
assert!(error.message.contains("Failed to parse integer"));
183179
}
184180

0 commit comments

Comments
 (0)