Merge pull request #4163 from systeminit/nick/eng-2603

Increase log level and log when module not found for schema
systeminit · Jul 16, 2024 · c3ae11a · c3ae11a
2 parents 5fc05dc + ca59b78
commit c3ae11a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 14 deletions.
diff --git a/lib/dal/src/module.rs b/lib/dal/src/module.rs
@@ -41,8 +41,6 @@ pub enum ModuleError {
     MissingSchemaId(String, String),
     #[error("node weight error: {0}")]
     NodeWeight(#[from] NodeWeightError),
-    #[error("module not found for schema: {0}")]
-    NotFoundForSchema(SchemaId),
     #[error("schema error: {0}")]
     Schema(#[from] SchemaError),
     #[error("schema variant error: {0}")]
@@ -416,15 +414,16 @@ impl Module {
     /// [`Modules`](Module) can be upgraded and installed.
     #[instrument(
         name = "module.sync"
+        level = "info",
         skip_all,
-        level = "debug",
+        fields(
+            latest_modules_count = latest_modules.len()
+        )
     )]
     pub async fn sync(
         ctx: &DalContext,
         latest_modules: Vec<frontend_types::LatestModule>,
     ) -> ModuleResult<frontend_types::SyncedModules> {
-        debug!("working with {} latest modules", latest_modules.len());
-
         let start = Instant::now();
 
         // Collect all user facing schema variants. We need to see what can be upgraded.
@@ -438,10 +437,14 @@ impl Module {
             seen_schema_ids.insert(schema_id);
 
             if let Entry::Vacant(entry) = local_hashes.entry(schema_id) {
-                let local_module = Self::find_for_member_id(ctx, schema_id)
-                    .await?
-                    .ok_or(ModuleError::NotFoundForSchema(schema_id))?;
-                entry.insert(local_module.root_hash().to_owned());
+                match Self::find_for_member_id(ctx, schema_id).await? {
+                    Some(found_local_module) => {
+                        entry.insert(found_local_module.root_hash().to_owned());
+                    }
+                    None => {
+                        error!(%schema_id, %schema_variant.schema_variant_id, "found orphaned schema (has no corresponding module)");
+                    }
+                }
             }
         }
 
@@ -477,7 +480,7 @@ impl Module {
                 synced_modules.installable.push(latest_module.to_owned());
             }
         }
-        trace!(?synced_modules.installable, "collected installable modules");
+        debug!(?synced_modules.installable, "collected installable modules");
 
         // Populate upgradeable modules.
         for schema_variant in schema_variants {
@@ -506,9 +509,9 @@ impl Module {
                 }
             }
         }
-        trace!(?synced_modules.upgradeable, "collected upgradeable modules");
+        debug!(?synced_modules.upgradeable, "collected upgradeable modules");
 
-        debug!("syncing modules took: {:?}", start.elapsed());
+        info!("syncing modules took: {:?}", start.elapsed());
 
         Ok(synced_modules)
     }

diff --git a/lib/sdf-server/src/server/service/v2/module.rs b/lib/sdf-server/src/server/service/v2/module.rs
@@ -37,8 +37,10 @@ impl IntoResponse for ModulesAPIError {
             Self::Transactions(dal::TransactionsError::ConflictsOccurred(_)) => {
                 StatusCode::CONFLICT
             }
-            Self::Module(dal::module::ModuleError::NotFoundForSchema(_))
-            | Self::SchemaVariant(dal::SchemaVariantError::NotFound(_)) => StatusCode::NOT_FOUND,
+            Self::SchemaVariant(dal::SchemaVariantError::NotFound(schema_variant_id)) => {
+                error!(%schema_variant_id, "schema variant not found");
+                StatusCode::NOT_FOUND
+            }
             _ => ApiError::DEFAULT_ERROR_STATUS_CODE,
         };