Improve parser error context for manifests, lockfiles, and config

packages/zpm-config/src/lib.rs

@@ -1,7 +1,7 @@
 use std::{collections::BTreeMap, fmt::Display, ops::Deref, sync::Arc, time::UNIX_EPOCH};
 
 use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
-use zpm_utils::{AbstractValue, Container, Cpu, DataType, FromFileString, IoResultExt, LastModifiedAt, Libc, Os, Path, RawString, Serialized, System, ToFileString, ToHumanString, tree};
+use zpm_utils::{AbstractValue, Container, Cpu, DataType, EcoString, FromFileString, IoResultExt, LastModifiedAt, Libc, Os, Path, RawString, Serialized, System, ToFileString, ToHumanString, tree};
 
 #[derive(Debug, Clone)]
 pub struct ConfigurationContext {
@@ -793,6 +793,18 @@ pub enum ConfigurationError {
 
     #[error(transparent)]
     SerdeError(#[from] Arc<serde_yaml::Error>),
+
+    #[error("Invalid user configuration file ({}): {message}", path.to_print_string())]
+    UserConfigParseError {
+        path: Path,
+        message: EcoString,
+    },
+
+    #[error("Invalid project configuration file ({}): {message}", path.to_print_string())]
+    ProjectConfigParseError {
+        path: Path,
+        message: EcoString,
+    },
 }
 
 impl From<std::io::Error> for ConfigurationError {
@@ -882,7 +894,11 @@ impl Configuration {
                         .fs_read_text_with_size(metadata.len())?;
 
                 let user_config: intermediate::Settings
-                    = serde_yaml::from_str(&user_config_text)?;
+                    = serde_yaml::from_str(&user_config_text)
+                        .map_err(|error| ConfigurationError::UserConfigParseError {
+                            path: user_config_path.clone(),
+                            message: error.to_string().into(),
+                        })?;
 
                 intermediate_user_config = Partial::Value(user_config);
             }
@@ -906,7 +922,11 @@ impl Configuration {
                         .fs_read_text_with_size(metadata.len())?;
 
                 let project_config: intermediate::Settings
-                    = serde_yaml::from_str(&project_config_text)?;
+                    = serde_yaml::from_str(&project_config_text)
+                        .map_err(|error| ConfigurationError::ProjectConfigParseError {
+                            path: project_config_path.clone(),
+                            message: error.to_string().into(),
+                        })?;
 
                 intermediate_project_config = Partial::Value(project_config);
             }

packages/zpm/src/content_flags.rs

@@ -9,7 +9,7 @@ use zpm_primitives::{Ident, Locator, Reference};
 use zpm_utils::{Path, Requirements};
 
 use crate::{
-    build, error::Error, fetchers::PackageData, manifest::bin::BinField
+    build, error::{Error, remote_manifest_parse_error}, fetchers::PackageData, manifest::bin::BinField
 };
 
 static UNPLUG_SCRIPTS: &[&str] = &["preinstall", "install", "postinstall"];
@@ -156,7 +156,8 @@ impl ContentFlags {
             = zpm_formats::zip::first_entry_from_zip(&package_bytes)?;
 
         let meta_manifest: Manifest
-            = JsonDocument::hydrate_from_slice(&first_entry.data)?;
+            = JsonDocument::hydrate_from_slice(&first_entry.data)
+                .map_err(|error| remote_manifest_parse_error(locator, "cached package archive", "package.json", error))?;
 
         let mut build_commands = UNPLUG_SCRIPTS.iter()
             .filter_map(|k| meta_manifest.scripts.get(*k).map(|s| (k, s)))

packages/zpm/src/error.rs

@@ -1,7 +1,7 @@
 use std::{future::Future, sync::Arc};
 
 use zpm_primitives::{Descriptor, Ident, Locator, Range};
-use zpm_utils::{DataType, Path, ToHumanString};
+use zpm_utils::{DataType, EcoString, Path, ToHumanString};
 use tokio::task::JoinError;
 
 fn render_backtrace(backtrace: &std::backtrace::Backtrace) -> String {
@@ -12,6 +12,50 @@ fn render_backtrace(backtrace: &std::backtrace::Backtrace) -> String {
     }
 }
 
+fn render_remote_manifest_reason(raw_reason: &str) -> String {
+    let first_line = raw_reason.lines().next().unwrap_or(raw_reason).trim();
+
+    if let Some(rest) = first_line.strip_prefix("invalid type: ")
+        && let Some((actual, expected_tail)) = rest.split_once(", expected ")
+    {
+        if let Some((expected, line_col)) = expected_tail.split_once(" at line ") {
+            return format!(
+                "type mismatch: expected {}, got {} (line {})",
+                expected.trim(),
+                actual.trim(),
+                line_col.trim(),
+            );
+        }
+
+        return format!(
+            "type mismatch: expected {}, got {}",
+            expected_tail.trim(),
+            actual.trim(),
+        );
+    }
+
+    first_line.to_string()
+}
+
+pub fn remote_manifest_parse_error(
+    locator: &Locator,
+    origin: impl Into<EcoString>,
+    path: impl Into<EcoString>,
+    parser_error: zpm_parsers::Error,
+) -> Error {
+    let raw_reason = match &parser_error {
+        zpm_parsers::Error::InvalidSyntax(message) => message.clone(),
+        _ => parser_error.to_string(),
+    };
+
+    Error::RemoteManifestParseError {
+        locator: locator.clone(),
+        origin: origin.into(),
+        path: path.into(),
+        reason: render_remote_manifest_reason(&raw_reason).into(),
+    }
+}
+
 pub async fn set_timeout<F: Future>(timeout: std::time::Duration, f: F) -> Result<F::Output, Error> {
     let res = tokio::time::timeout(timeout, f).await
         .map_err(|_| Error::TaskTimeout)?;
@@ -144,6 +188,14 @@ pub enum Error {
     #[error("File parsing error ({0})")]
     FileParsingError(#[from] zpm_parsers::Error),
 
+    #[error("Invalid package metadata in {path} ({origin}): {reason}")]
+    RemoteManifestParseError {
+        locator: Locator,
+        origin: EcoString,
+        path: EcoString,
+        reason: EcoString,
+    },
+
     #[error("Semver error ({0})")]
     SemverError(#[from] zpm_semver::Error),
 
@@ -165,8 +217,11 @@ pub enum Error {
     #[error("Package manifest not found ({})", .0.to_print_string())]
     ManifestNotFound(Path),
 
-    #[error("Package manifest failed to parse ({}): {}", .0.to_print_string(), .1)]
-    ManifestParseError(Path, Arc<dyn std::error::Error + Send + Sync>),
+    #[error("Package manifest failed to parse ({}): {reason}", path.to_print_string())]
+    ManifestParseError {
+        path: Path,
+        reason: EcoString,
+    },
 
     #[error("Invalid descriptor ({0})")]
     InvalidDescriptor(String),
@@ -249,8 +304,11 @@ pub enum Error {
     #[error("An error occured while reading the lockfile from disk")]
     LockfileReadError(Arc<std::io::Error>),
 
-    #[error("An error occured while parsing the lockfile: {0}")]
-    LockfileParseError(zpm_parsers::Error),
+    #[error("An error occured while parsing the lockfile ({}): {reason}", path.to_print_string())]
+    LockfileParseError {
+        path: Path,
+        reason: EcoString,
+    },
 
     #[error("Can't perform this operation without a git root")]
     NoGitRoot,
@@ -261,8 +319,11 @@ pub enum Error {
     #[error("No merge base could be found between any of HEAD and {args}", args = .0.join(", "))]
     NoMergeBaseFound(Vec<String>),
 
-    #[error("An error occured while parsing the Yarn Berry lockfile: {0}")]
-    LegacyLockfileParseError(Arc<serde_yaml::Error>),
+    #[error("An error occured while parsing the Yarn Berry lockfile ({}): {reason}", path.to_print_string())]
+    LegacyLockfileParseError {
+        path: Path,
+        reason: EcoString,
+    },
 
     #[error("Failed to read pnpm node_modules directory")]
     PnpmNodeModulesReadError,
@@ -500,6 +561,30 @@ pub enum Error {
     SilentError,
 }
 
+#[cfg(test)]
+mod tests {
+    use super::render_remote_manifest_reason;
+
+    #[test]
+    fn renders_manifest_type_mismatch_compactly() {
+        let reason = render_remote_manifest_reason(
+            "invalid type: string \"glibc\", expected a sequence at line 1 column 1687",
+        );
+
+        assert_eq!(
+            reason,
+            "type mismatch: expected a sequence, got string \"glibc\" (line 1 column 1687)"
+        );
+    }
+
+    #[test]
+    fn keeps_first_line_for_multiline_reasons() {
+        let reason = render_remote_manifest_reason("foo\nbar\nbaz");
+
+        assert_eq!(reason, "foo");
+    }
+}
+
 impl Error {
     pub fn ignore<T, F: FnOnce(&Error) -> bool>(self, f: F) -> Result<Option<T>, Error> {
         match f(&self) {

packages/zpm/src/fetchers/folder.rs

@@ -2,7 +2,7 @@ use zpm_parsers::JsonDocument;
 use zpm_primitives::{FolderReference, Locator};
 
 use crate::{
-    error::Error, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, npm::NpmEntryExt, resolvers::Resolution
+    error::{Error, remote_manifest_parse_error}, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, npm::NpmEntryExt, resolvers::Resolution
 };
 
 use super::PackageData;
@@ -49,7 +49,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
         = zpm_formats::zip::first_entry_from_zip(&pkg_blob.data)?;
 
     let remote_manifest: RemoteManifest
-        = JsonDocument::hydrate_from_slice(&first_entry.data)?;
+        = JsonDocument::hydrate_from_slice(&first_entry.data)
+            .map_err(|error| remote_manifest_parse_error(locator, "package archive", "package.json", error))?;
 
     let resolution
         = Resolution::from_remote_manifest(locator.clone(), remote_manifest);

packages/zpm/src/fetchers/git.rs

@@ -3,7 +3,7 @@ use zpm_parsers::JsonDocument;
 use zpm_primitives::{GitReference, Locator};
 
 use crate::{
-    error::Error, git, install::{FetchResult, InstallContext}, manifest::RemoteManifest, npm::NpmEntryExt, prepare, resolvers::Resolution
+    error::{Error, remote_manifest_parse_error}, git, install::{FetchResult, InstallContext}, manifest::RemoteManifest, npm::NpmEntryExt, prepare, resolvers::Resolution
 };
 
 use super::PackageData;
@@ -62,7 +62,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
         = zpm_formats::zip::first_entry_from_zip(&pkg_blob.data)?;
 
     let remote_manifest: RemoteManifest
-        = JsonDocument::hydrate_from_slice(&first_entry.data)?;
+        = JsonDocument::hydrate_from_slice(&first_entry.data)
+            .map_err(|error| remote_manifest_parse_error(locator, "package archive", "package.json", error))?;
 
     let resolution
         = Resolution::from_remote_manifest(locator.clone(), remote_manifest);

packages/zpm/src/fetchers/patch.rs

@@ -4,7 +4,7 @@ use zpm_primitives::{Ident, Locator, PatchReference};
 use zpm_utils::Hash64;
 
 use crate::{
-    error::Error, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, misc::unpack_brotli_data, npm::NpmEntryExt, patch::apply::apply_patch, resolvers::Resolution
+    error::{Error, remote_manifest_parse_error}, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, misc::unpack_brotli_data, npm::NpmEntryExt, patch::apply::apply_patch, resolvers::Resolution
 };
 
 use super::PackageData;
@@ -97,6 +97,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
 
     let package_subdir
         = locator.ident.nm_subdir();
+    let locator_for_patch_source
+        = locator.clone();
 
     let cached_blob = package_cache.upsert_blob(locator.clone(), ".zip", || async {
         let original_bytes = match &original_data.package_data {
@@ -130,7 +132,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
                 .ok_or(Error::MissingPackageManifest)?;
 
         let manifest: RemoteManifest
-            = JsonDocument::hydrate_from_slice(&package_json_entry.data)?;
+            = JsonDocument::hydrate_from_slice(&package_json_entry.data)
+                .map_err(|error| remote_manifest_parse_error(&locator_for_patch_source, "patch source archive", "package.json", error))?;
 
         let package_version
             = manifest.version
@@ -159,7 +162,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
         = zpm_formats::zip::first_entry_from_zip(&cached_blob.data)?;
 
     let manifest: RemoteManifest
-        = JsonDocument::hydrate_from_slice(&package_json_entry.data)?;
+        = JsonDocument::hydrate_from_slice(&package_json_entry.data)
+            .map_err(|error| remote_manifest_parse_error(&locator, "patched package archive", "package.json", error))?;
 
     let resolution
         = Resolution::from_remote_manifest(locator.clone(), manifest);

packages/zpm/src/fetchers/tarball.rs

@@ -3,7 +3,7 @@ use zpm_parsers::JsonDocument;
 use zpm_primitives::{Locator, TarballReference};
 
 use crate::{
-    error::Error, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, npm::NpmEntryExt, resolvers::Resolution
+    error::{Error, remote_manifest_parse_error}, install::{FetchResult, InstallContext, InstallOpResult}, manifest::RemoteManifest, npm::NpmEntryExt, resolvers::Resolution
 };
 
 use super::PackageData;
@@ -63,7 +63,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
         = zpm_formats::zip::first_entry_from_zip(&cached_blob.data)?;
 
     let manifest: RemoteManifest
-        = JsonDocument::hydrate_from_slice(&first_entry.data)?;
+        = JsonDocument::hydrate_from_slice(&first_entry.data)
+            .map_err(|error| remote_manifest_parse_error(locator, "package archive", "package.json", error))?;
 
     let resolution
         = Resolution::from_remote_manifest(locator.clone(), manifest);

packages/zpm/src/fetchers/url.rs

@@ -5,7 +5,7 @@ use zpm_parsers::JsonDocument;
 use zpm_primitives::{Locator, UrlReference};
 
 use crate::{
-    error::Error,
+    error::{Error, remote_manifest_parse_error},
     http_npm::{self, AuthorizationMode, GetAuthorizationOptions},
     install::{FetchResult, InstallContext},
     manifest::RemoteManifest,
@@ -91,7 +91,8 @@ pub async fn fetch_locator<'a>(context: &InstallContext<'a>, locator: &Locator,
         = zpm_formats::zip::first_entry_from_zip(&cached_blob.data)?;
 
     let manifest: RemoteManifest
-        = JsonDocument::hydrate_from_slice(&first_entry.data)?;
+        = JsonDocument::hydrate_from_slice(&first_entry.data)
+            .map_err(|error| remote_manifest_parse_error(locator, "package archive", "package.json", error))?;
 
     let resolution
         = Resolution::from_remote_manifest(locator.clone(), manifest);

packages/zpm/src/lockfile.rs

@@ -249,13 +249,16 @@ struct LegacyBerryLockfilePayload {
     entries: TolerantMap<MultiKey<Descriptor>, LegacyBerryLockfileEntry>,
 }
 
-pub fn from_legacy_berry_lockfile(data: &str) -> Result<Lockfile, Error> {
+pub fn from_legacy_berry_lockfile(data: &str, lockfile_path: &Path) -> Result<Lockfile, Error> {
     if data.starts_with("# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.") {
         return Err(Error::LockfileV1Error);
     }
 
     let payload: LegacyBerryLockfilePayload = serde_yaml::from_str(data)
-        .map_err(|err| Error::LegacyLockfileParseError(Arc::new(err)))?;
+        .map_err(|error| Error::LegacyLockfileParseError {
+            path: lockfile_path.clone(),
+            reason: error.to_string().into(),
+        })?;
 
     let mut lockfile
         = Lockfile::new();
@@ -291,12 +294,12 @@ pub fn from_legacy_berry_lockfile(data: &str) -> Result<Lockfile, Error> {
                         url: params.url.clone(),
                     }.into());
 
-                lockfile.entries.insert(entry.resolution.clone(), LockfileEntry {
+                lockfile.entries.insert(aliased_locator.clone(), LockfileEntry {
                     checksum: None,
-                    resolution: Resolution::new_empty(aliased_locator, Default::default()),
+                    resolution: Resolution::new_empty(aliased_locator.clone(), Default::default()),
                 });
 
-                lockfile.resolutions.insert(descriptor, entry.resolution.clone());
+                lockfile.resolutions.insert(descriptor, aliased_locator);
             }
         }
     }

packages/zpm/src/manifest/helpers.rs

@@ -1,5 +1,3 @@
-use std::sync::Arc;
-
 use zpm_parsers::JsonDocument;
 use zpm_utils::{IoResultExt, Path};
 
@@ -21,7 +19,18 @@ pub fn read_manifest_with_size(abs_path: &Path, size: u64) -> Result<Manifest, E
         .ok_or_else(|| Error::ManifestNotFound(abs_path.clone()))?;
 
     parse_manifest(&manifest_text)
-        .map_err(|err| Error::ManifestParseError(abs_path.clone(), Arc::new(err)))
+        .map_err(|error| {
+            let reason = if let Error::FileParsingError(parser_error) = &error {
+                parser_error.to_string()
+            } else {
+                error.to_string()
+            };
+
+            Error::ManifestParseError {
+                path: abs_path.clone(),
+                reason: reason.into(),
+            }
+        })
 }
 
 pub fn parse_manifest_from_bytes(bytes: &[u8]) -> Result<Manifest, Error> {