Skip to content

Commit

Permalink
Improve error messages in components (#27664)
Browse files Browse the repository at this point in the history
improve error messages when printing a UDF path or module that is in a component other than the root.
also reduce usage of functions `as_root_udf_path` and `into_root_udf_path` which throw a system error when given a path that's not in the root component.

GitOrigin-RevId: 10689577d5fe4a10329d5f6a98ddb8299cd134ce
  • Loading branch information
ldanilek authored and Convex, Inc. committed Jul 8, 2024
1 parent 5a6d356 commit 11fc9ea
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 29 deletions.
5 changes: 2 additions & 3 deletions crates/application/src/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,9 @@ pub struct CacheKey {
impl fmt::Debug for CacheKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut builder = f.debug_struct("CacheKey");
if let Ok(p) = self.path.as_root_udf_path() {
builder.field("path", p);
}
builder
.field("path", &self.path.udf_path)
.field("component", &self.path.component)
.field("args", &self.args)
.field("identity", &self.identity)
.field("journal", &self.journal)
Expand Down
7 changes: 4 additions & 3 deletions crates/application/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,9 +1157,10 @@ impl<RT: Runtime> Application<RT> {
})
else {
let missing_or_internal = format!(
"Could not find function for '{}'. Did you forget to run `npx convex dev` or `npx \
convex deploy`?",
String::from(canonicalized_path.into_root_udf_path()?.strip())
"Could not find function for '{}'{}. Did you forget to run `npx convex dev` or \
`npx convex deploy`?",
String::from(canonicalized_path.udf_path.strip()),
canonicalized_path.component.in_component_str(),
);
return Ok(Err(FunctionError {
error: RedactedJsError::from_js_error(
Expand Down
2 changes: 1 addition & 1 deletion crates/application/src/tests/cron_jobs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async fn create_cron_job(
udf_path: "basic:insertObject".parse()?,
};
let cron_spec = CronSpec {
udf_path: path.as_root_udf_path()?.clone().canonicalize(),
udf_path: path.udf_path.clone().canonicalize(),
udf_args: parse_udf_args(&path, vec![JsonValue::Object(map)])?,
cron_schedule: CronSchedule::Interval { seconds: 60 },
};
Expand Down
11 changes: 11 additions & 0 deletions crates/common/src/components/component_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ impl ComponentPath {
path.push(name);
Self { path }
}

/// Returns a debug or error string to put immediately after something which
/// should be scoped to this component,
/// like `format!("'{udf_path}'{}", component_path.in_component_str())`.
pub fn in_component_str(&self) -> String {
if self.is_root() {
"".to_string()
} else {
format!(" in '{}'", String::from(self.clone()))
}
}
}

impl Deref for ComponentPath {
Expand Down
10 changes: 0 additions & 10 deletions crates/common/src/components/function_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ pub struct ComponentFunctionPath {
}

impl ComponentFunctionPath {
pub fn as_root_udf_path(&self) -> anyhow::Result<&UdfPath> {
anyhow::ensure!(self.component.is_root());
Ok(&self.udf_path)
}

pub fn into_root_udf_path(self) -> anyhow::Result<UdfPath> {
anyhow::ensure!(self.component.is_root());
Ok(self.udf_path)
Expand Down Expand Up @@ -97,11 +92,6 @@ pub struct CanonicalizedComponentFunctionPath {
}

impl CanonicalizedComponentFunctionPath {
pub fn as_root_udf_path(&self) -> anyhow::Result<&CanonicalizedUdfPath> {
anyhow::ensure!(self.component.is_root());
Ok(&self.udf_path)
}

pub fn into_root_udf_path(self) -> anyhow::Result<CanonicalizedUdfPath> {
anyhow::ensure!(self.component.is_root());
Ok(self.udf_path)
Expand Down
22 changes: 13 additions & 9 deletions crates/isolate/src/environment/helpers/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ pub async fn validate_schedule_args<RT: Runtime>(
"InvalidScheduledFunction",
format!(
"Attempted to schedule function, but no exported function {} found in the \
file: {}. Did you forget to export it?",
file: {}{}. Did you forget to export it?",
function_name,
String::from(path.as_root_udf_path()?.module().clone()),
String::from(path.udf_path.module().clone()),
path.component.in_component_str(),
),
));
}
Expand All @@ -144,9 +145,10 @@ pub async fn validate_schedule_args<RT: Runtime>(

fn missing_or_internal_error(path: &CanonicalizedComponentFunctionPath) -> anyhow::Result<String> {
Ok(format!(
"Could not find public function for '{}'. Did you forget to run `npx convex dev` or `npx \
convex deploy`?",
String::from(path.as_root_udf_path()?.clone().strip())
"Could not find public function for '{}'{}. Did you forget to run `npx convex dev` or \
`npx convex deploy`?",
String::from(path.udf_path.clone().strip()),
path.component.in_component_str()
))
}

Expand Down Expand Up @@ -414,8 +416,9 @@ impl ValidatedPathAndArgs {
},
None => {
anyhow::bail!(
"No visibility found for analyzed function {}",
path.as_root_udf_path()?
"No visibility found for analyzed function {}{}",
path.udf_path,
path.component.in_component_str(),
);
},
},
Expand All @@ -424,8 +427,9 @@ impl ValidatedPathAndArgs {
if expected_udf_type != analyzed_function.udf_type {
anyhow::ensure!(path.component.is_root());
return Ok(Err(JsError::from_message(format!(
"Trying to execute {} as {}, but it is defined as {}.",
path.as_root_udf_path()?,
"Trying to execute {}{} as {}, but it is defined as {}.",
path.udf_path,
path.component.in_component_str(),
expected_udf_type,
analyzed_function.udf_type
))));
Expand Down
6 changes: 5 additions & 1 deletion crates/isolate/src/isolate2/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,11 @@ async fn run_request<RT: Runtime>(
query_journal: QueryJournal,
) -> anyhow::Result<UdfOutcome> {
let (path, arguments, udf_server_version) = path_and_args.consume();
let udf_path = path.as_root_udf_path()?;
anyhow::ensure!(
path.component.is_root(),
"TODO: non-root components not supported yet"
);
let udf_path = &path.udf_path;

// Spawn a separate Tokio thread to receive log lines.
let (log_line_tx, log_line_rx) = oneshot::channel();
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/src/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ impl<RT: Runtime, P: Persistence + Clone> UdfTest<RT, P> {
};

self.database
.commit_with_write_source(tx, Some(canonicalized_path.into_root_udf_path()?.into()))
.commit_with_write_source(tx, Some(canonicalized_path.udf_path.into()))
.await?;
Ok(outcome)
}
Expand Down
6 changes: 5 additions & 1 deletion crates/model/src/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> {
.ok_or_else(|| {
ErrorMetadata::bad_request(
"InvalidReference",
format!("Module {:?} not found", udf_path.module()),
format!(
"Module {:?}{} not found",
udf_path.module(),
component_path.in_component_str()
),
)
})?;
let analyze_result = module_metadata
Expand Down

0 comments on commit 11fc9ea

Please sign in to comment.