Skip to content

Commit

Permalink
Don't require that a UDF is a function (#31727)
Browse files Browse the repository at this point in the history
UDFs should not be called as if they were javascript functions.

i.e.

```ts
export const foo = mutation(handler);

await foo(ctx); // this is not supported
```

To that end, analyze and UDF execution should not require the registered UDFs to be functions. They only need to be objects.

GitOrigin-RevId: 524eb3ac07aeff03f3436a80dbd7ac9d74f7a6f7
  • Loading branch information
ldanilek authored and Convex, Inc. committed Dec 16, 2024
1 parent 76fcd5a commit 620d92b
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 15 deletions.
2 changes: 1 addition & 1 deletion crates/isolate/src/environment/action/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ impl<RT: Runtime> ActionEnvironment<RT> {
);
return Ok(Err(JsError::from_message(message)));
}
let function: v8::Local<v8::Function> = namespace
let function: v8::Local<v8::Object> = namespace
.get(&mut scope, function_str)
.ok_or_else(|| anyhow!("Did not find function in module after checking?"))?
.try_into()?;
Expand Down
30 changes: 20 additions & 10 deletions crates/isolate/src/environment/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ fn make_str_val<'s>(
#[minitrace::trace]
fn parse_args_validator<'s, RT: Runtime>(
scope: &mut ExecutionScope<RT, AnalyzeEnvironment>,
function: v8::Local<v8::Function>,
function: v8::Local<v8::Object>,
function_identifier_for_error: String,
) -> anyhow::Result<Result<ArgsValidator, JsError>> {
// Call `exportArgs` to get the args validator.
Expand Down Expand Up @@ -503,7 +503,7 @@ fn parse_args_validator<'s, RT: Runtime>(
#[minitrace::trace]
fn parse_returns_validator<'s, RT: Runtime>(
scope: &mut ExecutionScope<RT, AnalyzeEnvironment>,
function: v8::Local<v8::Function>,
function: v8::Local<v8::Object>,
function_identifier_for_error: String,
) -> anyhow::Result<Result<ReturnsValidator, JsError>> {
// TODO(CX-6287) unify argument and returns validators
Expand Down Expand Up @@ -585,7 +585,7 @@ fn udf_analyze<RT: Runtime>(
let property_value = namespace
.get(scope, property_name)
.ok_or_else(|| anyhow!("Failed to get property name on module namespace"))?;
let function: v8::Local<v8::Function> = match property_value.try_into() {
let function: v8::Local<v8::Object> = match property_value.try_into() {
Ok(f) => f,
Err(_) => continue,
};
Expand Down Expand Up @@ -647,12 +647,17 @@ fn udf_analyze<RT: Runtime>(
let handler: v8::Local<v8::Function> = handler_value.try_into()?;
handler
},
Some(handler_value) if handler_value.is_undefined() => function,
Some(_) => {
Some(handler_value) if !handler_value.is_undefined() => {
let message = format!("{module_path:?}:{property_name}.handler is not a function.");
return Ok(Err(JsError::from_message(message)));
},
None => function,
_ => match function.try_into() {
Ok(f) => f,
Err(_) => {
let message = format!("{module_path:?}:{property_name} is not a function.");
return Ok(Err(JsError::from_message(message)));
},
},
};

// These are originally zero-indexed, so we just add 1
Expand Down Expand Up @@ -880,7 +885,7 @@ fn http_analyze<RT: Runtime>(
let Some(function) = entry.get_index(scope, 2) else {
return routes_error(format!("problem with third element of {} of array", i).as_str());
};
let function: Result<v8::Local<v8::Function>, _> = function.try_into();
let function: Result<v8::Local<v8::Object>, _> = function.try_into();
let Ok(function) = function else {
return routes_error(format!("arr[{}][2] not an HttpAction", i).as_str());
};
Expand All @@ -891,12 +896,17 @@ fn http_analyze<RT: Runtime>(
let handler: v8::Local<v8::Function> = handler_value.try_into()?;
handler
},
Some(handler_value) if handler_value.is_undefined() => function,
Some(_) => {
Some(handler_value) if !handler_value.is_undefined() => {
let message = format!("arr[{}][2].handler is not a function", i);
return Ok(Err(JsError::from_message(message)));
},
None => function,
_ => match function.try_into() {
Ok(f) => f,
Err(_) => {
let message = format!("arr[{}][2] is not an HttpAction", i);
return Ok(Err(JsError::from_message(message)));
},
},
};

// These are originally zero-indexed, so we just add 1
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/src/environment/udf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
);
return Ok(Err(JsError::from_message(message)));
}
let function: v8::Local<v8::Function> = namespace
let function: v8::Local<v8::Object> = namespace
.get(&mut scope, function_str)
.ok_or_else(|| anyhow!("Did not find function in module after checking?"))?
.try_into()?;
Expand Down
6 changes: 3 additions & 3 deletions crates/isolate/src/isolate2/entered_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'enter, 'scope: 'enter> EnteredContext<'enter, 'scope> {
FunctionNotFoundError::new(udf_path.function_name(), udf_path.module().as_str());
anyhow::bail!(JsError::from_message(err.to_string()));
}
let function: v8::Local<v8::Function> = exports
let function: v8::Local<v8::Object> = exports
.get(self.scope, name_str)
.ok_or_else(|| {
let err = FunctionNotFoundError::new(
Expand Down Expand Up @@ -335,7 +335,7 @@ impl<'enter, 'scope: 'enter> EnteredContext<'enter, 'scope> {
&mut self,
udf_type: UdfType,
udf_path: &CanonicalizedUdfPath,
function: &v8::Local<v8::Function>,
function: &v8::Local<v8::Object>,
) -> anyhow::Result<v8::Local<'scope, v8::String>> {
let is_query = self.classify_function_object(&strings::isQuery, function)?;
let is_mutation = self.classify_function_object(&strings::isMutation, function)?;
Expand Down Expand Up @@ -381,7 +381,7 @@ impl<'enter, 'scope: 'enter> EnteredContext<'enter, 'scope> {
fn classify_function_object(
&mut self,
function_type: &'static StaticString,
function: &v8::Local<v8::Function>,
function: &v8::Local<v8::Object>,
) -> anyhow::Result<bool> {
let function_type_str = function_type.create(self.scope)?.into();
let has_function_type = function.has(self.scope, function_type_str) == Some(true);
Expand Down

0 comments on commit 620d92b

Please sign in to comment.