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.

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

Display chained errors #43

Open
madig opened this issue Dec 16, 2021 · 12 comments · May be fixed by #44
Open

Display chained errors #43

madig opened this issue Dec 16, 2021 · 12 comments · May be fixed by #44

Comments

@madig
Copy link
Collaborator

madig commented Dec 16, 2021

I'm trying to make norad cleanly chain errors instead of hoisting the error messages form lower into wrapping error messages. Compare what anyhow does:

Error: Failed to read glyph file from 'C:\...\MutatorSansLightWide.ufo\glyphs\A_.glif'

Caused by:
    0: Failed to read or parse XML file
    1: Expecting </array> found </dict>

vs. ufofmt's

[ERROR] norad read error: C:\...\MutatorSansLightWide.ufo: Failed to read glyph file from 'C:\...\MutatorSansLightWide.ufo\glyphs\A_.glif'
@chrissimpkins
Copy link
Member

Excellent! That is really nice Nikolaus. Let me look into how to get at the data in the inner error messages. Please let me know if you have thoughts about how the approach modifications on the ufofmt side. I would like to support this.

@madig
Copy link
Collaborator Author

madig commented Dec 16, 2021

Hm, I think it is basically about recursively calling source() on the errors until you get None and print the messages of all, but maybe looking at the anyhow source code is more enlightening.

@chrissimpkins
Copy link
Member

Is the example that yielded the error in your OP:

Error: Failed to read glyph file from 'C:\...\MutatorSansLightWide.ufo\glyphs\A_.glif'

Caused by:
    0: Failed to read or parse XML file
    1: Expecting </array> found </dict>

in the norad repo?

@chrissimpkins
Copy link
Member

Note to self: https://docs.rs/anyhow/1.0.51/anyhow/struct.Error.html#display-representations

I don't have a problem with simply adding anyhow error handling in this project. It looks like we get the recursive calls handled with that library.

@madig
Copy link
Collaborator Author

madig commented Dec 22, 2021

I modified the UFO in question to end a <dict> with </array> somewhere, but the error messages in question should be in main.

@madig
Copy link
Collaborator Author

madig commented Dec 22, 2021

https://www.lpalmieri.com/posts/error-handling-rust/ contains an example on how to print error sources:

//! src/routes/subscriptions.rs
// [...]

fn error_chain_fmt(
    e: &impl std::error::Error,
    f: &mut std::fmt::Formatter<'_>,
) -> std::fmt::Result {
    writeln!(f, "{}\n", e)?;
    let mut current = e.source();
    while let Some(cause) = current {
        writeln!(f, "Caused by:\n\t{}", cause)?;
        current = cause.source();
    }
    Ok(())
}

@chrissimpkins
Copy link
Member

Thanks Nikolaus!

Let's see if #44 addresses this. I'll prepare test sources to have a look at the output format from chained norad errors.

@madig
Copy link
Collaborator Author

madig commented Dec 23, 2021

Cool :) it might also be an opportunity to review the error hierarchy, as in, do the errors tell you what you need to know. See https://kazlauskas.me/entries/errors

@chrissimpkins
Copy link
Member

Cool :) it might also be an opportunity to review the error hierarchy, as in, do the errors tell you what you need to know. See https://kazlauskas.me/entries/errors

sg!

@chrissimpkins
Copy link
Member

chrissimpkins commented Jan 28, 2022

Hmm. It looks like there was a big refactor in error types and now I can't build the branch in #44 based on norad master branch. norad::Error appears to be gone.

@chrissimpkins
Copy link
Member

chrissimpkins commented Jan 28, 2022

Sorted out in e4948b07ce088ad957c35fad9f125a402704094b

@chrissimpkins
Copy link
Member

chrissimpkins commented Jan 28, 2022

Looking good!

Here's what I see in the #44 branch if I mangle a cap A glif file:

[ERROR] norad read error: /Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo
failed to load layer 'foreground' from '/Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo/glyphs'

Caused by:
	failed to load glyph 'A' from '/Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo/glyphs/A_.glif'
Caused by:
	failed to parse glyph data: missing close tag

and when I remove the glyph element that defines the glyph name:

[ERROR] norad read error: /Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo
failed to load layer 'foreground' from '/Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo/glyphs'

Caused by:
	failed to load glyph 'A' from '/Users/chris/Desktop/ufofmt-test/MutatorSansBoldCondensed.ufo/glyphs/A_.glif'
Caused by:
	failed to parse glyph data: wrong first XML element in glif file

This looks great! Nicely done @madig and @cmyr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants