Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve error reporting of JSON parsing #3676

Merged
merged 3 commits into from
Feb 8, 2023
Merged

improve error reporting of JSON parsing #3676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
410ad6e
improve error reporting of JSON parsing
sokra Feb 7, 2023
8568fd6
clippy
sokra Feb 7, 2023
74b823c
fix off by one
sokra Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/next-core/src/next_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions crates/next-core/src/next_font_google/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>;

Expand Down Expand Up @@ -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)
}
41 changes: 21 additions & 20 deletions crates/next-core/src/next_font_google/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -196,7 +197,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "Inter",
Expand All @@ -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": {
Expand All @@ -229,7 +230,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand Down Expand Up @@ -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": {
Expand All @@ -272,7 +273,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand All @@ -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": {
Expand All @@ -308,7 +309,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand Down Expand Up @@ -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": {
Expand All @@ -347,7 +348,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand All @@ -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": {
Expand All @@ -385,7 +386,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand All @@ -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": {
Expand All @@ -417,7 +418,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand All @@ -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": {
Expand All @@ -449,7 +450,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand Down Expand Up @@ -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": {
Expand All @@ -489,7 +490,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand Down Expand Up @@ -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": {
Expand All @@ -530,7 +531,7 @@ mod tests {
"#,
)?;

let request: NextFontRequest = serde_json::from_str(
let request: NextFontRequest = parse_json_with_source_context(
r#"
{
"import": "ABeeZee",
Expand Down
11 changes: 6 additions & 5 deletions crates/next-core/src/next_font_google/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions crates/next-core/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()),
Expand Down
1 change: 1 addition & 0 deletions crates/turbo-tasks-fs/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Loading