Skip to content

Commit

Permalink
bug: Add diagnostic information on font load error (#235)
Browse files Browse the repository at this point in the history
* bug: Add diagnostic information on font load error

I ran into a bug a while back that prevent me from using the plotters
library for months. I did finally figure out what was causing it: a file
in my font library documented in issue#231. Removing the file fixed my
issue, but the opaque problem is sure to ensnare others.

This PR adds a `LoadError(Cow<'static, str>)` variant to
`SelectionError` enum. It removes `Copy` because `Cow` isn't copyable.
And now when I run the test suite with the bad file in place, it reports
this error, which is far and away more helpful than before.

```sh
> RUST_BACKTRACE=1 cargo test --all
...
running 5 tests
test loaders::core_text::test::test_core_text_to_css_font_stretch ... ok
test loaders::core_text::test::test_core_text_to_css_font_weight ... ok
test sources::core_text::test::test_css_to_core_text_font_stretch ... ok
test sources::core_text::test::test_css_to_core_text_font_weight ... ok
test loaders::core_text::test::test_from_core_graphics_font ... FAILED

failures:

---- loaders::core_text::test::test_from_core_graphics_font stdout ----
thread 'loaders::core_text::test::test_from_core_graphics_font' panicked at src/loaders/core_text.rs:940:14:
called `Result::unwrap()` on an `Err` value: LoadError("Parse error on path \"/Users/shane/Library/Fonts/Arial\"")
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1649:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/result.rs:1073:23
   4: font_kit::loaders::core_text::test::test_from_core_graphics_font
             at ./src/loaders/core_text.rs:938:21
   5: font_kit::loaders::core_text::test::test_from_core_graphics_font::{{closure}}
             at ./src/loaders/core_text.rs:937:38
   6: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

failures:
    loaders::core_text::test::test_from_core_graphics_font

test result: FAILED. 4 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

error: test failed, to rerun pass `--lib`
```

A variant that used `String` would suffice for this case. I tend to use
`Cow<'static, str>` on errors since many times they can be `&'static
str`.

* refactor: Prefer CannotAccessSource to new variant

* chore: Run cargo fmt.
  • Loading branch information
shanecelis authored Mar 9, 2024
1 parent 66d53bc commit 428a2e5
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
10 changes: 7 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

//! Various types of errors that `font-kit` can return.
use std::borrow::Cow;
use std::convert::From;
use std::error::Error;
use std::io;
Expand Down Expand Up @@ -91,18 +92,21 @@ impl From<winapi::um::winnt::HRESULT> for GlyphLoadingError {
}

/// Reasons why a source might fail to look up a font or fonts.
#[derive(Clone, Copy, PartialEq, Debug)]
#[derive(Clone, PartialEq, Debug)]
pub enum SelectionError {
/// No font matching the given query was found.
NotFound,
/// The source was inaccessible because of an I/O or similar error.
CannotAccessSource,
CannotAccessSource {
/// Additional diagnostic information may include file name
reason: Option<Cow<'static, str>>,
},
}

impl Error for SelectionError {}

impl_display! { SelectionError, {
NotFound => "no font found",
CannotAccessSource => "failed to access source",
CannotAccessSource { reason: ref maybe_cow } => maybe_cow.as_deref().unwrap_or("failed to access source")
}
}
8 changes: 5 additions & 3 deletions src/sources/core_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,13 +230,13 @@ fn create_handle_from_descriptor(descriptor: &CTFontDescriptor) -> Result<Handle
let mut file = if let Ok(file) = File::open(&font_path) {
file
} else {
return Err(SelectionError::CannotAccessSource);
return Err(SelectionError::CannotAccessSource { reason: None });
};

let font_data = if let Ok(font_data) = utils::slurp_file(&mut file) {
Arc::new(font_data)
} else {
return Err(SelectionError::CannotAccessSource);
return Err(SelectionError::CannotAccessSource { reason: None });
};

match Font::analyze_bytes(Arc::clone(&font_data)) {
Expand All @@ -255,7 +255,9 @@ fn create_handle_from_descriptor(descriptor: &CTFontDescriptor) -> Result<Handle
Err(SelectionError::NotFound)
}
Ok(FileType::Single) => Ok(Handle::from_memory(font_data, 0)),
Err(_) => Err(SelectionError::CannotAccessSource),
Err(e) => Err(SelectionError::CannotAccessSource {
reason: Some(format!("{:?} error on path {:?}", e, font_path).into()),
}),
}
}

Expand Down

0 comments on commit 428a2e5

Please sign in to comment.