nmandery/h3ron

Handling error codes

ManevilleF opened this issue · 3 comments

Hello,

According to H3 RFC 4 any H3 Methods can return some error code which would be good to implement to improve the error system.
I wrote a draft like this:

/// Errors as defines in [RFC] 4.0.0
///
/// [RFC]: https://github.com/uber/h3/blob/master/dev-docs/RFCs/v4.0.0/error-handling-rfc.md
#[derive(Debug, Copy, Clone, PartialOrd, PartialEq)]
pub(crate) enum InternalErrorCode {
    /// The operation failed but a more specific error is not available
    Failed, // 1
    /// Argument was outside of acceptable range (when a more specific error code is not available)
    Domain, // 2
    /// Latitude or longitude arguments were outside of acceptable range
    LatLonDomain, // 3
    /// Resolution argument was outside of acceptable range
    ResDomain, // 4
    /// H3Index cell argument was not valid
    CellInvalid, // 5
    /// H3Index directed edge argument was not valid
    DirectionalEdgeInvalid, // 6
    /// H3Index undirected edge argument was not valid
    UniDirectionalEdgeInvalid, // 7
    /// H3Index vertex argument was not valid
    VertexInvalid, // 8
    /// Pentagon distortion was encountered which the algorithm could not handle it
    Pentagon, // 9
    /// Duplicate input was encountered in the arguments and the algorithm could not handle it
    DuplicateInput, // 10
    /// H3Index cell arguments were not neighbors
    NotNeighbors, // 11
    /// H3Index cell arguments had incompatible resolutions
    ResMismatch, // 12
    /// Necessary memory allocation failed
    Memory, // 13
    /// Unknown error code
    UnknownError(i32),
}

impl InternalErrorCode {
    /// Checks if the H3 return value is an error and returns the associated error code
    pub fn get_error(value: i32) -> Option<Self> {
        match value {
            0 => None,
            1 => Some(InternalErrorCode::Failed),
            2 => Some(InternalErrorCode::Domain),
            3 => Some(InternalErrorCode::LatLonDomain),
            4 => Some(InternalErrorCode::ResDomain),
            5 => Some(InternalErrorCode::CellInvalid),
            6 => Some(InternalErrorCode::DirectionalEdgeInvalid),
            7 => Some(InternalErrorCode::UniDirectionalEdgeInvalid),
            8 => Some(InternalErrorCode::VertexInvalid),
            9 => Some(InternalErrorCode::Pentagon),
            10 => Some(InternalErrorCode::DuplicateInput),
            11 => Some(InternalErrorCode::NotNeighbors),
            12 => Some(InternalErrorCode::ResMismatch),
            13 => Some(InternalErrorCode::Memory),
            v => Some(InternalErrorCode::UnknownError(v)),
        }
    }

    /// Checks if the H3 return value is an error
    pub fn is_error(value: i32) -> bool {
        value != 0
    }
}

But in many cases the error codes don't work as the RFC intended.
For example: the is_neighbor_to method according to documentation should return E_SUCCESS = 0 in case of success, but instead it returns 1 as h3ron correctly checks.

In different documentation I found out that it should indeed return 1 on success and 0 on failure, as opposed to the RFC v4.

So I don't know if we should try to implement this partially or completely forget about it.

It seems theRFC V4.0.0 is still in draft so that explains it

I would propose to wait with this until H3 v.4 gets released - as you mentioned. I had a look into the changes coming in the next major version - there will be plenty of them affecting this crate. So I guess there will a major refactoring be necessary here anyways.

For completeness, here are the relevant RFCs, including the one you mentioned: https://github.com/uber/h3/tree/master/dev-docs/RFCs/v4.0.0

Thank you for this draft - it has been included in #33