improve error reporting of JSON parsing (vercel/turborepo#3676)

also make source context generation more reusable This adds a source context snippet of the JSON that failed parsing and the json path to the error. ``` An error occurred while generating the chunk item [project]/crates/turbopack-tests/tests/snapshot/imports/json/input/invalid.json (json) at Execution of module_factory failed at Execution of JsonChunkItem::content failed at Unable to make a module from invalid JSON: expected `,` or `}` at line 3 column 26 at nested.? 1 | { 2 | "nested": { | v 3 | "this-is": "invalid" // lint-staged will remove trailing commas, so here's a comment | ^ 4 | } 5 | } ```
vercel · Feb 8, 2023 · 2695344 · 2695344
1 parent 5994495
commit 2695344
Show file tree

Hide file tree

Showing 24 changed files with 562 additions and 156 deletions.
diff --git a/crates/next-core/src/next_config.rs b/crates/next-core/src/next_config.rs
@@ -8,6 +8,7 @@ use turbo_tasks::{
     Value,
 };
 use turbo_tasks_env::EnvMapVc;
+use turbo_tasks_fs::json::parse_json_rope_with_source_context;
 use turbopack::{
     evaluate_context::node_evaluate_asset_context,
     module_options::{WebpackLoadersOptions, WebpackLoadersOptionsVc},
@@ -485,7 +486,7 @@ pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<N
     .await?;
     match &*config_value {
         JavaScriptValue::Value(val) => {
-            let next_config: NextConfig = serde_json::from_reader(val.read())?;
+            let next_config: NextConfig = parse_json_rope_with_source_context(val)?;
             let next_config = next_config.cell();
 
             Ok(next_config)

diff --git a/crates/next-core/src/next_font_google/mod.rs b/crates/next-core/src/next_font_google/mod.rs
@@ -4,7 +4,7 @@ use indoc::formatdoc;
 use once_cell::sync::Lazy;
 use turbo_tasks::primitives::{OptionStringVc, OptionU16Vc, StringVc, U32Vc};
 use turbo_tasks_fetch::fetch;
-use turbo_tasks_fs::{FileContent, FileSystemPathVc};
+use turbo_tasks_fs::{json::parse_json_with_source_context, FileContent, FileSystemPathVc};
 use turbo_tasks_hash::hash_xxh3_hash64;
 use turbopack_core::{
     issue::IssueSeverity,
@@ -34,8 +34,9 @@ pub(crate) mod request;
 mod util;
 
 pub const GOOGLE_FONTS_STYLESHEET_URL: &str = "https://fonts.googleapis.com/css2";
-static FONT_DATA: Lazy<FontData> =
-    Lazy::new(|| serde_json::from_str(include_str!("__generated__/font-data.json")).unwrap());
+static FONT_DATA: Lazy<FontData> = Lazy::new(|| {
+    parse_json_with_source_context(include_str!("__generated__/font-data.json")).unwrap()
+});
 
 type FontData = IndexMap<String, FontDataEntry>;
 
@@ -371,6 +372,6 @@ async fn font_options_from_query_map(query: QueryMapVc) -> Result<NextFontGoogle
             bail!("Expected one entry");
         };
 
-    self::options::options_from_request(&serde_json::from_str(json)?, &FONT_DATA)
+    self::options::options_from_request(&parse_json_with_source_context(json)?, &FONT_DATA)
         .map(NextFontGoogleOptionsVc::cell)
 }
diff --git a/crates/next-core/src/next_font_google/options.rs b/crates/next-core/src/next_font_google/options.rs
@@ -179,13 +179,14 @@ pub fn options_from_request(
 mod tests {
     use anyhow::Result;
     use indexmap::{indexset, IndexMap};
+    use turbo_tasks_fs::json::parse_json_with_source_context;
 
     use super::{options_from_request, FontDataEntry, NextFontGoogleOptions};
     use crate::next_font_google::{options::FontWeights, request::NextFontRequest};
 
     #[test]
     fn test_errors_on_unknown_font() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -196,7 +197,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "Inter",
@@ -218,7 +219,7 @@ mod tests {
 
     #[test]
     fn test_default_values_when_no_arguments() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -229,7 +230,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -261,7 +262,7 @@ mod tests {
 
     #[test]
     fn test_errors_when_no_weights_chosen_no_variable() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -272,7 +273,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -297,7 +298,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_unnecessary_weights() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -308,7 +309,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -336,7 +337,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_unvavailable_weights() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -347,7 +348,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -374,7 +375,7 @@ mod tests {
 
     #[test]
     fn test_defaults_to_only_style_when_one_available() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -385,7 +386,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -406,7 +407,7 @@ mod tests {
 
     #[test]
     fn test_defaults_to_normal_style_when_multiple() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -417,7 +418,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -438,7 +439,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_unknown_styles() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -449,7 +450,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -478,7 +479,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_unknown_display() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -489,7 +490,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",
@@ -519,7 +520,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_axes_without_variable() -> Result<()> {
-        let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
+        let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -530,7 +531,7 @@ mod tests {
   "#,
         )?;
 
-        let request: NextFontRequest = serde_json::from_str(
+        let request: NextFontRequest = parse_json_with_source_context(
             r#"
             {
                 "import": "ABeeZee",

diff --git a/crates/next-core/src/next_font_google/util.rs b/crates/next-core/src/next_font_google/util.rs
@@ -214,6 +214,7 @@ pub(crate) fn get_stylesheet_url(
 mod tests {
     use anyhow::Result;
     use indexmap::indexset;
+    use turbo_tasks_fs::json::parse_json_with_source_context;
 
     use super::get_font_axes;
     use crate::next_font_google::{
@@ -224,7 +225,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_unknown_font() -> Result<()> {
-        let data: FontData = serde_json::from_str(
+        let data: FontData = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -252,7 +253,7 @@ mod tests {
 
     #[test]
     fn test_errors_on_missing_axes() -> Result<()> {
-        let data: FontData = serde_json::from_str(
+        let data: FontData = parse_json_with_source_context(
             r#"
             {
                 "ABeeZee": {
@@ -280,7 +281,7 @@ mod tests {
 
     #[test]
     fn test_selecting_axes() -> Result<()> {
-        let data: FontData = serde_json::from_str(
+        let data: FontData = parse_json_with_source_context(
             r#"
             {
                 "Inter": {
@@ -327,7 +328,7 @@ mod tests {
 
     #[test]
     fn test_no_wght_axis() -> Result<()> {
-        let data: FontData = serde_json::from_str(
+        let data: FontData = parse_json_with_source_context(
             r#"
             {
                 "Inter": {
@@ -368,7 +369,7 @@ mod tests {
 
     #[test]
     fn test_no_variable() -> Result<()> {
-        let data: FontData = serde_json::from_str(
+        let data: FontData = parse_json_with_source_context(
             r#"
             {
                 "Hind": {

diff --git a/crates/next-core/src/router.rs b/crates/next-core/src/router.rs
@@ -4,7 +4,7 @@ use turbo_tasks::{
     primitives::{JsonValueVc, StringsVc},
     Value,
 };
-use turbo_tasks_fs::{to_sys_path, FileSystemPathVc};
+use turbo_tasks_fs::{json::parse_json_rope_with_source_context, to_sys_path, FileSystemPathVc};
 use turbopack::evaluate_context::node_evaluate_asset_context;
 use turbopack_core::{
     asset::AssetVc,
@@ -187,7 +187,7 @@ pub async fn route(
 
     match &*result {
         JavaScriptValue::Value(val) => {
-            let result: RouterIncomingMessage = serde_json::from_reader(val.read())?;
+            let result: RouterIncomingMessage = parse_json_rope_with_source_context(val)?;
             Ok(RouterResult::from(result).cell())
         }
         JavaScriptValue::Error => Ok(RouterResult::Error.cell()),

diff --git a/crates/turbo-tasks-fs/Cargo.toml b/crates/turbo-tasks-fs/Cargo.toml
@@ -27,6 +27,7 @@ notify = "4.0.17"
 parking_lot = "0.12.1"
 serde = { version = "1.0.136", features = ["rc"] }
 serde_json = "1.0.85"
+serde_path_to_error = "0.1.9"
 tokio = "1.21.2"
 turbo-tasks = { path = "../turbo-tasks" }
 turbo-tasks-hash = { path = "../turbo-tasks-hash" }