-
Notifications
You must be signed in to change notification settings - Fork 20
Add ErrorMessage enum, start using it #355
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
Conversation
88b9580
to
9132927
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly curious what @jedel1043 thinks as he had a much more vocal stance.
I'm a bit mixed, but leaning towards this is fine. I think my mixed opinion comes mostly from it being an intermediary.
674c3bc
to
9132927
Compare
Yeah, it's an intermediary, but at the very least it's an intermediary that lets the error enum be Copy, which is super nice. |
Hmm, I'm thinking maybe this should be moved to Yeah, I like this approach. Let's go with this and see how it would fit on the Rust side. We can still do more adjustments when we migrate the non-static error messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Wonderful, I'll try and fill in the rest |
This can even become non-optional after #355 (maybe), but this allows V8 to start throwing readable errors without needing #355 to be landed first. Co-authored-by: Kevin Ness <[email protected]>
This is a step towards #113. I didn't finish converting all of the enums, just some of them to give an idea of what this might look like.
This error enum will be private for now: It's primarily encompassing messages, and will wrap all static string messages. The non-static ones I think we should convert to something else at the boundary, potentially logging them with a feature.
Once this is fully filled in, I plan to:
This is not primarily designed with user matching in mind: it can be matched by the user, but I don't think this is the right level of granularity. Instead, we can use this error message ontology to map to a more structured public error ontology if we want. Basically, in the medium term we can have a RustErrorKind that can be internally obtained from ErrorMessage, which we expose to the user.
Thoughts?