Skip to content

Commit

Permalink
Add context to more errors
Browse files Browse the repository at this point in the history
Specifically paths and filenames.
  • Loading branch information
Ortham committed Oct 17, 2023
1 parent 12609a9 commit 139507a
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 149 deletions.
12 changes: 6 additions & 6 deletions ffi/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,19 @@ fn map_error(err: &Error) -> c_uint {
use Error::*;
match *err {
InvalidPath(_) => LIBLO_ERROR_FILE_NOT_FOUND,
IoError(ref x) => map_io_error(x),
IoError(_, ref x) => map_io_error(x),
NoFilename => LIBLO_ERROR_FILE_PARSE_FAIL,
SystemTimeError(_) => LIBLO_ERROR_TIMESTAMP_WRITE_FAIL,
NotUtf8(_) => LIBLO_ERROR_FILE_NOT_UTF8,
DecodeError(_) => LIBLO_ERROR_TEXT_DECODE_FAIL,
EncodeError(_) => LIBLO_ERROR_TEXT_ENCODE_FAIL,
PluginParsingError => LIBLO_ERROR_FILE_PARSE_FAIL,
PluginParsingError(_) => LIBLO_ERROR_FILE_PARSE_FAIL,
PluginNotFound(_) => LIBLO_ERROR_INVALID_ARGS,
TooManyActivePlugins => LIBLO_ERROR_INVALID_ARGS,
InvalidRegex => LIBLO_ERROR_INTERNAL_LOGIC_ERROR,
DuplicatePlugin => LIBLO_ERROR_INVALID_ARGS,
NonMasterBeforeMaster => LIBLO_ERROR_INVALID_ARGS,
GameMasterMustLoadFirst => LIBLO_ERROR_INVALID_ARGS,
DuplicatePlugin(_) => LIBLO_ERROR_INVALID_ARGS,
NonMasterBeforeMaster { .. } => LIBLO_ERROR_INVALID_ARGS,
GameMasterMustLoadFirst(_) => LIBLO_ERROR_INVALID_ARGS,
InvalidEarlyLoadingPluginPosition { .. } => LIBLO_ERROR_INVALID_ARGS,
InvalidPlugin(_) => LIBLO_ERROR_INVALID_ARGS,
ImplicitlyActivePlugin(_) => LIBLO_ERROR_INVALID_ARGS,
Expand All @@ -75,7 +75,7 @@ fn map_error(err: &Error) -> c_uint {
UnrepresentedHoist { .. } => LIBLO_ERROR_INVALID_ARGS,
InstalledPlugin(_) => LIBLO_ERROR_INVALID_ARGS,
IniParsingError { .. } => LIBLO_ERROR_FILE_PARSE_FAIL,
VdfParsingError(_) => LIBLO_ERROR_FILE_PARSE_FAIL,
VdfParsingError(_, _) => LIBLO_ERROR_FILE_PARSE_FAIL,
SystemError(_, _) => LIBLO_ERROR_SYSTEM_ERROR,
}
}
Expand Down
92 changes: 24 additions & 68 deletions src/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,22 @@ impl GameId {
#[derive(Debug)]
pub enum Error {
InvalidPath(PathBuf),
IoError(io::Error),
IoError(PathBuf, io::Error),
NoFilename,
SystemTimeError(time::SystemTimeError),
NotUtf8(Vec<u8>),
DecodeError(Cow<'static, str>),
EncodeError(Cow<'static, str>),
PluginParsingError,
PluginParsingError(PathBuf),
PluginNotFound(String),
TooManyActivePlugins,
InvalidRegex,
DuplicatePlugin,
NonMasterBeforeMaster,
GameMasterMustLoadFirst,
DuplicatePlugin(String),
NonMasterBeforeMaster {
master: String,
non_master: String,
},
GameMasterMustLoadFirst(String),
InvalidEarlyLoadingPluginPosition {
name: String,
pos: usize,
Expand All @@ -105,20 +108,15 @@ pub enum Error {
},
InstalledPlugin(String),
IniParsingError {
path: PathBuf,
line: usize,
column: usize,
message: String,
},
VdfParsingError(String),
VdfParsingError(PathBuf, String),
SystemError(i32, OsString),
}

impl From<io::Error> for Error {
fn from(error: io::Error) -> Self {
Error::IoError(error)
}
}

impl From<time::SystemTimeError> for Error {
fn from(error: time::SystemTimeError) -> Self {
Error::SystemTimeError(error)
Expand All @@ -137,49 +135,6 @@ impl From<FromUtf8Error> for Error {
}
}

impl From<esplugin::Error> for Error {
fn from(error: esplugin::Error) -> Self {
match error {
esplugin::Error::IoError(x) => Error::IoError(x),
esplugin::Error::NoFilename => Error::NoFilename,
esplugin::Error::ParsingIncomplete | esplugin::Error::ParsingError(_, _) => {
Error::PluginParsingError
}
esplugin::Error::DecodeError => Error::DecodeError("invalid sequence".into()),
}
}
}

impl From<ini::Error> for Error {
fn from(error: ini::Error) -> Self {
match error {
ini::Error::Io(x) => Error::IoError(x),
ini::Error::Parse(x) => Error::from(x),
}
}
}

impl From<ini::ParseError> for Error {
fn from(error: ini::ParseError) -> Self {
Error::IniParsingError {
line: error.line,
column: error.col,
message: error.msg,
}
}
}

impl From<keyvalues_parser::error::Error> for Error {
fn from(error: keyvalues_parser::error::Error) -> Self {
match error {
keyvalues_parser::error::Error::ParseError(e) => Error::VdfParsingError(e.to_string()),
keyvalues_parser::error::Error::InvalidTokenStream(c) => {
Error::VdfParsingError(c.to_string())
}
}
}
}

#[cfg(windows)]
impl From<windows::core::Error> for Error {
fn from(error: windows::core::Error) -> Self {
Expand All @@ -191,14 +146,14 @@ impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Error::InvalidPath(ref x) => write!(f, "The path \"{:?}\" is invalid", x),
Error::IoError(ref x) => x.fmt(f),
Error::IoError(ref p, ref x) => write!(f, "I/O error involving the path \"{:?}\": {}", p, x),
Error::NoFilename => write!(f, "The plugin path has no filename part"),
Error::SystemTimeError(ref x) => x.fmt(f),
Error::NotUtf8(ref x) => write!(f, "Expected a UTF-8 string, got bytes {:?}", x),
Error::DecodeError(_) => write!(f, "Text could not be decoded from Windows-1252"),
Error::EncodeError(_) => write!(f, "Text could not be encoded in Windows-1252"),
Error::PluginParsingError => {
write!(f, "An error was encountered while parsing a plugin")
Error::PluginParsingError(ref p) => {
write!(f, "An error was encountered while parsing the plugin at \"{:?}\"", p)
}
Error::PluginNotFound(ref x) => {
write!(f, "The plugin \"{}\" is not in the load order", x)
Expand All @@ -208,18 +163,18 @@ impl fmt::Display for Error {
f,
"Internal error: regex is invalid"
),
Error::DuplicatePlugin => write!(f, "The given plugin list contains duplicates"),
Error::NonMasterBeforeMaster => write!(
Error::DuplicatePlugin(ref n) => write!(f, "The given plugin list contains more than one instance of \"{}\"", n),
Error::NonMasterBeforeMaster{ref master, ref non_master} => write!(
f,
"Attempted to load a non-master plugin before a master plugin"
"Attempted to load the non-master plugin \"{}\" before the master plugin \"{}\"", non_master, master
),
Error::GameMasterMustLoadFirst => write!(
Error::GameMasterMustLoadFirst(ref n) => write!(
f,
"The game's master file must load first"
"The game's master file \"{}\" must load first", n
),
Error::InvalidEarlyLoadingPluginPosition{ ref name, pos, expected_pos } => write!(
f,
"Attempted to load the early-loading plugin {} at position {}, its expected position is {}", name, pos, expected_pos
"Attempted to load the early-loading plugin \"{}\" at position {}, its expected position is {}", name, pos, expected_pos
),
Error::InvalidPlugin(ref x) => write!(f, "The plugin file \"{}\" is invalid", x),
Error::ImplicitlyActivePlugin(ref x) => write!(
Expand All @@ -244,15 +199,16 @@ impl fmt::Display for Error {
plugin
),
Error::IniParsingError {
ref path,
line,
column,
ref message,
} => write!(
f,
"Failed to parse ini file, error at line {}, column {}: {}",
line, column, message
"Failed to parse ini file at \"{:?}\", error at line {}, column {}: {}",
path, line, column, message
),
Error::VdfParsingError(ref message) => write!(f, "Failed to parse VDF file: {}", message),
Error::VdfParsingError(ref path, ref message) => write!(f, "Failed to parse VDF file at \"{:?}\": {}", path, message),
Error::SystemError(code, ref message) => write!(f, "Error returned by the operating system, code {}: {:?}", code, message),
}
}
Expand All @@ -261,7 +217,7 @@ impl fmt::Display for Error {
impl error::Error for Error {
fn cause(&self) -> Option<&dyn error::Error> {
match *self {
Error::IoError(ref x) => Some(x),
Error::IoError(_, ref x) => Some(x),
Error::SystemTimeError(ref x) => Some(x),
_ => None,
}
Expand Down
6 changes: 4 additions & 2 deletions src/game_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ fn find_nam_plugins(plugins_path: &Path) -> Result<Vec<String>, Error> {
}

let dir_iter = plugins_path
.read_dir()?
.read_dir()
.map_err(|e| Error::IoError(plugins_path.to_path_buf(), e))?
.filter_map(Result::ok)
.filter(|e| e.file_type().map(|f| f.is_file()).unwrap_or(false))
.filter(|e| {
Expand Down Expand Up @@ -554,7 +555,8 @@ fn early_loading_plugins(

if let Some(file_path) = ccc_file_path(game_id, game_path) {
if file_path.exists() {
let reader = BufReader::new(File::open(file_path)?);
let reader =
BufReader::new(File::open(&file_path).map_err(|e| Error::IoError(file_path, e))?);

let lines = reader
.lines()
Expand Down
4 changes: 2 additions & 2 deletions src/ghostable_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl GhostablePath for Path {
Ok(self.to_path_buf())
} else {
let new_path = self.as_ghosted_path()?;
rename(self, &new_path)?;
rename(self, &new_path).map_err(|e| Error::IoError(self.to_path_buf(), e))?;
Ok(new_path)
}
}
Expand All @@ -51,7 +51,7 @@ impl GhostablePath for Path {
Ok(self.to_path_buf())
} else {
let new_path = self.as_unghosted_path()?;
rename(self, &new_path)?;
rename(self, &new_path).map_err(|e| Error::IoError(self.to_path_buf(), e))?;
Ok(new_path)
}
}
Expand Down
23 changes: 19 additions & 4 deletions src/ini.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,18 @@ type TestFiles = [Option<String>; 10];
fn read_ini(ini_path: &Path) -> Result<ini::Ini, Error> {
// Read ini as Windows-1252 bytes and then convert to UTF-8 before parsing,
// as the ini crate expects the content to be valid UTF-8.
let contents = std::fs::read(ini_path)?;
let contents =
std::fs::read(ini_path).map_err(|e| Error::IoError(ini_path.to_path_buf(), e))?;

// My Games is used if bUseMyGamesDirectory is not present or set to 1.
let contents = WINDOWS_1252.decode_without_bom_handling(&contents).0;

ini::Ini::load_from_str(&contents).map_err(Error::from)
ini::Ini::load_from_str(&contents).map_err(|e| Error::IniParsingError {
path: ini_path.to_path_buf(),
line: e.line,
column: e.col,
message: e.msg,
})
}

pub fn use_my_games_directory(ini_path: &Path) -> Result<bool, Error> {
Expand Down Expand Up @@ -189,9 +195,18 @@ fn starfield_language(game_path: &Path) -> Result<&'static str, Error> {
}

fn read_steam_language_config(appmanifest_acf_path: &Path) -> Result<Option<String>, Error> {
let content = std::fs::read_to_string(appmanifest_acf_path)?;
let content = std::fs::read_to_string(appmanifest_acf_path)
.map_err(|e| Error::IoError(appmanifest_acf_path.to_path_buf(), e))?;

let language = keyvalues_parser::Vdf::parse(&content)
.map_err(|e| {
let detail = match e {
keyvalues_parser::error::Error::ParseError(e) => e.to_string(),
keyvalues_parser::error::Error::InvalidTokenStream(c) => c.to_string(),
};

let language = keyvalues_parser::Vdf::parse(&content)?
Error::VdfParsingError(appmanifest_acf_path.to_path_buf(), detail)
})?
.value
.get_obj()
.and_then(|o| o.get("UserConfig"))
Expand Down
32 changes: 18 additions & 14 deletions src/load_order/asterisk_based.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ impl WritableLoadOrder for AsteriskBasedLoadOrder {
}

fn save(&mut self) -> Result<(), Error> {
create_parent_dirs(self.game_settings().active_plugins_file())?;
let path = self.game_settings().active_plugins_file();
create_parent_dirs(path)?;

let file = File::create(self.game_settings().active_plugins_file())?;
let file = File::create(path).map_err(|e| Error::IoError(path.clone(), e))?;
let mut writer = BufWriter::new(file);
for plugin in self.plugins() {
if self.game_settings().loads_early(plugin.name()) {
Expand All @@ -135,10 +136,12 @@ impl WritableLoadOrder for AsteriskBasedLoadOrder {
}

if plugin.is_active() {
write!(writer, "*")?;
write!(writer, "*").map_err(|e| Error::IoError(path.clone(), e))?;
}
writer.write_all(&strict_encode(plugin.name())?)?;
writeln!(writer)?;
writer
.write_all(&strict_encode(plugin.name())?)
.map_err(|e| Error::IoError(path.clone(), e))?;
writeln!(writer).map_err(|e| Error::IoError(path.clone(), e))?;
}

if self.ignore_active_plugins_file() {
Expand All @@ -161,12 +164,14 @@ impl WritableLoadOrder for AsteriskBasedLoadOrder {
}

fn set_load_order(&mut self, plugin_names: &[&str]) -> Result<(), Error> {
let game_master_file = self.game_settings().master_file();

let is_game_master_first = plugin_names
.first()
.map(|name| eq(*name, self.game_settings().master_file()))
.map(|name| eq(*name, game_master_file))
.unwrap_or(false);
if !is_game_master_first {
return Err(Error::GameMasterMustLoadFirst);
return Err(Error::GameMasterMustLoadFirst(game_master_file.to_string()));
}

// Check that all early loading plugins that are present load in
Expand Down Expand Up @@ -197,14 +202,13 @@ impl WritableLoadOrder for AsteriskBasedLoadOrder {
}

fn set_plugin_index(&mut self, plugin_name: &str, position: usize) -> Result<usize, Error> {
if position != 0
&& !self.plugins().is_empty()
&& eq(plugin_name, self.game_settings().master_file())
{
return Err(Error::GameMasterMustLoadFirst);
let game_master_file = self.game_settings().master_file();

if position != 0 && !self.plugins().is_empty() && eq(plugin_name, game_master_file) {
return Err(Error::GameMasterMustLoadFirst(game_master_file.to_string()));
}
if position == 0 && !eq(plugin_name, self.game_settings().master_file()) {
return Err(Error::GameMasterMustLoadFirst);
if position == 0 && !eq(plugin_name, game_master_file) {
return Err(Error::GameMasterMustLoadFirst(game_master_file.to_string()));
}

self.move_or_insert_plugin_with_index(plugin_name, position)
Expand Down
Loading

0 comments on commit 139507a

Please sign in to comment.