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

return the valhalla error Path distance exceeds the max distance limit. #1781

Merged
merged 7 commits into from
May 1, 2019

Conversation

gknisely
Copy link
Member

@gknisely gknisely commented May 1, 2019

Issue

Return the Valhalla error Path distance exceeds the max distance limit for OSRM responses when the route is greater than the service limits.

Tasklist

  • Review - you must request approval to merge any PR to master
  • Generally use squash merge to rebase and clean comments before merging
  • Update the changelog

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

Original response.

image

New response.

image

src/worker.cc Outdated
if (found == OSRM_ERRORS_CODES.cend())
found = OSRM_ERRORS_CODES.find(199);
body << (request.options.has_jsonp() ? request.options.jsonp() + "(" : "") << found->second
<< (request.options.has_jsonp() ? ")" : "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of all of this, couldnt you just change the code above to be:

{154, R"({"code":"NoRoute","message":"Path distance exceeds the max distance limit."})"},

kevinkreiser
kevinkreiser previously approved these changes May 1, 2019
Copy link
Member

@kevinkreiser kevinkreiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ship when tests pass

@gknisely gknisely merged commit 4361f70 into master May 1, 2019
@purew purew deleted the distance_exceeds branch February 13, 2020 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants