Skip to content

Commit

Permalink
use miette for json parsing errors (#1318)
Browse files Browse the repository at this point in the history
* use miette for everything, i think i should use miette just for json decode for now

* only on ls-remote

* fix test

* use .json instead of failing all the time

* fix test

* changeset

* remove unnecessary feature

* remove test for now

* remove unnecessary .to_string()
  • Loading branch information
Schniz authored Nov 12, 2024
1 parent dd6d17d commit 65f7a22
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/nasty-roses-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fnm": patch
---

better error handling for malformed json
111 changes: 109 additions & 2 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ clap_complete = "4.5.2"
anyhow = "1.0.86"
indicatif = { version = "0.17.8", features = ["improved_unicode"] }
flate2 = "1.0.30"
miette = { version = "7.2.0", features = ["fancy"] }

[dev-dependencies]
pretty_assertions = "1.4.0"
Expand Down
3 changes: 3 additions & 0 deletions e2e/shellcode/shells/cmdEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type EnvConfig = {
logLevel: string
corepackEnabled: boolean
resolveEngines: boolean
nodeDistMirror: string
}
export type HasEnv = { env(cfg: Partial<EnvConfig>): ScriptLine }

Expand All @@ -16,13 +17,15 @@ function stringify(envConfig: Partial<EnvConfig> = {}) {
corepackEnabled,
resolveEngines,
executableName = "fnm",
nodeDistMirror,
} = envConfig
return [
`${executableName} env`,
useOnCd && "--use-on-cd",
logLevel && `--log-level=${logLevel}`,
corepackEnabled && "--corepack-enabled",
resolveEngines && `--resolve-engines`,
nodeDistMirror && `--node-dist-mirror=${JSON.stringify(nodeDistMirror)}`,
]
.filter(Boolean)
.join(" ")
Expand Down
4 changes: 2 additions & 2 deletions src/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ pub enum Error {
},
#[error("Can't find version in dotfiles. Please provide a version manually to the command.")]
CantInferVersion,
#[error("Having a hard time listing the remote versions: {}", source)]
CantListRemoteVersions { source: crate::http::Error },
#[error(transparent)]
CantListRemoteVersions { source: remote_node_index::Error },
#[error(
"Can't find a Node version that matches {} in remote",
requested_version
Expand Down
4 changes: 2 additions & 2 deletions src/commands/ls_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ impl super::command::Command for LsRemote {
#[derive(Debug, Error)]
pub enum Error {
#[error(transparent)]
HttpError {
RemoteListing {
#[from]
source: crate::http::Error,
source: remote_node_index::Error,
},
}

Expand Down
13 changes: 8 additions & 5 deletions src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
//! In the future, if we want to migrate to a different HTTP library,
//! we can easily change this facade instead of multiple places in the crate.

use reqwest::blocking::Client;
use reqwest::{blocking::Client, IntoUrl};

pub type Error = reqwest::Error;
#[derive(Debug, thiserror::Error, miette::Diagnostic)]
#[error(transparent)]
#[diagnostic(code("fnm::http::error"))]
pub struct Error(#[from] reqwest::Error);
pub type Response = reqwest::blocking::Response;

pub fn get(url: &str) -> Result<Response, Error> {
Client::new()
pub fn get(url: impl IntoUrl) -> Result<Response, Error> {
Ok(Client::new()
.get(url)
// Some sites require a user agent.
.header("User-Agent", concat!("fnm ", env!("CARGO_PKG_VERSION")))
.send()
.send()?)
}
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ mod version_files;
mod log_level;
mod default_version;
mod directories;
mod pretty_serde;

fn main() {
env_logger::init();
Expand Down
51 changes: 51 additions & 0 deletions src/pretty_serde.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use miette::SourceOffset;

#[derive(Debug, thiserror::Error, miette::Diagnostic)]
#[error("malformed json\n{}", self.report())]
pub struct DecodeError {
cause: serde_json::Error,
#[source_code]
input: String,
#[label("at this position")]
location: SourceOffset,
}

#[derive(Debug, thiserror::Error, miette::Diagnostic)]
#[error("")]
pub struct ClonedError {
message: String,
#[source_code]
input: String,
#[label("{message}")]
location: SourceOffset,
}

impl DecodeError {
pub fn from_serde(input: impl Into<String>, cause: serde_json::Error) -> Self {
let input = input.into();
let location = SourceOffset::from_location(&input, cause.line(), cause.column());
DecodeError {
cause,
input,
location,
}
}

pub fn report(&self) -> String {
use colored::Colorize;
let report = miette::Report::from(ClonedError {
message: self.cause.to_string().italic().to_string(),
input: self.input.clone(),
location: self.location,
});

let mut output = String::new();

for line in format!("{report:?}").lines().skip(1) {
use std::fmt::Write;
writeln!(&mut output, "{line}").unwrap();
}

output.white().to_string()
}
}
24 changes: 20 additions & 4 deletions src/remote_node_index.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::version::Version;
use crate::{pretty_serde::DecodeError, version::Version};
use serde::Deserialize;
use url::Url;

Expand Down Expand Up @@ -66,15 +66,31 @@ pub struct IndexedNodeVersion {
// pub files: Vec<String>,
}

#[derive(Debug, thiserror::Error, miette::Diagnostic)]
pub enum Error {
#[error("can't get remote versions file: {0}")]
#[diagnostic(transparent)]
Http(#[from] crate::http::Error),
#[error("can't decode remote versions file: {0}")]
#[diagnostic(transparent)]
Decode(#[from] DecodeError),
}

/// Prints
///
/// ```rust
/// use crate::remote_node_index::list;
/// ```
pub fn list(base_url: &Url) -> Result<Vec<IndexedNodeVersion>, crate::http::Error> {
pub fn list(base_url: &Url) -> Result<Vec<IndexedNodeVersion>, Error> {
let base_url = base_url.as_str().trim_end_matches('/');
let index_json_url = format!("{base_url}/index.json");
let resp = crate::http::get(&index_json_url)?;
let mut value: Vec<IndexedNodeVersion> = resp.json()?;
let resp = crate::http::get(&index_json_url)
.map_err(crate::http::Error::from)?
.error_for_status()
.map_err(crate::http::Error::from)?;
let text = resp.text().map_err(crate::http::Error::from)?;
let mut value: Vec<IndexedNodeVersion> =
serde_json::from_str(&text[..]).map_err(|cause| DecodeError::from_serde(text, cause))?;
value.sort_by(|a, b| a.version.cmp(&b.version));
Ok(value)
}
Expand Down

0 comments on commit 65f7a22

Please sign in to comment.