Skip to content
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

Error refactoring #138

Open
nyurik opened this issue Apr 9, 2023 · 2 comments
Open

Error refactoring #138

nyurik opened this issue Apr 9, 2023 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@nyurik
Copy link
Member

nyurik commented Apr 9, 2023

GeoZero has one Error enum, and many of the values are String objects that contain the actual error information, sometimes formatted.

I think it would better to refactor it so that each specific error can be easily analyzed without any extra performance cost.

Proposed structure

enum GeozeroError {
    arrow: ArrowError, csv: CsvError, gdal: GdalError, geojson: GeojsonError, geos: GeosError, gpkg: GpkgError, gpx: GpxError, mvt: MvtError, postgis: PostgisError, svg: SvgError, tessellator: TessellatorError, wkb: WkbError, wkt: WktError,
}

Each one of the sub-errors are their own enums with the actual values. So instead of this in src/mvt/mvt_reader.rs:

return Err(GeozeroError::Feature(format!(
    "invalid feature.tags length: {:?}", feature.tags.len()
)));

we will have this:

return Err(MvtError::InvalidFeatureTagsLength(feature.tags.len());

Note that thiserror has #[from] to simplify MvtError -> GeozeroError conversion.

Existing errors

Here are all the error usages at the moment (some of these are used multiple times)

GeozeroError::ColumnNotFound
GeozeroError::ColumnType(stringify!($e), format!("{v:?}")
GeozeroError::Dataset(error
GeozeroError::Feature("invalid feature.tags length: {feature_tags_count:?}"
GeozeroError::Feature("invalid key index {key_idx}"
GeozeroError::Feature("invalid value index {value_idx}"
GeozeroError::Geometry("CoordSeq missing"
GeozeroError::Geometry("Invalid UTF-8 encoding"
GeozeroError::Geometry("Missing Geometry"
GeozeroError::Geometry("Missing LineStrings for Polygon"
GeozeroError::Geometry("Missing container for LineString"
GeozeroError::Geometry("Missing container for Polygon"
GeozeroError::Geometry("Missing polygons for MultiPolygon"
GeozeroError::Geometry("No LineStrings for MultiLineString"
GeozeroError::Geometry("No coords for LineString"
GeozeroError::Geometry("No coords for MultiPoint"
GeozeroError::Geometry("No coords for Point"
GeozeroError::Geometry("Not ready for coords"
GeozeroError::Geometry("The input was an empty Point, but the output doesn't support empty Points"
GeozeroError::Geometry("Too few coordinates in line or ring"
GeozeroError::Geometry("Unexpected geometry type"
GeozeroError::Geometry("test"
GeozeroError::Geometry(error
GeozeroError::Geometry(format!("Unsupported geometry type {geometry_type}"
GeozeroError::GeometryFormat
GeozeroError::IoError(io_err
GeozeroError::Property(format!("unsupported value type for key {key}"
@nyurik nyurik added the help wanted Extra attention is needed label Apr 9, 2023
@nyurik nyurik added enhancement New feature or request good first issue Good for newcomers labels Jun 9, 2023
nyurik added a commit that referenced this issue Jun 22, 2023
Relates to  #138 

---------

Co-authored-by: Yuri Astrakhan <[email protected]>
@nrhill1
Copy link
Contributor

nrhill1 commented May 21, 2024

I’m thinking of taking this task on…is it still open? Or was it fixed in a different issue?

@michaelkirk
Copy link
Member

Take a look at the code and see?

github-merge-queue bot pushed a commit that referenced this issue Sep 18, 2024
Hey @nyurik ,

I made an error module for `csv` in response to #138 . Let me know if
you think any changes need to be made.

---------

Co-authored-by: Michael Kirk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants