Fix uncaught util::exception#7324
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #7323 by adding catch blocks for util::exception across six tool files to prevent uncaught exceptions. The util::exception class is the base exception class for OSRM, and osrm::RuntimeError (which was already being caught) inherits from it. This fix ensures that any util::exception that is not a RuntimeError will now be properly caught and logged.
Changes:
- Added catch blocks for
util::exceptionin six tool executables (store, routed, partition, extract, customize, contract) - Added explicit include of
util/exception.hppin extract.cpp - All catch blocks log the error and return EXIT_FAILURE
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/store.cpp | Added util::exception catch block after RuntimeError handler |
| src/tools/routed.cpp | Added util::exception catch block after RuntimeError handler |
| src/tools/partition.cpp | Added util::exception catch block with memory dump after RuntimeError handler |
| src/tools/extract.cpp | Added util::exception catch block with memory dump and explicit include of util/exception.hpp |
| src/tools/customize.cpp | Added util::exception catch block with memory dump after RuntimeError handler |
| src/tools/contract.cpp | Added util::exception catch block with memory dump after RuntimeError handler |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return e.GetCode(); | ||
| } | ||
| catch (const util::exception &e) | ||
| { |
There was a problem hiding this comment.
The util::exception catch block should call util::DumpMemoryStats() for consistency with other tool files (extract.cpp, partition.cpp, customize.cpp, and contract.cpp). This helps with debugging memory-related issues when exceptions occur.
| { | |
| { | |
| util::DumpMemoryStats(); |
| #include "osrm/exception.hpp" | ||
| #include "osrm/extractor.hpp" | ||
| #include "osrm/extractor_config.hpp" | ||
| #include "util/exception.hpp" |
There was a problem hiding this comment.
This explicit include of "util/exception.hpp" is redundant since "osrm/exception.hpp" (included at line 1) already includes "util/exception.hpp". Consider removing this line for consistency with other tool files (store.cpp, routed.cpp, partition.cpp, customize.cpp, contract.cpp) which rely on the transitive include.
| #include "util/exception.hpp" |
| return e.GetCode(); | ||
| } | ||
| catch (const util::exception &e) | ||
| { |
There was a problem hiding this comment.
The util::exception catch block should call util::DumpMemoryStats() for consistency with other tool files (extract.cpp, partition.cpp, customize.cpp, and contract.cpp). This helps with debugging memory-related issues when exceptions occur.
| { | |
| { | |
| util::DumpMemoryStats(); |
Issue
Fixes #7323
util::exceptionis never caughtTasklist
Requirements / Relations
Link any requirements here. Other pull requests this PR is based on?