Skip to content

Commit

Permalink
fix: ensure metadata loading errors are correctly logged (#2149)
Browse files Browse the repository at this point in the history
  • Loading branch information
glihm authored Jul 7, 2024
1 parent ed9f0ce commit 28d19df
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 32 deletions.
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl AuthArgs {
trace!(metadata=?env_metadata, "Loaded environment.");

let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws)?;

match self.command {
AuthCommand::Grant { kind, world, starknet, account, transaction } => {
Expand Down
28 changes: 21 additions & 7 deletions bin/sozo/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ impl BuildArgs {
pub fn run(self, config: &Config) -> Result<()> {
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;

if let Ok(current_package) = ws.current_package() {
if current_package.target(&TargetKind::new("dojo")).is_none() {
return Err(anyhow::anyhow!(
"No Dojo target found in the {} package. Add [[target.dojo]] to the {} \
manifest to enable Dojo features and compile with sozo.",
current_package.id.to_string(),
current_package.manifest_path()
));
}
}

// Namespaces are required to compute contracts/models data. Hence, we can't continue
// if no metadata are found.
// Once sozo will support package option, users will be able to do `-p` to select
// the package directly from the workspace instead of using `--manifest-path`.
let dojo_metadata = dojo_metadata_from_workspace(&ws)?;

let profile_name =
ws.current_profile().expect("Scarb profile is expected at this point.").to_string();

Expand Down Expand Up @@ -126,13 +143,10 @@ impl BuildArgs {
};
trace!(pluginManager=?bindgen, "Generating bindings.");

// Only generate bindgen if a current package is defined with dojo metadata.
if let Ok(dojo_metadata) = dojo_metadata_from_workspace(&ws) {
tokio::runtime::Runtime::new()
.unwrap()
.block_on(bindgen.generate(dojo_metadata.skip_migration))
.expect("Error generating bindings");
};
tokio::runtime::Runtime::new()
.unwrap()
.block_on(bindgen.generate(dojo_metadata.skip_migration))
.expect("Error generating bindings");

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl CallArgs {
self.tag_or_address
} else {
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws)?;
ensure_namespace(&self.tag_or_address, &default_namespace)
};

Expand Down
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl ExecuteArgs {
self.tag_or_address
} else {
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws)?;
ensure_namespace(&self.tag_or_address, &default_namespace)
};

Expand Down
2 changes: 1 addition & 1 deletion bin/sozo/src/commands/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl ModelArgs {
trace!(args = ?self);
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
let env_metadata = utils::load_metadata_from_config(config)?;
let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws)?;

config.tokio_handle().block_on(async {
match self.command {
Expand Down
2 changes: 1 addition & 1 deletion bin/sozo/tests/register_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async fn reregister_models() {
let target_path =
ws.target_dir().path_existent().unwrap().join(ws.config().profile().to_string());

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let migration = prepare_migration(
source_project_dir.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/dojo-lang/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl Compiler for DojoCompiler {
let props: Props = unit.main_component().target_props()?;
let target_dir = unit.target_dir(ws);

let default_namespace = get_default_namespace_from_ws(ws);
let default_namespace = get_default_namespace_from_ws(ws)?;

let compiler_config = build_compiler_config(&unit, ws);

Expand Down
25 changes: 15 additions & 10 deletions crates/dojo-world/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ fn build_artifact_from_filename(
/// # Returns
///
/// A [`String`] object containing the namespace.
pub fn get_default_namespace_from_ws(ws: &Workspace<'_>) -> String {
let metadata = dojo_metadata_from_workspace(ws)
.expect("Namespace key is already checked by the parsing of the Scarb.toml file.");
metadata.world.namespace
pub fn get_default_namespace_from_ws(ws: &Workspace<'_>) -> Result<String> {
let metadata = dojo_metadata_from_workspace(ws)?;
Ok(metadata.world.namespace)
}

/// Build world metadata with data read from the project configuration.
Expand Down Expand Up @@ -99,13 +98,18 @@ pub fn dojo_metadata_from_workspace(ws: &Workspace<'_>) -> Result<DojoMetadata>
let source_dir = source_dir.join(profile.as_str());

let project_metadata = if let Ok(current_package) = ws.current_package() {
current_package.manifest.metadata.dojo()?
current_package
.manifest
.metadata
.dojo()
.with_context(|| format!("Error parsing manifest file `{}`", ws.manifest_path()))?
} else {
// On workspaces, dojo metadata are not accessible because if no current package is defined
// (being the only package or using --package).
return Err(anyhow!(
"No current package with dojo metadata found, this subcommand is not yet support for \
workspaces."
"No current package with dojo metadata found, virtual manifest in workspace are not \
supported. Until package compilation is supported, you will have to provide the path \
to the Scarb.toml file using the --manifest-path option."
));
};

Expand Down Expand Up @@ -438,13 +442,14 @@ impl MetadataExt for ManifestMetadata {
.tool_metadata
.as_ref()
.and_then(|e| e.get("dojo"))
// TODO: see if we can make error more descriptive
.ok_or_else(|| anyhow!("Some of the fields in [tool.dojo] are required."))?
.with_context(|| "No [tool.dojo] section found in the manifest.".to_string())?
.clone();

// The details of which field has failed to be loaded are logged inside the `try_into`
// error.
let project_metadata: ProjectMetadata = metadata
.try_into()
.with_context(|| "Project metadata (i.e. [tool.dojo]) is not properly configured.")?;
.with_context(|| "Project metadata [tool.dojo] is not properly configured.")?;

Ok(project_metadata)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/sozo/ops/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
let target_dir = ws.target_dir().path_existent().unwrap();
let target_dir = target_dir.join(ws.config().profile().as_str());

let default_namespace = get_default_namespace_from_ws(ws);
let default_namespace = get_default_namespace_from_ws(ws)?;

// Load local and remote World manifests.
let (local_manifest, remote_manifest) = utils::load_world_manifests(
Expand Down
6 changes: 3 additions & 3 deletions crates/sozo/ops/src/tests/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() {
// adding correct calldata
manifest.merge(overlay);
}
let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let mut world = WorldDiff::compute(manifest, Some(remote_manifest));
world.update_order(&default_namespace).expect("Failed to update order");
Expand Down Expand Up @@ -375,7 +375,7 @@ async fn migrate_with_auto_authorize() {
let world_address = migration.world_address().expect("must be present");
let world = WorldContract::new(world_address, account);

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();
let res =
auto_authorize(&ws, &world, &txn_config, &manifest, &output, &default_namespace).await;
assert!(res.is_ok());
Expand Down Expand Up @@ -408,7 +408,7 @@ async fn migration_with_mismatching_world_address_and_seed() {
let base_dir = config.manifest_path().parent().unwrap().to_path_buf();
let target_dir = base_dir.join("target").join("dev");

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let strategy = prepare_migration_with_world_and_seed(
base_dir,
Expand Down
2 changes: 1 addition & 1 deletion crates/sozo/ops/src/tests/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn setup_migration(config: &Config) -> Result<MigrationStrategy> {
let base_dir = manifest_path.parent().unwrap();
let target_dir = format!("{}/target/dev", base_dir);

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

prepare_migration_with_world_and_seed(
base_dir.into(),
Expand Down
4 changes: 2 additions & 2 deletions crates/torii/core/src/sql_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async fn test_load_from_remote() {
let base_dir = manifest_path.parent().unwrap();
let target_dir = format!("{}/target/dev", base_dir);

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let mut migration = prepare_migration(
base_dir.into(),
Expand Down Expand Up @@ -222,7 +222,7 @@ async fn test_load_from_remote_del() {
let base_dir = manifest_path.parent().unwrap();
let target_dir = format!("{}/target/dev", base_dir);

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let mut migration = prepare_migration(
base_dir.into(),
Expand Down
2 changes: 1 addition & 1 deletion crates/torii/graphql/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ pub async fn spinup_types_test() -> Result<SqlitePool> {

let target_path = ws.target_dir().path_existent().unwrap().join(config.profile().to_string());

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let mut migration = prepare_migration(
source_project_dir,
Expand Down
2 changes: 1 addition & 1 deletion crates/torii/grpc/src/server/tests/entities_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ async fn test_entities_queries() {

let target_path = ws.target_dir().path_existent().unwrap().join(config.profile().to_string());

let default_namespace = get_default_namespace_from_ws(&ws);
let default_namespace = get_default_namespace_from_ws(&ws).unwrap();

let mut migration = prepare_migration(
config.manifest_path().parent().unwrap().into(),
Expand Down

0 comments on commit 28d19df

Please sign in to comment.