From d6afe91d8fcf4bff0cd347e766f36905f3bbf86b Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Bhagat-Conway Date: Thu, 23 Mar 2023 14:18:58 -0400 Subject: [PATCH] print tracebacks and line numbers for Lua runtime errors (#6564) * print tracebacks and line numbers for Lua runtime errors * revert format changes * update changelog with lua traceback, #6564 * revert using protected_function for old GetStringListFromFunction and source_function #6564 * add unit test for line numbers in tracebacks, #6564 * apply clang-format (#6564) * remove unused test helper function, #6564 * suppress leaksanitizer warnings in extract-tests, #6564 When the extractor encounters a lua runtime error, some osmium objects are not freed. In production this doesn't matter because these errors bring down OSRM. In the tests we catch them to ensure they occur, and the leaksanitizer flags them. --- CHANGELOG.md | 1 + .../extractor/scripting_environment_lua.hpp | 8 +- scripts/ci/leaksanitizer.conf | 9 ++ src/extractor/scripting_environment_lua.cpp | 73 +++++++-- test/data/profiles/bad_node.lua | 140 +++++++++++++++++ test/data/profiles/bad_segment.lua | 143 ++++++++++++++++++ test/data/profiles/bad_setup.lua | 140 +++++++++++++++++ test/data/profiles/bad_turn.lua | 140 +++++++++++++++++ test/data/profiles/bad_way.lua | 140 +++++++++++++++++ unit_tests/library/extract.cpp | 132 ++++++++++++++++ 10 files changed, 906 insertions(+), 20 deletions(-) create mode 100644 test/data/profiles/bad_node.lua create mode 100644 test/data/profiles/bad_segment.lua create mode 100644 test/data/profiles/bad_setup.lua create mode 100644 test/data/profiles/bad_turn.lua create mode 100644 test/data/profiles/bad_way.lua diff --git a/CHANGELOG.md b/CHANGELOG.md index 98a871bf4..cfad16822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - CHANGED: Make edge metrics strongly typed [#6420](https://github.com/Project-OSRM/osrm-backend/pull/6420) - FIXED: Typo in file name src/util/timed_historgram.cpp -> src/util/timed_histogram.cpp [#6428](https://github.com/Project-OSRM/osrm-backend/issues/6428) - CHANGED: Replace boost::string_ref with std::string_view [#6433](https://github.com/Project-OSRM/osrm-backend/pull/6433) + - ADDED: Print tracebacks for Lua runtime errors [#6564](https://github.com/Project-OSRM/osrm-backend/pull/6564) - Routing: - FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419) # 5.27.1 diff --git a/include/extractor/scripting_environment_lua.hpp b/include/extractor/scripting_environment_lua.hpp index b81e9024c..ef5243454 100644 --- a/include/extractor/scripting_environment_lua.hpp +++ b/include/extractor/scripting_environment_lua.hpp @@ -41,10 +41,10 @@ struct LuaScriptingContext final bool has_way_function = false; bool has_segment_function = false; - sol::function turn_function; - sol::function way_function; - sol::function node_function; - sol::function segment_function; + sol::protected_function turn_function; + sol::protected_function way_function; + sol::protected_function node_function; + sol::protected_function segment_function; int api_version = 4; sol::table profile_table; diff --git a/scripts/ci/leaksanitizer.conf b/scripts/ci/leaksanitizer.conf index 9f0e08c19..27c16d04a 100644 --- a/scripts/ci/leaksanitizer.conf +++ b/scripts/ci/leaksanitizer.conf @@ -6,3 +6,12 @@ # #1 0x7f7ae595d13e (/usr/lib/x86_64-linux-gnu/libtbb.so.2+0x2213e) leak:libtbb.so + +# The extract-tests leak some memory in the tests to confirm that +# lua errors print tracebacks. +# This appears to be because when these tests throw exceptions, the +# osmium objects being processed are not freed. In production this doesn't +# matter, as the exceptions bring down the entire osrm-extract process. In the +# tests, we catch the error to make sure it occurs, which is why the +# leaksanitizer flags it. +leak:extract-tests \ No newline at end of file diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp index 0ab3df52d..9576cc18b 100644 --- a/src/extractor/scripting_environment_lua.cpp +++ b/src/extractor/scripting_environment_lua.cpp @@ -91,6 +91,19 @@ struct to_lua_object : public boost::static_visitor }; } // namespace +// Handle a lua error thrown in a protected function by printing the traceback and bubbling +// exception up to caller. Lua errors are generally unrecoverable, so this exception should not be +// caught but instead should terminate the process. The point of having this error handler rather +// than just using unprotected Lua functions which terminate the process automatically is that this +// function provides more useful error messages including Lua tracebacks and line numbers. +void handle_lua_error(sol::protected_function_result &luares) +{ + sol::error luaerr = luares; + std::string msg = luaerr.what(); + std::cerr << msg << std::endl; + throw util::exception("Lua error (see stderr for traceback)"); +} + Sol2ScriptingEnvironment::Sol2ScriptingEnvironment( const std::string &file_name, const std::vector &location_dependent_data_paths) @@ -550,10 +563,16 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context) std::numeric_limits::max()); // call initialize function - sol::function setup_function = function_table.value()["setup"]; + sol::protected_function setup_function = function_table.value()["setup"]; if (!setup_function.valid()) throw util::exception("Profile must have an setup() function."); - sol::optional profile_table = setup_function(); + + auto setup_result = setup_function(); + + if (!setup_result.valid()) + handle_lua_error(setup_result); + + sol::optional profile_table = setup_result; if (profile_table == sol::nullopt) throw util::exception("Profile setup() must return a table."); else @@ -1123,7 +1142,9 @@ void Sol2ScriptingEnvironment::ProcessTurn(ExtractionTurn &turn) case 2: if (context.has_turn_penalty_function) { - context.turn_function(context.profile_table, std::ref(turn)); + auto luares = context.turn_function(context.profile_table, std::ref(turn)); + if (!luares.valid()) + handle_lua_error(luares); // Turn weight falls back to the duration value in deciseconds // or uses the extracted unit-less weight value @@ -1138,7 +1159,9 @@ void Sol2ScriptingEnvironment::ProcessTurn(ExtractionTurn &turn) case 1: if (context.has_turn_penalty_function) { - context.turn_function(std::ref(turn)); + auto luares = context.turn_function(std::ref(turn)); + if (!luares.valid()) + handle_lua_error(luares); // Turn weight falls back to the duration value in deciseconds // or uses the extracted unit-less weight value @@ -1184,24 +1207,28 @@ void Sol2ScriptingEnvironment::ProcessSegment(ExtractionSegment &segment) if (context.has_segment_function) { + sol::protected_function_result luares; switch (context.api_version) { case 4: case 3: case 2: - context.segment_function(context.profile_table, std::ref(segment)); + luares = context.segment_function(context.profile_table, std::ref(segment)); break; case 1: - context.segment_function(std::ref(segment)); + luares = context.segment_function(std::ref(segment)); break; case 0: - context.segment_function(std::ref(segment.source), - std::ref(segment.target), - segment.distance, - segment.duration); + luares = context.segment_function(std::ref(segment.source), + std::ref(segment.target), + segment.distance, + segment.duration); segment.weight = segment.duration; // back-compatibility fallback to duration break; } + + if (!luares.valid()) + handle_lua_error(luares); } } @@ -1211,20 +1238,27 @@ void LuaScriptingContext::ProcessNode(const osmium::Node &node, { BOOST_ASSERT(state.lua_state() != nullptr); + sol::protected_function_result luares; + + // TODO check for api version, make sure luares is always set switch (api_version) { case 4: case 3: - node_function(profile_table, std::cref(node), std::ref(result), std::cref(relations)); + luares = + node_function(profile_table, std::cref(node), std::ref(result), std::cref(relations)); break; case 2: - node_function(profile_table, std::cref(node), std::ref(result)); + luares = node_function(profile_table, std::cref(node), std::ref(result)); break; case 1: case 0: - node_function(std::cref(node), std::ref(result)); + luares = node_function(std::cref(node), std::ref(result)); break; } + + if (!luares.valid()) + handle_lua_error(luares); } void LuaScriptingContext::ProcessWay(const osmium::Way &way, @@ -1233,20 +1267,27 @@ void LuaScriptingContext::ProcessWay(const osmium::Way &way, { BOOST_ASSERT(state.lua_state() != nullptr); + sol::protected_function_result luares; + + // TODO check for api version, make sure luares is always set switch (api_version) { case 4: case 3: - way_function(profile_table, std::cref(way), std::ref(result), std::cref(relations)); + luares = + way_function(profile_table, std::cref(way), std::ref(result), std::cref(relations)); break; case 2: - way_function(profile_table, std::cref(way), std::ref(result)); + luares = way_function(profile_table, std::cref(way), std::ref(result)); break; case 1: case 0: - way_function(std::cref(way), std::ref(result)); + luares = way_function(std::cref(way), std::ref(result)); break; } + + if (!luares.valid()) + handle_lua_error(luares); } } // namespace osrm::extractor diff --git a/test/data/profiles/bad_node.lua b/test/data/profiles/bad_node.lua new file mode 100644 index 000000000..74c730587 --- /dev/null +++ b/test/data/profiles/bad_node.lua @@ -0,0 +1,140 @@ +-- copy of testbot with process_node throwing a runtime error + +api_version = 4 + +function setup() + return { + properties = { + continue_straight_at_waypoint = true, + max_speed_for_map_matching = 30/3.6, --km -> m/s + weight_name = 'duration', + process_call_tagless_node = false, + u_turn_penalty = 20, + traffic_light_penalty = 7, -- seconds + use_turn_restrictions = true + }, + + classes = {"motorway", "toll", "TooWords2"}, + + excludable = { + {["motorway"] = true}, + {["toll"] = true}, + {["motorway"] = true, ["toll"] = true} + }, + + default_speed = 24, + speeds = { + primary = 36, + secondary = 18, + tertiary = 12, + steps = 6 + } + } +end + +function process_node (profile, node, result) + if (2 < nil) then + print("2 is less than nil") + end + + -- check if node is a traffic light + -- TODO: a way to set the penalty value +end + +function process_way (profile, way, result) + local highway = way:get_value_by_key("highway") + local toll = way:get_value_by_key("toll") + local name = way:get_value_by_key("name") + local oneway = way:get_value_by_key("oneway") + local route = way:get_value_by_key("route") + local duration = way:get_value_by_key("duration") + local maxspeed = tonumber(way:get_value_by_key ( "maxspeed")) + local maxspeed_forward = tonumber(way:get_value_by_key( "maxspeed:forward")) + local maxspeed_backward = tonumber(way:get_value_by_key( "maxspeed:backward")) + local junction = way:get_value_by_key("junction") + + if name then + result.name = name + end + + result.forward_mode = mode.driving + result.backward_mode = mode.driving + + if duration and durationIsValid(duration) then + result.duration = math.max( 1, parseDuration(duration) ) + result.forward_mode = mode.route + result.backward_mode = mode.route + else + local speed_forw = profile.speeds[highway] or profile.default_speed + local speed_back = speed_forw + + if highway == "river" then + local temp_speed = speed_forw + result.forward_mode = mode.river_down + result.backward_mode = mode.river_up + speed_forw = temp_speed*1.5 + speed_back = temp_speed/1.5 + elseif highway == "steps" then + result.forward_mode = mode.steps_down + result.backward_mode = mode.steps_up + end + + if maxspeed_forward ~= nil and maxspeed_forward > 0 then + speed_forw = maxspeed_forward + else + if maxspeed ~= nil and maxspeed > 0 and speed_forw > maxspeed then + speed_forw = maxspeed + end + end + + if maxspeed_backward ~= nil and maxspeed_backward > 0 then + speed_back = maxspeed_backward + else + if maxspeed ~=nil and maxspeed > 0 and speed_back > maxspeed then + speed_back = maxspeed + end + end + + result.forward_speed = speed_forw + result.backward_speed = speed_back + end + + if oneway == "no" or oneway == "0" or oneway == "false" then + -- nothing to do + elseif oneway == "-1" then + result.forward_mode = mode.inaccessible + elseif oneway == "yes" or oneway == "1" or oneway == "true" or junction == "roundabout" then + result.backward_mode = mode.inaccessible + end + + if highway == 'motorway' then + result.forward_classes["motorway"] = true + result.backward_classes["motorway"] = true + end + + if toll == "yes" then + result.forward_classes["toll"] = true + result.backward_classes["toll"] = true + end + + if junction == 'roundabout' then + result.roundabout = true + end +end + +function process_turn (profile, turn) + if turn.is_u_turn then + turn.duration = turn.duration + profile.properties.u_turn_penalty + turn.weight = turn.weight + profile.properties.u_turn_penalty + end + if turn.has_traffic_light then + turn.duration = turn.duration + profile.properties.traffic_light_penalty + end +end + +return { + setup = setup, + process_way = process_way, + process_node = process_node, + process_turn = process_turn +} diff --git a/test/data/profiles/bad_segment.lua b/test/data/profiles/bad_segment.lua new file mode 100644 index 000000000..cbb5bac19 --- /dev/null +++ b/test/data/profiles/bad_segment.lua @@ -0,0 +1,143 @@ +-- A copy of testbot with process_segment throwing a runtime error + +api_version = 4 + +function setup() + return { + properties = { + continue_straight_at_waypoint = true, + max_speed_for_map_matching = 30/3.6, --km -> m/s + weight_name = 'duration', + process_call_tagless_node = false, + u_turn_penalty = 20, + traffic_light_penalty = 7, -- seconds + use_turn_restrictions = true + }, + + classes = {"motorway", "toll", "TooWords2"}, + + excludable = { + {["motorway"] = true}, + {["toll"] = true}, + {["motorway"] = true, ["toll"] = true} + }, + + default_speed = 24, + speeds = { + primary = 36, + secondary = 18, + tertiary = 12, + steps = 6 + } + } +end + +function process_node (profile, node, result) + -- check if node is a traffic light + -- TODO: a way to set the penalty value +end + +function process_way (profile, way, result) + local highway = way:get_value_by_key("highway") + local toll = way:get_value_by_key("toll") + local name = way:get_value_by_key("name") + local oneway = way:get_value_by_key("oneway") + local route = way:get_value_by_key("route") + local duration = way:get_value_by_key("duration") + local maxspeed = tonumber(way:get_value_by_key ( "maxspeed")) + local maxspeed_forward = tonumber(way:get_value_by_key( "maxspeed:forward")) + local maxspeed_backward = tonumber(way:get_value_by_key( "maxspeed:backward")) + local junction = way:get_value_by_key("junction") + + if name then + result.name = name + end + + result.forward_mode = mode.driving + result.backward_mode = mode.driving + + if duration and durationIsValid(duration) then + result.duration = math.max( 1, parseDuration(duration) ) + result.forward_mode = mode.route + result.backward_mode = mode.route + else + local speed_forw = profile.speeds[highway] or profile.default_speed + local speed_back = speed_forw + + if highway == "river" then + local temp_speed = speed_forw + result.forward_mode = mode.river_down + result.backward_mode = mode.river_up + speed_forw = temp_speed*1.5 + speed_back = temp_speed/1.5 + elseif highway == "steps" then + result.forward_mode = mode.steps_down + result.backward_mode = mode.steps_up + end + + if maxspeed_forward ~= nil and maxspeed_forward > 0 then + speed_forw = maxspeed_forward + else + if maxspeed ~= nil and maxspeed > 0 and speed_forw > maxspeed then + speed_forw = maxspeed + end + end + + if maxspeed_backward ~= nil and maxspeed_backward > 0 then + speed_back = maxspeed_backward + else + if maxspeed ~=nil and maxspeed > 0 and speed_back > maxspeed then + speed_back = maxspeed + end + end + + result.forward_speed = speed_forw + result.backward_speed = speed_back + end + + if oneway == "no" or oneway == "0" or oneway == "false" then + -- nothing to do + elseif oneway == "-1" then + result.forward_mode = mode.inaccessible + elseif oneway == "yes" or oneway == "1" or oneway == "true" or junction == "roundabout" then + result.backward_mode = mode.inaccessible + end + + if highway == 'motorway' then + result.forward_classes["motorway"] = true + result.backward_classes["motorway"] = true + end + + if toll == "yes" then + result.forward_classes["toll"] = true + result.backward_classes["toll"] = true + end + + if junction == 'roundabout' then + result.roundabout = true + end +end + +function process_turn (profile, turn) + if turn.is_u_turn then + turn.duration = turn.duration + profile.properties.u_turn_penalty + turn.weight = turn.weight + profile.properties.u_turn_penalty + end + if turn.has_traffic_light then + turn.duration = turn.duration + profile.properties.traffic_light_penalty + end +end + +function process_segment (profile, segment) + if (2 < nil) then + print("2 is less than nil") + end +end + +return { + setup = setup, + process_way = process_way, + process_node = process_node, + process_turn = process_turn, + process_segment = process_segment +} diff --git a/test/data/profiles/bad_setup.lua b/test/data/profiles/bad_setup.lua new file mode 100644 index 000000000..72e32cc40 --- /dev/null +++ b/test/data/profiles/bad_setup.lua @@ -0,0 +1,140 @@ +-- Copy of testbot profile, with setup throwing a runtime error + +api_version = 4 + +function setup() + if (2 < nil) then -- arithmetic with nil should error + return {} + end + + return { + properties = { + continue_straight_at_waypoint = true, + max_speed_for_map_matching = 30/3.6, --km -> m/s + weight_name = 'duration', + process_call_tagless_node = false, + u_turn_penalty = 20, + traffic_light_penalty = 7, -- seconds + use_turn_restrictions = true + }, + + classes = {"motorway", "toll", "TooWords2"}, + + excludable = { + {["motorway"] = true}, + {["toll"] = true}, + {["motorway"] = true, ["toll"] = true} + }, + + default_speed = 24, + speeds = { + primary = 36, + secondary = 18, + tertiary = 12, + steps = 6 + } + } +end + +function process_node (profile, node, result) + -- check if node is a traffic light + -- TODO: a way to set the penalty value +end + +function process_way (profile, way, result) + local highway = way:get_value_by_key("highway") + local toll = way:get_value_by_key("toll") + local name = way:get_value_by_key("name") + local oneway = way:get_value_by_key("oneway") + local route = way:get_value_by_key("route") + local duration = way:get_value_by_key("duration") + local maxspeed = tonumber(way:get_value_by_key ( "maxspeed")) + local maxspeed_forward = tonumber(way:get_value_by_key( "maxspeed:forward")) + local maxspeed_backward = tonumber(way:get_value_by_key( "maxspeed:backward")) + local junction = way:get_value_by_key("junction") + + if name then + result.name = name + end + + result.forward_mode = mode.driving + result.backward_mode = mode.driving + + if duration and durationIsValid(duration) then + result.duration = math.max( 1, parseDuration(duration) ) + result.forward_mode = mode.route + result.backward_mode = mode.route + else + local speed_forw = profile.speeds[highway] or profile.default_speed + local speed_back = speed_forw + + if highway == "river" then + local temp_speed = speed_forw + result.forward_mode = mode.river_down + result.backward_mode = mode.river_up + speed_forw = temp_speed*1.5 + speed_back = temp_speed/1.5 + elseif highway == "steps" then + result.forward_mode = mode.steps_down + result.backward_mode = mode.steps_up + end + + if maxspeed_forward ~= nil and maxspeed_forward > 0 then + speed_forw = maxspeed_forward + else + if maxspeed ~= nil and maxspeed > 0 and speed_forw > maxspeed then + speed_forw = maxspeed + end + end + + if maxspeed_backward ~= nil and maxspeed_backward > 0 then + speed_back = maxspeed_backward + else + if maxspeed ~=nil and maxspeed > 0 and speed_back > maxspeed then + speed_back = maxspeed + end + end + + result.forward_speed = speed_forw + result.backward_speed = speed_back + end + + if oneway == "no" or oneway == "0" or oneway == "false" then + -- nothing to do + elseif oneway == "-1" then + result.forward_mode = mode.inaccessible + elseif oneway == "yes" or oneway == "1" or oneway == "true" or junction == "roundabout" then + result.backward_mode = mode.inaccessible + end + + if highway == 'motorway' then + result.forward_classes["motorway"] = true + result.backward_classes["motorway"] = true + end + + if toll == "yes" then + result.forward_classes["toll"] = true + result.backward_classes["toll"] = true + end + + if junction == 'roundabout' then + result.roundabout = true + end +end + +function process_turn (profile, turn) + if turn.is_u_turn then + turn.duration = turn.duration + profile.properties.u_turn_penalty + turn.weight = turn.weight + profile.properties.u_turn_penalty + end + if turn.has_traffic_light then + turn.duration = turn.duration + profile.properties.traffic_light_penalty + end +end + +return { + setup = setup, + process_way = process_way, + process_node = process_node, + process_turn = process_turn +} diff --git a/test/data/profiles/bad_turn.lua b/test/data/profiles/bad_turn.lua new file mode 100644 index 000000000..0fecb7d2d --- /dev/null +++ b/test/data/profiles/bad_turn.lua @@ -0,0 +1,140 @@ +-- a copy of testbot with process_turn throwing an error + +api_version = 4 + +function setup() + return { + properties = { + continue_straight_at_waypoint = true, + max_speed_for_map_matching = 30/3.6, --km -> m/s + weight_name = 'duration', + process_call_tagless_node = false, + u_turn_penalty = 20, + traffic_light_penalty = 7, -- seconds + use_turn_restrictions = true + }, + + classes = {"motorway", "toll", "TooWords2"}, + + excludable = { + {["motorway"] = true}, + {["toll"] = true}, + {["motorway"] = true, ["toll"] = true} + }, + + default_speed = 24, + speeds = { + primary = 36, + secondary = 18, + tertiary = 12, + steps = 6 + } + } +end + +function process_node (profile, node, result) + -- check if node is a traffic light + -- TODO: a way to set the penalty value +end + +function process_way (profile, way, result) + local highway = way:get_value_by_key("highway") + local toll = way:get_value_by_key("toll") + local name = way:get_value_by_key("name") + local oneway = way:get_value_by_key("oneway") + local route = way:get_value_by_key("route") + local duration = way:get_value_by_key("duration") + local maxspeed = tonumber(way:get_value_by_key ( "maxspeed")) + local maxspeed_forward = tonumber(way:get_value_by_key( "maxspeed:forward")) + local maxspeed_backward = tonumber(way:get_value_by_key( "maxspeed:backward")) + local junction = way:get_value_by_key("junction") + + if name then + result.name = name + end + + result.forward_mode = mode.driving + result.backward_mode = mode.driving + + if duration and durationIsValid(duration) then + result.duration = math.max( 1, parseDuration(duration) ) + result.forward_mode = mode.route + result.backward_mode = mode.route + else + local speed_forw = profile.speeds[highway] or profile.default_speed + local speed_back = speed_forw + + if highway == "river" then + local temp_speed = speed_forw + result.forward_mode = mode.river_down + result.backward_mode = mode.river_up + speed_forw = temp_speed*1.5 + speed_back = temp_speed/1.5 + elseif highway == "steps" then + result.forward_mode = mode.steps_down + result.backward_mode = mode.steps_up + end + + if maxspeed_forward ~= nil and maxspeed_forward > 0 then + speed_forw = maxspeed_forward + else + if maxspeed ~= nil and maxspeed > 0 and speed_forw > maxspeed then + speed_forw = maxspeed + end + end + + if maxspeed_backward ~= nil and maxspeed_backward > 0 then + speed_back = maxspeed_backward + else + if maxspeed ~=nil and maxspeed > 0 and speed_back > maxspeed then + speed_back = maxspeed + end + end + + result.forward_speed = speed_forw + result.backward_speed = speed_back + end + + if oneway == "no" or oneway == "0" or oneway == "false" then + -- nothing to do + elseif oneway == "-1" then + result.forward_mode = mode.inaccessible + elseif oneway == "yes" or oneway == "1" or oneway == "true" or junction == "roundabout" then + result.backward_mode = mode.inaccessible + end + + if highway == 'motorway' then + result.forward_classes["motorway"] = true + result.backward_classes["motorway"] = true + end + + if toll == "yes" then + result.forward_classes["toll"] = true + result.backward_classes["toll"] = true + end + + if junction == 'roundabout' then + result.roundabout = true + end +end + +function process_turn (profile, turn) + if (2 < nil) then + print("2 is less than nil") + end + + if turn.is_u_turn then + turn.duration = turn.duration + profile.properties.u_turn_penalty + turn.weight = turn.weight + profile.properties.u_turn_penalty + end + if turn.has_traffic_light then + turn.duration = turn.duration + profile.properties.traffic_light_penalty + end +end + +return { + setup = setup, + process_way = process_way, + process_node = process_node, + process_turn = process_turn +} diff --git a/test/data/profiles/bad_way.lua b/test/data/profiles/bad_way.lua new file mode 100644 index 000000000..cb0a3eb72 --- /dev/null +++ b/test/data/profiles/bad_way.lua @@ -0,0 +1,140 @@ +-- copy of testbot with process_way throwing a runtime error + +api_version = 4 + +function setup() + return { + properties = { + continue_straight_at_waypoint = true, + max_speed_for_map_matching = 30/3.6, --km -> m/s + weight_name = 'duration', + process_call_tagless_node = false, + u_turn_penalty = 20, + traffic_light_penalty = 7, -- seconds + use_turn_restrictions = true + }, + + classes = {"motorway", "toll", "TooWords2"}, + + excludable = { + {["motorway"] = true}, + {["toll"] = true}, + {["motorway"] = true, ["toll"] = true} + }, + + default_speed = 24, + speeds = { + primary = 36, + secondary = 18, + tertiary = 12, + steps = 6 + } + } +end + +function process_node (profile, node, result) + -- check if node is a traffic light + -- TODO: a way to set the penalty value +end + +function process_way (profile, way, result) + if (2 < nil) then + print("2 less than nil") + end + + local highway = way:get_value_by_key("highway") + local toll = way:get_value_by_key("toll") + local name = way:get_value_by_key("name") + local oneway = way:get_value_by_key("oneway") + local route = way:get_value_by_key("route") + local duration = way:get_value_by_key("duration") + local maxspeed = tonumber(way:get_value_by_key ( "maxspeed")) + local maxspeed_forward = tonumber(way:get_value_by_key( "maxspeed:forward")) + local maxspeed_backward = tonumber(way:get_value_by_key( "maxspeed:backward")) + local junction = way:get_value_by_key("junction") + + if name then + result.name = name + end + + result.forward_mode = mode.driving + result.backward_mode = mode.driving + + if duration and durationIsValid(duration) then + result.duration = math.max( 1, parseDuration(duration) ) + result.forward_mode = mode.route + result.backward_mode = mode.route + else + local speed_forw = profile.speeds[highway] or profile.default_speed + local speed_back = speed_forw + + if highway == "river" then + local temp_speed = speed_forw + result.forward_mode = mode.river_down + result.backward_mode = mode.river_up + speed_forw = temp_speed*1.5 + speed_back = temp_speed/1.5 + elseif highway == "steps" then + result.forward_mode = mode.steps_down + result.backward_mode = mode.steps_up + end + + if maxspeed_forward ~= nil and maxspeed_forward > 0 then + speed_forw = maxspeed_forward + else + if maxspeed ~= nil and maxspeed > 0 and speed_forw > maxspeed then + speed_forw = maxspeed + end + end + + if maxspeed_backward ~= nil and maxspeed_backward > 0 then + speed_back = maxspeed_backward + else + if maxspeed ~=nil and maxspeed > 0 and speed_back > maxspeed then + speed_back = maxspeed + end + end + + result.forward_speed = speed_forw + result.backward_speed = speed_back + end + + if oneway == "no" or oneway == "0" or oneway == "false" then + -- nothing to do + elseif oneway == "-1" then + result.forward_mode = mode.inaccessible + elseif oneway == "yes" or oneway == "1" or oneway == "true" or junction == "roundabout" then + result.backward_mode = mode.inaccessible + end + + if highway == 'motorway' then + result.forward_classes["motorway"] = true + result.backward_classes["motorway"] = true + end + + if toll == "yes" then + result.forward_classes["toll"] = true + result.backward_classes["toll"] = true + end + + if junction == 'roundabout' then + result.roundabout = true + end +end + +function process_turn (profile, turn) + if turn.is_u_turn then + turn.duration = turn.duration + profile.properties.u_turn_penalty + turn.weight = turn.weight + profile.properties.u_turn_penalty + end + if turn.has_traffic_light then + turn.duration = turn.duration + profile.properties.traffic_light_penalty + end +end + +return { + setup = setup, + process_way = process_way, + process_node = process_node, + process_turn = process_turn +} diff --git a/unit_tests/library/extract.cpp b/unit_tests/library/extract.cpp index aa18c7552..342d672f7 100644 --- a/unit_tests/library/extract.cpp +++ b/unit_tests/library/extract.cpp @@ -1,10 +1,32 @@ #include +#include "osrm/exception.hpp" #include "osrm/extractor.hpp" #include "osrm/extractor_config.hpp" +#include #include +// utility class to redirect stderr so we can test it +// inspired by https://stackoverflow.com/questions/5405016 +class redirect_stderr +{ + // constructor: accept a pointer to a buffer where stderr will be redirected + public: + redirect_stderr(std::streambuf *buf) + // store the original buffer for later (original buffer returned by rdbuf) + : old(std::cerr.rdbuf(buf)) + { + } + + // destructor: restore the original cerr, regardless of how this class gets destroyed + ~redirect_stderr() { std::cerr.rdbuf(old); } + + // place to store the buffer + private: + std::streambuf *old; +}; + BOOST_AUTO_TEST_SUITE(library_extract) BOOST_AUTO_TEST_CASE(test_extract_with_invalid_config) @@ -26,4 +48,114 @@ BOOST_AUTO_TEST_CASE(test_extract_with_valid_config) BOOST_CHECK_NO_THROW(osrm::extract(config)); } +BOOST_AUTO_TEST_CASE(test_setup_runtime_error) +{ + osrm::ExtractorConfig config; + config.input_path = OSRM_TEST_DATA_DIR "/monaco.osm.pbf"; + config.UseDefaultOutputNames(OSRM_TEST_DATA_DIR "/monaco.osm.pbf"); + config.profile_path = OSRM_TEST_DATA_DIR "/profiles/bad_setup.lua"; + config.small_component_size = 1000; + config.requested_num_threads = std::thread::hardware_concurrency(); + + std::stringstream output; + + { + redirect_stderr redir(output.rdbuf()); + BOOST_CHECK_THROW(osrm::extract(config), osrm::util::exception); + } + + // We just look for the line number, file name, and error message. This avoids portability + // issues since the output contains the full path to the file, which may change between systems + BOOST_CHECK(boost::algorithm::contains(output.str(), + "bad_setup.lua:6: attempt to compare number with nil")); +} + +BOOST_AUTO_TEST_CASE(test_way_runtime_error) +{ + osrm::ExtractorConfig config; + config.input_path = OSRM_TEST_DATA_DIR "/monaco.osm.pbf"; + config.UseDefaultOutputNames(OSRM_TEST_DATA_DIR "/monaco.osm.pbf"); + config.profile_path = OSRM_TEST_DATA_DIR "/profiles/bad_way.lua"; + config.small_component_size = 1000; + config.requested_num_threads = std::thread::hardware_concurrency(); + + std::stringstream output; + + { + redirect_stderr redir(output.rdbuf()); + BOOST_CHECK_THROW(osrm::extract(config), osrm::util::exception); + } + + // We just look for the line number, file name, and error message. This avoids portability + // issues since the output contains the full path to the file, which may change between systems + BOOST_CHECK(boost::algorithm::contains(output.str(), + "bad_way.lua:41: attempt to compare number with nil")); +} + +BOOST_AUTO_TEST_CASE(test_node_runtime_error) +{ + osrm::ExtractorConfig config; + config.input_path = OSRM_TEST_DATA_DIR "/monaco.osm.pbf"; + config.UseDefaultOutputNames(OSRM_TEST_DATA_DIR "/monaco.osm.pbf"); + config.profile_path = OSRM_TEST_DATA_DIR "/profiles/bad_node.lua"; + config.small_component_size = 1000; + config.requested_num_threads = std::thread::hardware_concurrency(); + + std::stringstream output; + + { + redirect_stderr redir(output.rdbuf()); + BOOST_CHECK_THROW(osrm::extract(config), osrm::util::exception); + } + + // We just look for the line number, file name, and error message. This avoids portability + // issues since the output contains the full path to the file, which may change between systems + BOOST_CHECK(boost::algorithm::contains(output.str(), + "bad_node.lua:36: attempt to compare number with nil")); +} + +BOOST_AUTO_TEST_CASE(test_segment_runtime_error) +{ + osrm::ExtractorConfig config; + config.input_path = OSRM_TEST_DATA_DIR "/monaco.osm.pbf"; + config.UseDefaultOutputNames(OSRM_TEST_DATA_DIR "/monaco.osm.pbf"); + config.profile_path = OSRM_TEST_DATA_DIR "/profiles/bad_segment.lua"; + config.small_component_size = 1000; + config.requested_num_threads = std::thread::hardware_concurrency(); + + std::stringstream output; + + { + redirect_stderr redir(output.rdbuf()); + BOOST_CHECK_THROW(osrm::extract(config), osrm::util::exception); + } + + // We just look for the line number, file name, and error message. This avoids portability + // issues since the output contains the full path to the file, which may change between systems + BOOST_CHECK(boost::algorithm::contains( + output.str(), "bad_segment.lua:132: attempt to compare number with nil")); +} + +BOOST_AUTO_TEST_CASE(test_turn_runtime_error) +{ + osrm::ExtractorConfig config; + config.input_path = OSRM_TEST_DATA_DIR "/monaco.osm.pbf"; + config.UseDefaultOutputNames(OSRM_TEST_DATA_DIR "/monaco.osm.pbf"); + config.profile_path = OSRM_TEST_DATA_DIR "/profiles/bad_turn.lua"; + config.small_component_size = 1000; + config.requested_num_threads = std::thread::hardware_concurrency(); + + std::stringstream output; + + { + redirect_stderr redir(output.rdbuf()); + BOOST_CHECK_THROW(osrm::extract(config), osrm::util::exception); + } + + // We just look for the line number, file name, and error message. This avoids portability + // issues since the output contains the full path to the file, which may change between systems + BOOST_CHECK(boost::algorithm::contains(output.str(), + "bad_turn.lua:122: attempt to compare number with nil")); +} + BOOST_AUTO_TEST_SUITE_END()