Skip to content

Commit 0adae40

Browse files
sokrasokra
authored
improve error reporting of JSON parsing (#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 | } ```
1 parent de8a3f2 commit 0adae40

File tree

25 files changed

+563
-156
lines changed

25 files changed

+563
-156
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/next-core/src/next_config.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use turbo_tasks::{
88
Value,
99
};
1010
use turbo_tasks_env::EnvMapVc;
11+
use turbo_tasks_fs::json::parse_json_rope_with_source_context;
1112
use turbopack::{
1213
evaluate_context::node_evaluate_asset_context,
1314
module_options::{WebpackLoadersOptions, WebpackLoadersOptionsVc},
@@ -485,7 +486,7 @@ pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<N
485486
.await?;
486487
match &*config_value {
487488
JavaScriptValue::Value(val) => {
488-
let next_config: NextConfig = serde_json::from_reader(val.read())?;
489+
let next_config: NextConfig = parse_json_rope_with_source_context(val)?;
489490
let next_config = next_config.cell();
490491

491492
Ok(next_config)

‎crates/next-core/src/next_font_google/mod.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use indoc::formatdoc;
44
use once_cell::sync::Lazy;
55
use turbo_tasks::primitives::{OptionStringVc, OptionU16Vc, StringVc, U32Vc};
66
use turbo_tasks_fetch::fetch;
7-
use turbo_tasks_fs::{FileContent, FileSystemPathVc};
7+
use turbo_tasks_fs::{json::parse_json_with_source_context, FileContent, FileSystemPathVc};
88
use turbo_tasks_hash::hash_xxh3_hash64;
99
use turbopack_core::{
1010
issue::IssueSeverity,
@@ -34,8 +34,9 @@ pub(crate) mod request;
3434
mod util;
3535

3636
pub const GOOGLE_FONTS_STYLESHEET_URL: &str = "https://fonts.googleapis.com/css2";
37-
static FONT_DATA: Lazy<FontData> =
38-
Lazy::new(|| serde_json::from_str(include_str!("__generated__/font-data.json")).unwrap());
37+
static FONT_DATA: Lazy<FontData> = Lazy::new(|| {
38+
parse_json_with_source_context(include_str!("__generated__/font-data.json")).unwrap()
39+
});
3940

4041
type FontData = IndexMap<String, FontDataEntry>;
4142

@@ -371,6 +372,6 @@ async fn font_options_from_query_map(query: QueryMapVc) -> Result<NextFontGoogle
371372
bail!("Expected one entry");
372373
};
373374

374-
self::options::options_from_request(&serde_json::from_str(json)?, &FONT_DATA)
375+
self::options::options_from_request(&parse_json_with_source_context(json)?, &FONT_DATA)
375376
.map(NextFontGoogleOptionsVc::cell)
376377
}

‎crates/next-core/src/next_font_google/options.rs‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,14 @@ pub fn options_from_request(
179179
mod tests {
180180
use anyhow::Result;
181181
use indexmap::{indexset, IndexMap};
182+
use turbo_tasks_fs::json::parse_json_with_source_context;
182183

183184
use super::{options_from_request, FontDataEntry, NextFontGoogleOptions};
184185
use crate::next_font_google::{options::FontWeights, request::NextFontRequest};
185186

186187
#[test]
187188
fn test_errors_on_unknown_font() -> Result<()> {
188-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
189+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
189190
r#"
190191
{
191192
"ABeeZee": {
@@ -196,7 +197,7 @@ mod tests {
196197
"#,
197198
)?;
198199

199-
let request: NextFontRequest = serde_json::from_str(
200+
let request: NextFontRequest = parse_json_with_source_context(
200201
r#"
201202
{
202203
"import": "Inter",
@@ -218,7 +219,7 @@ mod tests {
218219

219220
#[test]
220221
fn test_default_values_when_no_arguments() -> Result<()> {
221-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
222+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
222223
r#"
223224
{
224225
"ABeeZee": {
@@ -229,7 +230,7 @@ mod tests {
229230
"#,
230231
)?;
231232

232-
let request: NextFontRequest = serde_json::from_str(
233+
let request: NextFontRequest = parse_json_with_source_context(
233234
r#"
234235
{
235236
"import": "ABeeZee",
@@ -261,7 +262,7 @@ mod tests {
261262

262263
#[test]
263264
fn test_errors_when_no_weights_chosen_no_variable() -> Result<()> {
264-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
265+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
265266
r#"
266267
{
267268
"ABeeZee": {
@@ -272,7 +273,7 @@ mod tests {
272273
"#,
273274
)?;
274275

275-
let request: NextFontRequest = serde_json::from_str(
276+
let request: NextFontRequest = parse_json_with_source_context(
276277
r#"
277278
{
278279
"import": "ABeeZee",
@@ -297,7 +298,7 @@ mod tests {
297298

298299
#[test]
299300
fn test_errors_on_unnecessary_weights() -> Result<()> {
300-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
301+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
301302
r#"
302303
{
303304
"ABeeZee": {
@@ -308,7 +309,7 @@ mod tests {
308309
"#,
309310
)?;
310311

311-
let request: NextFontRequest = serde_json::from_str(
312+
let request: NextFontRequest = parse_json_with_source_context(
312313
r#"
313314
{
314315
"import": "ABeeZee",
@@ -336,7 +337,7 @@ mod tests {
336337

337338
#[test]
338339
fn test_errors_on_unvavailable_weights() -> Result<()> {
339-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
340+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
340341
r#"
341342
{
342343
"ABeeZee": {
@@ -347,7 +348,7 @@ mod tests {
347348
"#,
348349
)?;
349350

350-
let request: NextFontRequest = serde_json::from_str(
351+
let request: NextFontRequest = parse_json_with_source_context(
351352
r#"
352353
{
353354
"import": "ABeeZee",
@@ -374,7 +375,7 @@ mod tests {
374375

375376
#[test]
376377
fn test_defaults_to_only_style_when_one_available() -> Result<()> {
377-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
378+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
378379
r#"
379380
{
380381
"ABeeZee": {
@@ -385,7 +386,7 @@ mod tests {
385386
"#,
386387
)?;
387388

388-
let request: NextFontRequest = serde_json::from_str(
389+
let request: NextFontRequest = parse_json_with_source_context(
389390
r#"
390391
{
391392
"import": "ABeeZee",
@@ -406,7 +407,7 @@ mod tests {
406407

407408
#[test]
408409
fn test_defaults_to_normal_style_when_multiple() -> Result<()> {
409-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
410+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
410411
r#"
411412
{
412413
"ABeeZee": {
@@ -417,7 +418,7 @@ mod tests {
417418
"#,
418419
)?;
419420

420-
let request: NextFontRequest = serde_json::from_str(
421+
let request: NextFontRequest = parse_json_with_source_context(
421422
r#"
422423
{
423424
"import": "ABeeZee",
@@ -438,7 +439,7 @@ mod tests {
438439

439440
#[test]
440441
fn test_errors_on_unknown_styles() -> Result<()> {
441-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
442+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
442443
r#"
443444
{
444445
"ABeeZee": {
@@ -449,7 +450,7 @@ mod tests {
449450
"#,
450451
)?;
451452

452-
let request: NextFontRequest = serde_json::from_str(
453+
let request: NextFontRequest = parse_json_with_source_context(
453454
r#"
454455
{
455456
"import": "ABeeZee",
@@ -478,7 +479,7 @@ mod tests {
478479

479480
#[test]
480481
fn test_errors_on_unknown_display() -> Result<()> {
481-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
482+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
482483
r#"
483484
{
484485
"ABeeZee": {
@@ -489,7 +490,7 @@ mod tests {
489490
"#,
490491
)?;
491492

492-
let request: NextFontRequest = serde_json::from_str(
493+
let request: NextFontRequest = parse_json_with_source_context(
493494
r#"
494495
{
495496
"import": "ABeeZee",
@@ -519,7 +520,7 @@ mod tests {
519520

520521
#[test]
521522
fn test_errors_on_axes_without_variable() -> Result<()> {
522-
let data: IndexMap<String, FontDataEntry> = serde_json::from_str(
523+
let data: IndexMap<String, FontDataEntry> = parse_json_with_source_context(
523524
r#"
524525
{
525526
"ABeeZee": {
@@ -530,7 +531,7 @@ mod tests {
530531
"#,
531532
)?;
532533

533-
let request: NextFontRequest = serde_json::from_str(
534+
let request: NextFontRequest = parse_json_with_source_context(
534535
r#"
535536
{
536537
"import": "ABeeZee",

‎crates/next-core/src/next_font_google/util.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ pub(crate) fn get_stylesheet_url(
214214
mod tests {
215215
use anyhow::Result;
216216
use indexmap::indexset;
217+
use turbo_tasks_fs::json::parse_json_with_source_context;
217218

218219
use super::get_font_axes;
219220
use crate::next_font_google::{
@@ -224,7 +225,7 @@ mod tests {
224225

225226
#[test]
226227
fn test_errors_on_unknown_font() -> Result<()> {
227-
let data: FontData = serde_json::from_str(
228+
let data: FontData = parse_json_with_source_context(
228229
r#"
229230
{
230231
"ABeeZee": {
@@ -252,7 +253,7 @@ mod tests {
252253

253254
#[test]
254255
fn test_errors_on_missing_axes() -> Result<()> {
255-
let data: FontData = serde_json::from_str(
256+
let data: FontData = parse_json_with_source_context(
256257
r#"
257258
{
258259
"ABeeZee": {
@@ -280,7 +281,7 @@ mod tests {
280281

281282
#[test]
282283
fn test_selecting_axes() -> Result<()> {
283-
let data: FontData = serde_json::from_str(
284+
let data: FontData = parse_json_with_source_context(
284285
r#"
285286
{
286287
"Inter": {
@@ -327,7 +328,7 @@ mod tests {
327328

328329
#[test]
329330
fn test_no_wght_axis() -> Result<()> {
330-
let data: FontData = serde_json::from_str(
331+
let data: FontData = parse_json_with_source_context(
331332
r#"
332333
{
333334
"Inter": {
@@ -368,7 +369,7 @@ mod tests {
368369

369370
#[test]
370371
fn test_no_variable() -> Result<()> {
371-
let data: FontData = serde_json::from_str(
372+
let data: FontData = parse_json_with_source_context(
372373
r#"
373374
{
374375
"Hind": {

‎crates/next-core/src/router.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use turbo_tasks::{
44
primitives::{JsonValueVc, StringsVc},
55
Value,
66
};
7-
use turbo_tasks_fs::{to_sys_path, FileSystemPathVc};
7+
use turbo_tasks_fs::{json::parse_json_rope_with_source_context, to_sys_path, FileSystemPathVc};
88
use turbopack::evaluate_context::node_evaluate_asset_context;
99
use turbopack_core::{
1010
asset::AssetVc,
@@ -187,7 +187,7 @@ pub async fn route(
187187

188188
match &*result {
189189
JavaScriptValue::Value(val) => {
190-
let result: RouterIncomingMessage = serde_json::from_reader(val.read())?;
190+
let result: RouterIncomingMessage = parse_json_rope_with_source_context(val)?;
191191
Ok(RouterResult::from(result).cell())
192192
}
193193
JavaScriptValue::Error => Ok(RouterResult::Error.cell()),

‎crates/turbo-tasks-fs/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ notify = "4.0.17"
2727
parking_lot = "0.12.1"
2828
serde = { version = "1.0.136", features = ["rc"] }
2929
serde_json = "1.0.85"
30+
serde_path_to_error = "0.1.9"
3031
tokio = "1.21.2"
3132
turbo-tasks = { path = "../turbo-tasks" }
3233
turbo-tasks-hash = { path = "../turbo-tasks-hash" }

0 commit comments

Comments
 (0)