Stream: project-error-handling

Topic: Additional context needed for error formatting


view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 18:22):

Hi all! In wgpu and naga, we have this common problem that the enum Error contains a bunch of things that are internal to the crate. Like the handles to objects. Displaying them directly is not very useful (just numbers), and we'd need extra context in order to get the descriptions.
Perhaps, somebody could say if this problem has been solved before? We have a rather verbose solution that I'm not happy with.

view this post on Zulip Jane Lusby (Aug 16 2021 at 18:51):

@Dzmitry Malyshau can you give more details on what you would like to do that is currently difficult / impossible? I'm not entirely understanding the issue

view this post on Zulip Jane Lusby (Aug 16 2021 at 18:52):

like, is it that you need access to external context when reporting errors in order to properly display the extra context stored in the errors or is it that you want to be able to extract these extra fields from the errors when reporting?

view this post on Zulip Jane Lusby (Aug 16 2021 at 18:52):

or something else entirely

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 18:54):

yes, pretty much. For example, Error variants contain byte ranges into some input. In order to cite the input chunks, the impl Display needs to have access to that input in addition to the error itself.

view this post on Zulip Jane Lusby (Aug 16 2021 at 18:59):

aah yea

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:00):

so this is related to a lot of the stuff I've worked on for error handling, between eyre and generic member access, so I can explain how I'd solve this though it's not super helpful because a major piece of this is currently missing in the Error trait still

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:01):

but basically, the way I would want to model this is as context you can extract from an arbitrary error, such as your range of bytes which I assume is essentially a Span in your input

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:02):

then a reporter that has access to the additional context such as the input file itself so that it can highlight the span and the snippet of relevant input or w/e

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:02):

so this RFC is supposed to make it so you can extract arbitrary types from errors

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:03):

so you could do something like if let Some(span) = error.context::<MySpanType>() { ... }

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:05):

then you could use something like eyre to write your own reporting context which would do the job of iterating over the errors through the error trait when reporting, looking for spans, and then displaying them with the additional context as well as including their error messages from their Display impls

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:05):

this assumes that you're type erasing your errors though

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:05):

which may not be a constraint you have

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:05):

I'm not sure

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:06):

if you're reporting a concrete top level error type and you can access your context directly then I'd skip all the generic member access and dyn reporter stuff and write this all by hand

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:06):

since it would be much simpler

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:08):

does that help or am I still misunderstanding?

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 19:51):

(just finishing a meeting, sorry about the delay)

if let Some(span) = error.context::<MySpanType>()

this assumes a single span for the whole error. Our errors may refer to multiple different things (like. "a + b" expression failing would want to refer to the locations where "a" and "b" are defined). So this API doesn't appear to be solving our issue.

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:53):

It's generic so you should be able to represent it however you like. You could extract it as a slice of spans, some other type that contextualizes the spans, an iterator, etc

view this post on Zulip Jane Lusby (Aug 16 2021 at 19:56):

The API is designed to be as flexible as possible and will support owned values, references, DSTs, and even types that contain references, such as a mutable closure which happens to be relevant to executors where you want to be able to pass out a callback that can modify the executors state

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:01):

if you're reporting a concrete top level error type and you can access your context directly then I'd skip all the generic member access and dyn reporter stuff and write this all by hand

we have a chain of errors. Supposedly, we have the context, and we can just implement something like fn display(Error, Context, Formatter).

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:02):

this would mean we can't benefit from thiserror crate any more (but that's ok)

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:02):

I think I'll need to read your RFC carefully. Thank you for pointing to it!

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:02):

Dzmitry Malyshau said:

this would mean we can't benefit from thiserror crate any more (but that's ok)

I'm confused, how would this interact negatively with thiserror?

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:03):

with thiserror, we already put stuff like #[error("blabla")] on each variant. So this will need to go away

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:04):

i.e. if we are manually implementing it, then we no longer use thiserror

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:04):

I think there's a disconnect. Are we talking about reporting errors to users as in when you control the entire application

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:04):

or is this designing an error API for a library to be consumed by your users?

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:05):

our case is more of a former

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:05):

because I was assuming the former case where the fn display would just be the last step when printing a chain of errors, and the errors themselves could still have display impls created with derives

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:06):

you are saying fn display would be the last step. What would be the first steps?

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:06):

so i would still expect you to use thiserror for the error messages themselves, but I would expect those error messages to be pieces of the final error report you create at the end that does the last step of contextualizing the spans

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:07):

so the first step would be defining the error types and messages, and then constructing them. The error messages would be simple things like "invalid expression" and then you'd have a report that when it hits that invalid expression error in the chain knows to go look for the spans and add an additional bit of information after the error message itself sort of like rustc errors

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:07):

and that reporting happens in one function at the end when you're handling a concrete error at runtime

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:08):

and that takes the error (chain of errors), the context with the file content and what not, and the Formatter that we're writing the full report back out too

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:08):

and doing all of that is the job of eyre

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:08):

but eyre works on dyn Errors

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:08):

so it needs generic member access to get the context out of an error type after it has been type erased

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:08):

though if you know the types you can always just downcast

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:09):

so eyre may still be a good call here

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:10):

I don't think I understand very well, yet. You are saying we can look at the error, and report additionally what stuff is in the error. Like if the error is "Unable to add [1] and [2]", we'd report on separate lines something like: "[1]: 'let x = 2'", and "[2]: 'let y = 2*x;'". Right?

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:12):

yea, basically, though you have to add an extra interface to expose this additional information that needs to be reported after the main error message

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:12):

https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.Diagnostic.html

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:13):

I think this is how rustc does it for their error messages

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:13):

ok, and then what do we do with the chain? are you suggesting to try downcasting the errors in the chain?

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:14):

I don't know enough about your specific constraints to know if that would be applicable, but if you can then that's the best way to get additional context out of errors right now if you want to report more than just an error message at each step in a chain of errors

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:14):

there are some other patterns you can use

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:15):

like a context wrapper type where you can go ErrorContext<MyError> and the ErrorContext stores all the extra information, then you transform the inner error type with a map function, though that doesn't map well to having some context at each step in the chain

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:15):

this sounds close to what we currently have - https://github.com/gfx-rs/wgpu/blob/ee3b85928e99f44194d0d4abedd88bf65a3a2614/wgpu-core/src/error.rs#L102
and I don't like it :(

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:16):

yea :/

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:16):

hence generic member access

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:16):

then you can type erase all the errors and just have a couple common types you're extracting at every step in the chain to get the extra info to report

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:16):

first, because effectively there are 2 places that match the errors: one for the fn display and another for resolving those extra things that need contexts. And secondly, because of downcasts are easy to miss/forget when messing with the error enum.

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:17):

yea, its a mess

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:17):

it sounds like, what we are doing is already not the worst of what we can do today, and in the future I should keep an eye on your RFC

view this post on Zulip Dzmitry Malyshau (Aug 16 2021 at 20:18):

Alright, thank you for your time explaining things to me, and good luck with this RFC!

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:18):

ty!

view this post on Zulip Jane Lusby (Aug 16 2021 at 20:18):

and my pleasure


Last updated: Jan 26 2022 at 13:32 UTC