Crate evaluation for 2017-06-27: error-chain

[withoutboats]

Perhaps this comment is looking a bit beyond the libs blitz, but it seems like an apropos time to bring it up.

I think error chain is very good (significantly better than the status quo before error chain), but I feel slightly concerned about how its trending toward becoming the de facto solution. I suspect error chain could be even better if the problem space were examined in a lang+libs framework, instead of just a libs framework (that is, considering potential extensions to the language as well).

For example, the existence of both links and foreign_links seems inelegant to me; is there a solution based on specialization that could combine them?

Even more abstractly, if error chain is going to become “the wayto define an error in Rust,” I’d like to see a more thorough discussion of what other options were considered and discarded, what the fundamental trade offs are, etc. Maybe that discussion exists and I just haven’t read it.

[brson]

@withoutboats FWIW I’ve always considered error-chain to be something of hack and wish it didn’t have to exist, and I expect other solutions to appear. Both the existence of error-chain and its popularity in my mind speak to a weakness of the language and std. I hope though that even as error-chain is adopted it doesn’t prevent use of other error handling techniques, that it interoperates well.

It would be a fine result for me if error-chain got bumped to 1.0 basically as-is, with the understanding that error handling will continue to evolve, that there may be an error-chain 2.0, that other solutions might deprecate it completely.

[killercup]

Hey folks, I’m not sure if this is the right place and time, but maybe I’m lucky and you can help me out! If not, consider this a request for a “best practices” document that should be part of the error-chain docs!

I’m currently evaluating switching Diesel to use error-chain internally. One of our main concerns is presenting an interface to the user of the query builder that is forward-compatible, i.e., one we can add new errors to without breaking user code, while at the same time give return nice error that are easy to debug. My current idea is using error-chain (because it’s pretty nice to use; we’d probably have one root error type, plus maybe more specific error types in modules) and only exposing a struct DieselError { inner: LeRootErrorChainError } to the user. I’ve written a bit more here.

From what I’ve seen of error-chain so far, it seem geared toward application developers, not library authors. Would you recommend us using it at all? Is there anything we should definitely keep in mind?

[theduke]

To echo what @withoutboats said, I think that it would be big for ergonomics and usability to get something like error chain into the language or std.

A standardized convention for getting error traces and error nesting across all libraries would be huge for debugging and usability.

[bascule]

Well, I got error-chain to work with no_std, but the result is not particularly pretty:

https://github.com/brson/error-chain/pull/157

I think it speaks to some changes which it would be nice to have happen upstream in Rust. Most notably this one:

Move `::std::error` to `::core::error`

[brson]

Thanks for the efforts @bascule.

[brson]

I’ve updated the op with discussion points raised in this thread, and some of my own concerns.

There’s still quite a bit to do here and and a little over a week to do it:

• checklist needs to be completed and potential issues documented
• need to review outstanding issues with @Yamakaky for potential 1.0 blockers

@withoutboats, @bascule, are either of you interested in attending the libs meeting for this on the 27th, 1PM, to discuss?

Anybody up for tackling any of the remaining checklist sections? The "macros" section seems especially important.

[bascule]

@brson I should be able to come then. It’s at Mozilla SF?

[withoutboats]

I felt like your response was a good resolution & I’m not sure my comment is near-term actionable.

[KodrAus]

Is the plan to remove the quick_main macro once this RFC lands? Looks like it’s proposed to merge, but there’s still some unresolved issues.

The implementation isn’t too far off what quick_main does anyways.

[KodrAus]

It also seems a bit unfortunate to have an almost identical public copy of quick_error in the crate. If someone uses both quick_error and error_chain, the order the crates are imported will determine what version of the quick_error macro gets used:

#[macro_use]
extern crate quick_error;
#[macro_use]
extern crate error_chain;

// quick_error from error_chain
quick_error!(
    pub enum MyError {

    }
);

#[macro_use]
extern crate error_chain;
#[macro_use]
extern crate quick_error;

// quick_error from quick_error
// This doesn't actually compile, because quick_error requires you `#[derive(Debug)]
quick_error!(
    pub enum MyError {

    }
);

• Could the error_chain version be given a different name, like error_chain_quick_error and made #[doc(hidden)]?
• Is this use-case of re-exporting quick_error covered by one of the current macros RFCs?

[KodrAus]

• I’ve filled out some more detail in the evaluation and left some thoughts on the macro syntax and public implementation details
• I’ve added some discussion points to the bottom of the list in the OP (@aturon you might want to review those and see if they’re worthwhile)

[aturon]

Thanks, folks, for advancing this while I was away on vacation last week!

Today I did a careful review of the crate myself, and added a bunch of discussion items, including pulling the most important guidelines issues into scope. You can see these at the top, but for reference:

• There are several issues around the macro syntax:

  • The syntax not matching corresponding declaration form
  • The types declaration, with its hand-rolled system of defaults, doesn’t quite feel “rustic”
  • Attributes only coming after items
  • No ability to place attributes or visibility modifiers on Error or ErrorKind
  • No ability to use generics in error variants
  • Cannot document fields in struct type error
  • Cannot currently use derive on the Error type
  • Links with custom description/display

• There’s not currently a good story for boxing up chainable errors

• The names “links” and “foreign links” are not fully evocative of the crucial distinction: whether chaining occurs

  • It’d be good to dive more deeply into the rationale for this precise split.

• What are the pros/cons of locally defining Error and ResultExt?

  • It seems like links could be simplified if Errors were just type aliases

• Error derefs to ErrorKind, how do we feel about that?

• Msg(String) always being a variant, with conversions – worth talking about

  • This may be one aspect that separates libs from apps

• error_chain, error_chain_processed and quick_error – can these be hidden?

• Why the display method on ChainedError?

• Can ChainedError be hidden or made private?

• Do we want to allow conversions with chain_err

[aturon]

Even more abstractly, if error chain is going to become “the wayto define an error in Rust,” I’d like to see a more thorough discussion of what other options were considered and discarded, what the fundamental trade offs are, etc.

I definitely agree, and I think such a discussion would ideally be part of the crate evaluation. I have similar reservations to yours about the links/foreign links split and its rationale, for example.

I’m not sure we’ll be able to complete a deep dive into the tradeoffs during the official libs team meeting, but I think it’s something we can and should do out of band. Even if we don’t end up changing the library much for a 1.0, it’d be great to document the design rationale and note shortcomings that might be addressed with new language features.

[brson]

@bascule oh, crap, sorry for taking so long to circle back to this. Yes, by good chance, this particular meeting will take place at the Mozilla SF office, at 1 PM, tomorrow, since it takes place during the Mozilla all-hands, and you are welcome to be there. It’ll also be over video conf. I’ll send you connection info in a bit.

[brson]

Is the plan to remove the quick_main macro once this RFC lands? Looks like it’s proposed to merge, but there’s still some unresolved issues.

I would not plan to remove quick_main, for compatibility with older Rust; and I expect ? in main to take a long time to stabilize.

[brson]

I filled in the OP with some of my own commentary, including around the reasoning for the ‘links’ / ‘foreign_links’ split (links have special pattern-matching / backtrace propagation capabilities that foreign_links don’t); and the definition of local error types / traits (coherence violations otherwise).

I moved the renaming of quick_error to an issue - it is plainly an implementation detail and should not be clobbering the quick_error crate.

The internal quick_error macro was just a way to get the quick_error syntax without figuring out how to write it myself. It supports syntax that error-chain doesn’t even use, so would be better to pare it down to something error-chain specific.

[budziq]

Hi this crate still needs quality cookbook examples! There are some proposals no suggestion has been found particularly persuasive yet.

[brson]

The libs team discussed this at last week’s meeting. It was a pretty productive meeting, and a lot of remediations fell out of it. We’ll get either @aturon or myself to file the bugs soon.

Some of the big resolutions were:

• lower the Error trait into probably the alloc crate so no_std error-chain makes sense
• investigate a complete syntactic redesign to accommodate all the missing capabilities raised in the review. This will certainly require using some form of procedural macros. It may or may not be a 1.0 blocker

I’m uh, missing the etherpad link we used to take notes, so I hope somebody else has it.

[brson]

Ah, here’s the etherpad from the meeting:

https://public.etherpad-mozilla.org/p/rust-api-guidelines-error-chain