Post 1.0 - updating functions in an API non-breaking way

[tcharding]

This is non-urgent musing about how our current error code will work post 1.0

We are making an effort to make it possible to update error types in a non-API breaking way. Are we painting ourselves into a place where to maintain the "correctness" of our error code we can no longer update functions.

Consider this code

pub fn foo_checker(&self) -> Result<(), Error> {
    if self.x != SOME_X_INVARIANT {
        return Err(Error::WrongX);
    }
    if self.y != SOME_Y_INVARIANT {
        return Err(Error::WrongY);
    }

    Ok(())
}

/// An error when checking a foo.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub enum Error {
    /// Wrong x.
    WrongX(WrongXError),
    /// Wrong y.
    WrongY(WrongYError),
}

// Hider errors `WrongXError` and `WrongYError` implement as is conventional.

Now consider post 1.0 something changes in the foo type that means the pubic foo_checker now has different function exits, including a new error variant and removal of an old error variant.

pub fn foo_checker(&self) -> Result<(), Error> {
    if self.x != SOME_X_INVARIANT {
        return Err(Error::WrongX);
    }
    if self.some_other_field != SOME_OTHER_INVARIANT {
        return Err(Error::Other);
    }

    Ok(())
}


/// An error when checking a foo.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub enum Error {
    /// Wrong x.
    WrongX(WrongXError),
    /// Wrong y.
    WrongY(WryngYError),
    /// Some new error post 1.0 release (we can add this later because of non_exhaustive).
    Other(OtherError),
}

The change is not API breaking so we have been successful in that regard, however the function now returns an error with unused variants. How do we handle that, accumulate all such changes until the next major release?

[apoelstra]

In short, I think yes. It seems unlikely that we'd be removing variants very often post 1.0. We may slightly change the meaning of variants, but if we actually remove them that seems like we've changed conceptually what a function is doing, which seems like a situation where we'd want to create a new function anyway.

I'm not saying it won't happen, just that it'll be rare and I think it's ok if it results in our error variants being somewhat crufty.

[tcharding]

Cool, sounds reasonable. Thanks

[Kixunil]

removal of an old error variant

We simply must be confident that we will always return each variant. E.g. it's inconceivable that hex parsing will ever not return InvalidChar. However we must resolve things like we had OddLengthString and InvalidLength in the same error.

This also makes me question if having a length limit on Amount is actually correct.

If we can't promise that, our only option is to return struct Error(InnerError); enum InnerError { ... }.

Note that there's also an argument that we could stabilize sooner if we completely hide each error type and unhide them later. (Unhiding is strictly breaking but for such silly reason that I'd make an exception there.) Though it's not a silver bullet and I am confident about some error types.