Missing OSPModelDescription.xml causes logic_error to be thrown

[kyllingstad]

If a user tries to use OSP-specific features like variable group connections in their OspSystemStructure.xml, but the required *_OspModelDescription.xml files are missing or not found, a std::logic_error will be thrown during parsing of the system structure file.

In other words, a user (runtime) error leads to an exception which signals a program (logic) error. Exceptions that represent runtime errors should always derive from std::runtime_error.

The source of the exception is get_emd() in src/cosim/osp_config_parser.cpp, which throws std::out_of_range, a subclass of std::logic_error. I'd say this is a slight misuse of this exception class.

Example of someone encountering this in the wild: open-simulation-platform/cosim-cli#115

[restenb]

Should then all instances of user misconfiguration lead to std::runtime_error? In addition to std::out_of_range, std::invalid_argument is also used a few times in osp_config_parser, such as when encountering an invalid step size.

[kyllingstad]

This is what I get for oversimplifying a complicated issue: difficult questions in return. :D It is especially complicated in the case of a library, because who is really the "user" here? Is it the end application user or the developer which develops something based on libcosim?

The point is that a std::logic_error-derived exception signals – as the name implies – errors in the program logic, aka. bugs. Arguably (and people will disagree with this), in a correctly written program, no std::logic_error should ever be thrown.

An error which occurs at runtime and could not have been anticipated, for example that a file gets deleted just as we were about to read it, or that a user misconfigured the software, should generally cause a std::runtime_error-derived exception.

It gets complicated, because in a low-level function you usually don't know where your input comes from.

void processThis(const std::string& nonEmptyString) {
    if (nonEmptyString.empty()) throw std::invalid_argument("I told you the string should be non-empty!");
    // ...
}

Here, the string could come from end-user input, but the function doesn't know this. Throwing a std::invalid_argument here indicates that the programmer using the function is responsible for checking the input for validity before passing it on.

Anyway, in the case of osp_config_parser, it seems clear to me. We have one public API function in that module:

osp_config load_osp_config(
    const cosim::filesystem::path& configPath,
    cosim::model_uri_resolver& resolver);

It is completely fair to throw, say, std::invalid_argument if the application developer (i.e., immediate libcosim user) passes an empty configPath here. That is just a bug, and easily avoided. But the XML files pointed to by configPath will not be under the developer's control, they will come from end users, and we cannot expect the developer to pre-validate them. Errors that occur during the parsing of those files should never cause a std::logic_error to escape out of this function.

[restenb]

And I take the answer to the question as "yes, please" 😀

Since I've been going over osp_config_parser lately anyway with the seaco work, I might as well fling up a quick PR to address this.