`impl Error for String`

libs
1

Now that description is deprecated for Error, maybe it’s time to implement it for String. Would look something like

impl Error for String {}

String errors are often useful - for example failure allows errors to be constructed using either format_err! or err_msg. I often find myself making my own string newtype so I can implement Error over it, for when errors represent conditions that are unrecoverable and I’m using Errors rather than panics for nicer crashing, or when I’m writing code quickly and I don’t want to spend ages making my ErrorKinds.

EDIT String is also 'static + Send + Sync + Clone, so it works when you want an error that is these things as well.

An alternative would be to create an error type like

pub struct StringError {
    msg: String,
    cause: Option<Box<Error + 'static + Send + Sync>>
}
1 Like
2

Stringly typed programming is generally viewed as an antipattern, and a brittle one at that. Yet you suggest exactly that for error handling. Is the convenience of not having to define error types really worth the risk of shifting the “default” thing to do (ie. defining an error type) towards something that is technologically inferior w.r.t. error handling?

Because that’s the consequence: handling string errors is somewhere between inefficient and impossible, depending on the type. If it’s string literals (&str), they at least can be matched against, but this is O(n) if n is the length of the string, compared to O(1) for e.g. enums.

However, if they’re potentially formatted strings (as any String value is) then matching goes out the window too, and with it the capability to handle errors properly rather than just dumping them to e.g. stderr.

Personally I’m fairly sure I’d avoid a library that did this at all costs, as I can’t do anything about errors originating there.

9 Likes
3

I write a lot of Go at work where I have to .String() errors and hope I can introspect them correctly. It is very, very unpleasant. And don’t get me started on changing an error message and having all your tests fail…

@jjpe makes a great case for why this is a terrible idea.

1 Like
4

A selection of quotes:

I think both these comments bring up an important concern: that people won't type their errors enough. There is a balance here: with syscalls you usually just get an errno, which is very typed (an enum with no associated data), but quite often doesn't give you the information you need to fix the error (I've found alsa to be particularly bad in this regard, for instance). I fully support the argument that stringly-typed errors are not best solution in many situations.

However, I don't think this impl will cause that to happen for the reasons below:

Firstly, you can already use strings as errors (Result<(), String>). Something doesn't need to implement the error trait to be an error. However, it does need to implement Error if you want it to be the source of another error, which is where using a string error is useful. Until recently, it was not possible to downcast causes into a specific type, so basically the only thing you could do with them was write them to a string or some Writer. So I would argue that the issue with typing errors is orthogonal to this RFC.

In my experience (using failure a lot, also see error-chain), having a causal chain of errors gives really great UX, both to code writers and end users. It encourages really detailed error messages allowing the reason to be pinpointed and fixed. The structure I have found most effective is to have string errors near the bottom of the chain, to supply as much information as possible, and typed errors near the top, where you are most likely to attempt recovery, depending on the error type.

Note that if you wanted to have causes to a String error, you'd need a newtype, which could for example be

pub struct DisplayError<T> {
    msg: T,
    source: Option<Box<(dyn Error + 'static)>>,
}

impl From<String> for StringError<String> { .. }
// etc.

impl<T: Display> Display for DisplayError {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        Display::fmt(self.msg, f)
    }
}

impl Error for DisplayError {
    fn source(&self) -> &(dyn Error + 'static) {
        self.source.as_ref().map(|e| e.as_ref())
    }
}

(added) otherwise you force the String error to be at the bottom of the chain (it cannot have a source), which may be desirable!

Hopefully this response has reassured you that this impl won't be abused and will just enable better more detailed error chains. :slight_smile:

2 Likes
5

I think you're conflating error messages with stringly-typed errors. Having good error messages is extremely important, but not something that can be easily enforced. On the other hand, error types must be introspectible by code; looking at Error::FileNotFound("/my/file") is far, far easier than looking at "file does not exist: /my/file".

I believe this is Go's failure in designing its error handling, rather than the if err != nil people seem to complain about: instead of being able to compare errno to things in a header, you can't expect to make any sane comparisons (except with nil), at all! Making it easier to return Strings as errors than it already is will lead to proliferation of this style.

Unfortunately, experience has proven to me that anything you put in a standard library will be abused, so, while it sounds like you won't be intentionally doing anything bad with it, I don't agree with your conclusion.

2 Likes
6

There’s already a From impl for conversion of &str to Box<dyn Error>, so you can already use it for stringly-typed errors.

7 Likes
7

That’s interesting - I can’t find it in the docs, is it discoverable? Also is it impl<'a> From<&'a str> for Box<(dyn Error + 'a)>, or impl From<&'_ str> for Box<(dyn Error)> (does it copy the contents of the str or reference it)?

EDIT found them - in the Box documentation. There are loads of different variants. Thanks

1 Like
8

It copies. I haven’t checked the docs (trait impls are messy indeed), but Err("foo")? definitely works.

1 Like
9

Here is the canonical impl

#[stable(feature = "rust1", since = "1.0.0")]
impl From<String> for Box<dyn Error + Send + Sync> {
    fn from(err: String) -> Box<dyn Error + Send + Sync> {
        #[derive(Debug)]
        struct StringError(String);

        impl Error for StringError {
            fn description(&self) -> &str { &self.0 }
        }

        impl Display for StringError {
            fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                Display::fmt(&self.0, f)
            }
        }

        Box::new(StringError(err))
    }
}

looks good, thanks for the suggestion!

10

I wonder if this inner error value could be a DSO wrapping an str. Then the From<String> impl could convert the string pointer directly into the Box without extra allocation and copying.

11

Maybe some ppl just don’t want their errors to be handled (outside of printing them somewhere)

1 Like
12

@soni The status quo does not prevent that. On the other hand, implementing stringly typed errors and then using them in a library forces that choice on all users, which is not very nice (both in terms of being social, but also as a technical solution), to say the least.

1 Like
13

This is degrading into a philosophical discussion, but...

Example 1 - Exceptional failure

I have misconfigured my audio setup, and using the C lib alsa causes an error.

It's not sensible to return a complex strongly typed error in this case. A string describing why the system is misconfigured is absolutely the best thing to return - it allows the user to fix their issue. No one is ever going to do recovery based on the type of misconfiguration, and if they just want to continue with no sound they match on Err(_).

This is essentially like an expect, but with a single recovery path (as opposed to no recovery).

Example 2

I am parsing a toml file, and error when the file is not valid TOML.

Again it's very unlikely that the user will want to do recovery based on the type of syntax error. Normally, you just need to output a helpful error message that describes why the file isn't valid TOML. A string error is perfect here.

Aside: In reality you may want to type your errors here since TOML is such a widely used part of the ecosystem, but if there were an equivalent format that you used internally, you will not be interested in 50% of your codebase being error handling.

Example 3 - An alternative to a panic

I'm writing a 0.1 version of a library and I want to present a MVP, so I can get feedback.

Here you could spend a lot of time working on errors, only for it all to be replaced when someone points out a flaw in your design that requires a refactor. It's silly to do this. Just tidy up error handling before your 1.0 release.

Having said all this, I think it's enough that impl From<String> for Box<dyn Error> exists. But I would absolutely argue that there are times that string errors are appropriate.

I'll finish by saying that in the wild what I see is that if someone can't use string errors, they will panic. We're not forcing them to write beautifully algebraic error types, we're forcing them the other way if anything.

14

Some applications would also like the line number and column; for example, code editor integration would like to know where to display a squiggly mark.

In general, a dedicated error type is always preferable in a library API, even if it wraps a string or a Box<dyn Error> internally.

2 Likes
15

Agreed.

But that is why I put the caveat that because toml is a very widely used crate, it's not the perfect example. Imagine another format that you only use internally in your crate/organisation, where you have full code control. You could just add this as and when you needed it, and get prototyping quicker.

EDIT Also in this case the core message is still a string, you just want a bit of metadata as well (maybe akin to log vs slog). There are cases where structured logging is unnecessary, just as there are cases where structured errors are unnecessary.

16

To summarize arguments against, and my responses

It would encourage bad practice.

I think this is the best argument against, but even then I believe the reasons for are more powerful. I would dispute the claim that it encourages bad practice, based on the evidence from the failure crate, where there is the option of string errors. I don't think there have been many (any?) cases where this has been abused. On the upside, it avoids boilerplate that is unnecessary and confuses matters.

Library maintainers might be lazy and write bad errors.

This already happens. In the amethyst engine, a lot of problems just panicked, until the engine was fairly mature, when error-chain was used. It still makes extensive use of string errors (caveat I haven't looked for a while, may have changed), which is easy with error-chain or failure. At some point in the future it will probably be refactored to use typed errors, which can be done incrementally.

If its open source, you can always submit a PR to turn string errors into something more typed. I've done this a few times.

I don't think this is true - it would depend on the library and what it's doing. Or you can fork it and add typed errors if you need them. It only takes 1 person to do this and PR it, and then you have typed errors.

Check out go by example errors - URGH! Bad error handling in go is being driven by the lack of algebraic data types, not stringy errors. Structured types in rust are beautiful, and I think this will mean string errors won't be abused in the same way.

Yes matching on string errors is inefficient. But string errors are only for certain situations. If you're matching on a string error, you (or someone else) needs to refactor the library. Also I think people will use algebraic data types by default anyway because they are beautiful. I would/do.

I understand concerns but I'm pretty confident that my argument is convincing: there are occasions when this is correct program behavior, and the option for it should be in std.

An alternative is to have a type like StringError(String) and fn from_display(d: impl Display) -> StringError in std::error, but I think this is less clear, and it introduces a boilerplate type.

EDIT A few edits expanding this post.

17

Also see urlo for some fun involving the current arrangement (Into<Box<dyn Error>> for String)

18

To be clear, in practice, everyone just uses fmt.Errorf.

2 Likes
19

I wouldn't do anything like that, unless:

  1. it was something I needed really badly (this is not often the case)
  2. it would be more work to write from scratch, than it would be to provide a fork with proper errors and then maintain that fork (which is a bad idea for the longer term).

But even if I decided to write a patch, there's a large probability that the PR wouldn't be accepted, or else it would be the maintainer essentially admitting that stringly typed errors were a suboptimal choice (in which case they should have done something about it themselves, or better yet, they shouldn't have used stringly typed errors at all).

Besides, it is not the community's responsibility to fix design flaws in dependencies.

You seem to contradict yourself here. You argue in favor of stringly typed errors, yet you readily admit it's an absolute disaster in Go. Aside from that, is that really what you think will happen, especially if the poorer option seems easier to use at the beginning than the proper one? I think that experienced Rustaceans will know not to touch stringly typed errors with a 10 foot pole, but as the Rust Survey 2018 makes clear, most Rustaceans (rougly 75%) are not all that experienced with Rust at this time (i.e. that 75% has < 2 years of experience with the language). Therefore, the better assumption is that inexperienced users will go for the easier-seeming option, which leads to "error handling" similar to the Go example above.

It's a mistake to think others think like you yourself do. In fact, the larger the population, the more solutions to a given problem look like a local search / evolutionary process: all options are tried at some point or another. Of course at some point consensus might be formed, but by that time the damage is done, and we have a bunch of nigh-unusable crates.

Agreed, that's one of the reasons I'm arguing against stringly typed errors.

It fails to convince me, so no.

The ALSA example above is a bad one BTW: ALSA is notorious for its poor code quality, to the point that PulseAudio needed to paper over its bugs to some extent.

The TOML example equally fails to convince me: just because you happen not to do error recovery doesn't mean it should be hard to do or even impossible there. Quite the opposite in fact: A flexible software design is a good design (flexibility being defined as an ability to react to previously-unexpected demands from new use cases).

As for the 3rd example i.e. an alternative to panic!: there already is one. It's the enum-based error system we already have.

What I suspect you're really arguing is that you find it too much work to properly design and implement enum errors (optionally enhanced by std::error::Error or failure::Fail). If that's indeed the case then that I can understand. But the solution to that is decidedly not stringly typed errors.

1 Like
20

Sorry this didn't really make my point. What I meant was that a crate won't be successful if another crate exists with better code quality. The natural competition in an open ecosystem like crates.io will ensure that better error handling is favored in successful crates.

Sorry the point I was making was that because of the lack of a Result<T, E> type, it's almost like the C situation where you have to return an int and set an error number - you just don't have the rich types you do in rust.

I was saying that what users will do depends on carrot (algeraic data types) and stick (no impl Error for string). In rust there is a big carrot that doesn't exist in go, which will mean that behavior is different (we don't know how different we don't have any experimental evidence for this).

This is an interesting point - that people may do what is familiar to them. I assume that because the documentation wouldn't even mention error handling using strings, that people would only discover it once they were already seasoned rustaceans. However there is a risk that they discover it too early.

I agree with this, of course. I have got into trouble in a few jobs for refusing to rush coding and accruing technical debt, because I know it will cost the company money in the long term. The fact that I like to write aesthetically beautiful programs is secondary :wink:

I would also prefer a better solution if one exists.

I think the argument boils down to whether you think having this would mean it is abused, and for the reasons I've stated I think that it wouldn't be in rust (especially given that it wouldn't be advertised), but I concede that there is no experimental evidence for that. In any case, I think this thread has laid out the arguments for and against reasonably well.