diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d27c7d65..1d2699618 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - `osrm-partition` now ensures it is called before `osrm-contract` and removes inconsitent .hsgr files automatically. - Features - Added conditional restriction support with `parse-conditional-restrictions=true|false` to osrm-extract. This option saves conditional turn restrictions to the .restrictions file for parsing by contract later. Added `parse-conditionals-from-now=utc time stamp` and `--time-zone-file=/path/to/file` to osrm-contract + - Command-line tools (osrm-extract, osrm-contract, osrm-routed, etc) now return error codes and legible error messages for common problem scenarios, rather than ugly C++ crashes - Files - .osrm.nodes file was renamed to .nbg_nodes and .ebg_nodes was added - Guidance diff --git a/CMakeLists.txt b/CMakeLists.txt index a2ae97db9..9b3436e86 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -126,6 +126,7 @@ file(GLOB UpdaterGlob src/updater/*.cpp) file(GLOB StorageGlob src/storage/*.cpp) file(GLOB ServerGlob src/server/*.cpp src/server/**/*.cpp) file(GLOB EngineGlob src/engine/*.cpp src/engine/**/*.cpp) +file(GLOB ErrorcodesGlob src/osrm/errorcodes.cpp) add_library(UTIL OBJECT ${UtilGlob}) add_library(EXTRACTOR OBJECT ${ExtractorGlob}) diff --git a/include/osrm/error_codes.hpp b/include/osrm/error_codes.hpp new file mode 100644 index 000000000..0891cc7bc --- /dev/null +++ b/include/osrm/error_codes.hpp @@ -0,0 +1,37 @@ +#ifndef OSRM_ERRORCODES_HPP +#define OSRM_ERRORCODES_HPP + +#include + +namespace osrm +{ + +/** + * Various error codes that can be returned by OSRM internal functions. + * Note: often, these translate into return codes from `int main()` functions. + * Thus, do not change the order - if adding new codes, append them to the + * end, so the code values do not change for users that are checking for + * certain values. + */ +enum ErrorCode +{ + InvalidFingerprint = 2, // Start at 2 to avoid colliding with POSIX EXIT_FAILURE + IncompatibleFileVersion, + FileOpenError, + FileReadError, + FileWriteError, + FileIOError, + UnexpectedEndOfFile, + IncompatibleDataset, + UnknownAlgorithm +#ifndef NDEBUG + // Leave this at the end. In debug mode, we assert that the size of + // this enum matches the number of messages we have documented, and __ENDMARKER__ + // is used as the "count" value. + , + __ENDMARKER__ +#endif +}; +} + +#endif // OSRM_ERRORCODES_HPP \ No newline at end of file diff --git a/include/osrm/exception.hpp b/include/osrm/exception.hpp index 42548f5d4..e45df8aa9 100644 --- a/include/osrm/exception.hpp +++ b/include/osrm/exception.hpp @@ -33,6 +33,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace osrm { using util::exception; +using util::RuntimeError; } #endif diff --git a/include/storage/io.hpp b/include/storage/io.hpp index 2c92f075b..9a0853ebe 100644 --- a/include/storage/io.hpp +++ b/include/storage/io.hpp @@ -1,6 +1,8 @@ #ifndef OSRM_STORAGE_IO_HPP_ #define OSRM_STORAGE_IO_HPP_ +#include "osrm/error_codes.hpp" + #include "util/exception.hpp" #include "util/exception_utils.hpp" #include "util/fingerprint.hpp" @@ -10,6 +12,8 @@ #include #include +#include +#include #include #include #include @@ -49,12 +53,16 @@ class FileReader : filepath(filepath_), fingerprint(flag) { input_stream.open(filepath, std::ios::binary); + + // Note: filepath.string() is wrapped in std::string() because it can + // return char * on some platforms, which makes the + operator not work if (!input_stream) - throw util::exception("Error opening " + filepath.string()); + throw util::RuntimeError( + filepath.string(), ErrorCode::FileOpenError, SOURCE_REF, std::strerror(errno)); if (flag == VerifyFingerprint && !ReadAndCheckFingerprint()) { - throw util::exception("Fingerprint mismatch in " + filepath_.string() + SOURCE_REF); + throw util::RuntimeError(filepath.string(), ErrorCode::InvalidFingerprint, SOURCE_REF); } } @@ -66,7 +74,11 @@ class FileReader if (file_size == boost::filesystem::ifstream::pos_type(-1)) { - throw util::exception("File size for " + filepath.string() + " failed " + SOURCE_REF); + throw util::RuntimeError("Unable to determine file size for " + + std::string(filepath.string()), + ErrorCode::FileIOError, + SOURCE_REF, + std::strerror(errno)); } // restore the current position @@ -101,10 +113,11 @@ class FileReader { if (result.eof()) { - throw util::exception("Error reading from " + filepath.string() + - ": Unexpected end of file " + SOURCE_REF); + throw util::RuntimeError( + filepath.string(), ErrorCode::UnexpectedEndOfFile, SOURCE_REF); } - throw util::exception("Error reading from " + filepath.string() + " " + SOURCE_REF); + throw util::RuntimeError( + filepath.string(), ErrorCode::FileReadError, SOURCE_REF, std::strerror(errno)); } } @@ -145,23 +158,19 @@ class FileReader if (!loaded_fingerprint.IsValid()) { - util::Log(logERROR) << "Fingerprint magic number or checksum is invalid in " - << filepath.string(); - return false; + throw util::RuntimeError(filepath.string(), ErrorCode::InvalidFingerprint, SOURCE_REF); } if (!expected_fingerprint.IsDataCompatible(loaded_fingerprint)) { - util::Log(logERROR) << filepath.string() - << " is not compatible with this version of OSRM"; - - util::Log(logERROR) << "It was prepared with OSRM " - << loaded_fingerprint.GetMajorVersion() << "." - << loaded_fingerprint.GetMinorVersion() << "." - << loaded_fingerprint.GetPatchVersion() << " but you are running " - << OSRM_VERSION; - util::Log(logERROR) << "Data is only compatible between minor releases."; - return false; + const std::string fileversion = + std::to_string(loaded_fingerprint.GetMajorVersion()) + "." + + std::to_string(loaded_fingerprint.GetMinorVersion()) + "." + + std::to_string(loaded_fingerprint.GetPatchVersion()); + throw util::RuntimeError(std::string(filepath.string()) + " prepared with OSRM " + + fileversion + " but this is " + OSRM_VERSION, + ErrorCode::IncompatibleFileVersion, + SOURCE_REF); } return true; @@ -192,7 +201,10 @@ class FileWriter { output_stream.open(filepath, std::ios::binary); if (!output_stream) - throw util::exception("Error opening " + filepath.string()); + { + throw util::RuntimeError( + filepath.string(), ErrorCode::FileOpenError, SOURCE_REF, std::strerror(errno)); + } if (flag == GenerateFingerprint) { @@ -216,7 +228,8 @@ class FileWriter if (!result) { - throw util::exception("Error writing to " + filepath.string()); + throw util::RuntimeError( + filepath.string(), ErrorCode::FileWriteError, SOURCE_REF, std::strerror(errno)); } } diff --git a/include/util/exception.hpp b/include/util/exception.hpp index 5ba9fc567..55a0a5428 100644 --- a/include/util/exception.hpp +++ b/include/util/exception.hpp @@ -28,10 +28,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef OSRM_EXCEPTION_HPP #define OSRM_EXCEPTION_HPP +#include #include +#include #include #include +#include "osrm/error_codes.hpp" #include namespace osrm @@ -39,7 +42,7 @@ namespace osrm namespace util { -class exception final : public std::exception +class exception : public std::exception { public: explicit exception(const char *message) : message(message) {} @@ -54,6 +57,72 @@ class exception final : public std::exception virtual void anchor() const; const std::string message; }; + +/** + * Indicates a class of error that occurred that was caused by some kind of + * external input (common examples are out of disk space, file permission errors, + * user supplied bad data, etc). + */ + +constexpr const std::array ErrorDescriptions = {{ + "", // Dummy - ErrorCode values start at 2 + "", // Dummy - ErrorCode values start at 2 + "Fingerprint did not match the expected value", // InvalidFingerprint + "File is incompatible with this version of OSRM", // IncompatibleFileVersion + "Problem opening file", // FileOpenError + "Problem reading from file", // FileReadError + "Problem writing to file", // FileWriteError + "I/O error occurred", // FileIOError + "Unexpected end of file", // UnexpectedEndOfFile + "The dataset you are trying to load is not " // IncompatibleDataset + "compatible with the routing algorithm you want to use." // ...continued... + "Incompatible algorithm" // IncompatibleAlgorithm +}}; + +#ifndef NDEBUG +// Check that we have messages for every enum +static_assert(ErrorDescriptions.size() == ErrorCode::__ENDMARKER__, + "ErrorCode list and ErrorDescription lists are different sizes"); +#endif + +class RuntimeError : public exception +{ + using Base = exception; + using Base::Base; + + public: + explicit RuntimeError(const std::string &message, + const ErrorCode code_, + const std::string &sourceref, + const char *possiblecause = nullptr) + : Base(BuildMessage(message, code_, sourceref, possiblecause)), code(code_) + { + } + + ErrorCode GetCode() const { return code; } + + private: + // This function exists to 'anchor' the class, and stop the compiler from + // copying vtable and RTTI info into every object file that includes + // this header. (Caught by -Wweak-vtables under Clang.) + virtual void anchor() const; + const ErrorCode code; + + static std::string BuildMessage(const std::string &message, + const ErrorCode code_, + const std::string &sourceref, + const char *possiblecause = nullptr) + { + std::string result; + result += ErrorDescriptions[code_]; + result += ": " + std::string(message); + result += possiblecause != nullptr + ? (std::string(" (possible cause: \"") + possiblecause + "\")") + : ""; + result += " (at " + sourceref + ")"; + return result; + } +}; } } diff --git a/include/util/exception_utils.hpp b/include/util/exception_utils.hpp index 1ae007851..e933fcb4e 100644 --- a/include/util/exception_utils.hpp +++ b/include/util/exception_utils.hpp @@ -10,6 +10,6 @@ #define OSRM_SOURCE_FILE_ PROJECT_RELATIVE_PATH_(__FILE__) // This is the macro to use -#define SOURCE_REF std::string(" (at ") + OSRM_SOURCE_FILE_ + ":" + std::to_string(__LINE__) + ")" +#define SOURCE_REF OSRM_SOURCE_FILE_ + ":" + std::to_string(__LINE__) #endif // SOURCE_MACROS_HPP diff --git a/src/extractor/raster_source.cpp b/src/extractor/raster_source.cpp index 03fffad43..d5be13f14 100644 --- a/src/extractor/raster_source.cpp +++ b/src/extractor/raster_source.cpp @@ -108,7 +108,8 @@ int SourceContainer::LoadRasterSource(const std::string &path_string, boost::filesystem::path filepath(path_string); if (!boost::filesystem::exists(filepath)) { - throw util::exception(path_string + " does not exist" + SOURCE_REF); + throw util::RuntimeError( + path_string, ErrorCode::FileOpenError, SOURCE_REF, "File not found"); } RasterGrid rasterData{filepath, ncols, nrows}; diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index 261ec2c79..ba401136d 100644 --- a/src/osrm/osrm.cpp +++ b/src/osrm/osrm.cpp @@ -36,7 +36,9 @@ OSRM::OSRM(engine::EngineConfig &config) // throw error if dataset is not usable with CoreCH if (config.algorithm == EngineConfig::Algorithm::CoreCH && !corech_compatible) { - throw util::exception("Dataset is not compatible with CoreCH."); + throw util::RuntimeError("Dataset is not compatible with CoreCH.", + ErrorCode::IncompatibleDataset, + SOURCE_REF); } } diff --git a/src/tools/contract.cpp b/src/tools/contract.cpp index c1107f279..13cbd63dd 100644 --- a/src/tools/contract.cpp +++ b/src/tools/contract.cpp @@ -1,6 +1,7 @@ #include "storage/io.hpp" #include "osrm/contractor.hpp" #include "osrm/contractor_config.hpp" +#include "osrm/exception.hpp" #include "util/log.hpp" #include "util/timezones.hpp" #include "util/version.hpp" @@ -187,9 +188,18 @@ int main(int argc, char *argv[]) try return EXIT_SUCCESS; } +catch (const osrm::RuntimeError &e) +{ + util::DumpSTXXLStats(); + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); + return e.GetCode(); +} catch (const std::bad_alloc &e) { - util::Log(logERROR) << "[exception] " << e.what(); + util::DumpSTXXLStats(); + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); util::Log(logERROR) << "Please provide more memory or consider using a larger swapfile"; return EXIT_FAILURE; } diff --git a/src/tools/customize.cpp b/src/tools/customize.cpp index ba7801bc2..66c019f3a 100644 --- a/src/tools/customize.cpp +++ b/src/tools/customize.cpp @@ -1,5 +1,6 @@ #include "customizer/customizer.hpp" +#include "osrm/exception.hpp" #include "util/log.hpp" #include "util/meminfo.hpp" #include "util/version.hpp" @@ -167,8 +168,15 @@ int main(int argc, char *argv[]) try return exitcode; } +catch (const osrm::RuntimeError &e) +{ + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); + return e.GetCode(); +} catch (const std::bad_alloc &e) { + util::DumpMemoryStats(); util::Log(logERROR) << "[exception] " << e.what(); util::Log(logERROR) << "Please provide more memory or consider using a larger swapfile"; return EXIT_FAILURE; diff --git a/src/tools/extract.cpp b/src/tools/extract.cpp index 22d6edff4..1332566db 100644 --- a/src/tools/extract.cpp +++ b/src/tools/extract.cpp @@ -1,3 +1,4 @@ +#include "osrm/exception.hpp" #include "osrm/extractor.hpp" #include "osrm/extractor_config.hpp" #include "util/log.hpp" @@ -168,8 +169,24 @@ int main(int argc, char *argv[]) try return EXIT_SUCCESS; } +catch (const osrm::RuntimeError &e) +{ + util::DumpSTXXLStats(); + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); + return e.GetCode(); +} +catch (const std::system_error &e) +{ + util::DumpSTXXLStats(); + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); + return e.code().value(); +} catch (const std::bad_alloc &e) { + util::DumpSTXXLStats(); + util::DumpMemoryStats(); util::Log(logERROR) << "[exception] " << e.what(); util::Log(logERROR) << "Please provide more memory or consider using a larger swapfile"; return EXIT_FAILURE; diff --git a/src/tools/partition.cpp b/src/tools/partition.cpp index 600bac561..eaaf716a7 100644 --- a/src/tools/partition.cpp +++ b/src/tools/partition.cpp @@ -1,6 +1,7 @@ #include "partition/partition_config.hpp" #include "partition/partitioner.hpp" +#include "osrm/exception.hpp" #include "util/log.hpp" #include "util/meminfo.hpp" #include "util/timing_util.hpp" @@ -238,8 +239,16 @@ int main(int argc, char *argv[]) try return exitcode; } +catch (const osrm::RuntimeError &e) +{ + util::DumpMemoryStats(); + util::Log(logERROR) << e.what(); + return EXIT_FAILURE; + return e.GetCode(); +} catch (const std::bad_alloc &e) { + util::DumpMemoryStats(); util::Log(logERROR) << "[exception] " << e.what(); util::Log(logERROR) << "Please provide more memory or consider using a larger swapfile"; return EXIT_FAILURE; diff --git a/src/tools/routed.cpp b/src/tools/routed.cpp index e2510c5a2..5fea572eb 100644 --- a/src/tools/routed.cpp +++ b/src/tools/routed.cpp @@ -1,9 +1,11 @@ #include "server/server.hpp" -#include "util/exception.hpp" +#include "util/exception_utils.hpp" #include "util/log.hpp" +#include "util/meminfo.hpp" #include "util/version.hpp" #include "osrm/engine_config.hpp" +#include "osrm/exception.hpp" #include "osrm/osrm.hpp" #include "osrm/storage_config.hpp" @@ -57,7 +59,7 @@ EngineConfig::Algorithm stringToAlgorithm(const std::string &algorithm) return EngineConfig::Algorithm::CoreCH; if (algorithm == "MLD") return EngineConfig::Algorithm::MLD; - throw util::exception("Invalid algorithm name: " + algorithm); + throw util::RuntimeError(algorithm, ErrorCode::UnknownAlgorithm, SOURCE_REF); } // generate boost::program_options object for the routing part @@ -331,8 +333,14 @@ int main(int argc, const char *argv[]) try routing_server.reset(); util::Log() << "shutdown completed"; } +catch (const osrm::RuntimeError &e) +{ + util::Log(logERROR) << e.what(); + return e.GetCode(); +} catch (const std::bad_alloc &e) { + util::DumpMemoryStats(); util::Log(logWARNING) << "[exception] " << e.what(); util::Log(logWARNING) << "Please provide more memory or consider using a larger swapfile"; return EXIT_FAILURE; diff --git a/src/tools/store.cpp b/src/tools/store.cpp index 08f570259..d6e639d2d 100644 --- a/src/tools/store.cpp +++ b/src/tools/store.cpp @@ -1,8 +1,9 @@ #include "storage/shared_memory.hpp" #include "storage/shared_monitor.hpp" #include "storage/storage.hpp" -#include "util/exception.hpp" +#include "osrm/exception.hpp" #include "util/log.hpp" +#include "util/meminfo.hpp" #include "util/typedefs.hpp" #include "util/version.hpp" @@ -172,8 +173,14 @@ int main(const int argc, const char *argv[]) try return storage.Run(max_wait); } +catch (const osrm::RuntimeError &e) +{ + util::Log(logERROR) << e.what(); + return e.GetCode(); +} catch (const std::bad_alloc &e) { + util::DumpMemoryStats(); util::Log(logERROR) << "[exception] " << e.what(); util::Log(logERROR) << "Please provide more memory or disable locking the virtual " "address space (note: this makes OSRM swap, i.e. slow)"; diff --git a/src/util/exception.cpp b/src/util/exception.cpp index 394d8c07c..8e7f4d184 100644 --- a/src/util/exception.cpp +++ b/src/util/exception.cpp @@ -17,5 +17,6 @@ namespace util { void exception::anchor() const {} +void RuntimeError::anchor() const {} } } diff --git a/test/nodejs/index.js b/test/nodejs/index.js index 7c82e3a05..c21b16b3a 100644 --- a/test/nodejs/index.js +++ b/test/nodejs/index.js @@ -25,7 +25,7 @@ test('constructor: does not accept more than one parameter', function(assert) { test('constructor: throws if necessary files do not exist', function(assert) { assert.plan(1); assert.throws(function() { new OSRM("missing.osrm"); }, - /Error opening missing.osrm.names/); + /Problem opening file: missing.osrm.names/); }); test('constructor: takes a shared memory argument', function(assert) { diff --git a/unit_tests/util/io.cpp b/unit_tests/util/io.cpp index ac94da34b..e14f960e0 100644 --- a/unit_tests/util/io.cpp +++ b/unit_tests/util/io.cpp @@ -49,11 +49,12 @@ BOOST_AUTO_TEST_CASE(io_nonexistent_file) osrm::storage::io::FileReader::VerifyFingerprint); BOOST_REQUIRE_MESSAGE(false, "Should not get here"); } - catch (const osrm::util::exception &e) + catch (const osrm::util::RuntimeError &e) { - const std::string expected("Error opening non_existent_test_io.tmp"); + const std::string expected("Problem opening file: " + IO_NONEXISTENT_FILE); const std::string got(e.what()); BOOST_REQUIRE(std::equal(expected.begin(), expected.end(), got.begin())); + BOOST_REQUIRE(e.GetCode() == osrm::ErrorCode::FileOpenError); } } @@ -83,12 +84,12 @@ BOOST_AUTO_TEST_CASE(file_too_small) osrm::storage::serialization::read(infile, buffer); BOOST_REQUIRE_MESSAGE(false, "Should not get here"); } - catch (const osrm::util::exception &e) + catch (const osrm::util::RuntimeError &e) { - const std::string expected( - "Error reading from file_too_small_test_io.tmp: Unexpected end of file"); + const std::string expected("Unexpected end of file: " + IO_TOO_SMALL_FILE); const std::string got(e.what()); BOOST_REQUIRE(std::equal(expected.begin(), expected.end(), got.begin())); + BOOST_REQUIRE(e.GetCode() == osrm::ErrorCode::UnexpectedEndOfFile); } } @@ -111,11 +112,13 @@ BOOST_AUTO_TEST_CASE(io_corrupt_fingerprint) osrm::storage::io::FileReader::VerifyFingerprint); BOOST_REQUIRE_MESSAGE(false, "Should not get here"); } - catch (const osrm::util::exception &e) + catch (const osrm::util::RuntimeError &e) { - const std::string expected("Fingerprint mismatch in corrupt_fingerprint_file_test_io.tmp"); + const std::string expected("Fingerprint did not match the expected value: " + + IO_CORRUPT_FINGERPRINT_FILE); const std::string got(e.what()); BOOST_REQUIRE(std::equal(expected.begin(), expected.end(), got.begin())); + BOOST_REQUIRE(e.GetCode() == osrm::ErrorCode::InvalidFingerprint); } } @@ -146,11 +149,13 @@ BOOST_AUTO_TEST_CASE(io_incompatible_fingerprint) osrm::storage::io::FileReader::VerifyFingerprint); BOOST_REQUIRE_MESSAGE(false, "Should not get here"); } - catch (const osrm::util::exception &e) + catch (const osrm::util::RuntimeError &e) { - const std::string expected("Fingerprint mismatch in " + IO_INCOMPATIBLE_FINGERPRINT_FILE); + const std::string expected("Fingerprint did not match the expected value: " + + IO_INCOMPATIBLE_FINGERPRINT_FILE); const std::string got(e.what()); BOOST_REQUIRE(std::equal(expected.begin(), expected.end(), got.begin())); + BOOST_REQUIRE(e.GetCode() == osrm::ErrorCode::InvalidFingerprint); } }