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

RouteResponseParser Error strategy #170

Open
Archdoog opened this issue Jul 31, 2024 · 3 comments
Open

RouteResponseParser Error strategy #170

Archdoog opened this issue Jul 31, 2024 · 3 comments
Labels
android core Related to the Rust core iOS

Comments

@Archdoog
Copy link
Collaborator

When using tools like createOsrmResponseParser, your API your API may return a standard data structure (e.g. Valhalla's {"code": "InvalidOptions", "message": "Options are invalid.", "routes": []}). We should offer a method to handle error parsing/throwing through the parser.

@Archdoog Archdoog added android iOS core Related to the Rust core labels Jul 31, 2024
@ianthetechie
Copy link
Contributor

Good catch! Yes, it only models the happy path and everything else is a parsing error for now. What we should do to fix this is model both the success and error responses. Serde can model this with an enum having two variants which it discriminates on.

I'm thinking we should:

  • Create a new enum with the right serde derive macros that can parse to either a RouteResponse or RouteErrorResponse (type name TBD; we need to determine if the error message format here is Valhalla-specific or not)
  • Add a new variant to RoutingResponseParseError which indicates that there wasn't a network or parsing error, but that the routing engine itself returned an error.
  • In OsrmResponseParser's parse_response method, the first deserialization step will parse to the new enum. If it's an error variant, we bail and return the error. The public API for parse_response doesn't have to change and you still get error handling semantics both in Rust and upstream (ex: Kotlin will throw a useful exception with the full error details from the backend).

I think this isn't even modeled in the OpenAPI stuff you've been looking at from us btw :P It probably should be.

@ianthetechie ianthetechie changed the title RouteParser Error strategy RouteResponseParser Error strategy Jul 31, 2024
@ianthetechie
Copy link
Contributor

Idea discussed on the call: passing more "internal" error info up the stack.

@ianthetechie
Copy link
Contributor

Related: #313 and #263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android core Related to the Rust core iOS
Projects
None yet
Development

No branches or pull requests

2 participants