celtera / libremidi

A modern C++ MIDI 1 / MIDI 2 real-time & file I/O library. Supports Windows, macOS, Linux and WebMIDI.
Other
441 stars 48 forks source link

Error handling is buggy/badly documented #109

Open xouillet opened 4 months ago

xouillet commented 4 months ago

Hi,

I'm trying to understand how error handling is supposed to work for a basic function such as midi.open_port. Documentation is quite evasive and say that exception should be raised, but it doesn't.

For example the following code:

  libremidi::observer obs;
  auto inputs = obs.get_input_ports();
  libremidi::midi_in midi{
    libremidi::input_configuration{.on_message = midi_cb}
  };
  std::cout << "Trying to open port " << 1 << std::endl;
  midi.open_port(inputs[1]);

No exception is thrown if inputs[1] doesn't exists.

 midi.open_port(inputs[1]).throw_exception()

Throws an exception every time, even if the open succeed.

For now, I'm using (hacky ?)

  if (midi.open_port(inputs[1]) != stdx::error()) {
    throw std::runtime_error("Unable to open midi port");
  }

I don't know if it's the right way to do it, if so it would be nice to have it documented.

Thanks

jcelerier commented 4 months ago

Hello ! Are you using an official release or the latest master ? Exception handling changed in current master (and has pretty much been removed) - now the handling is done through error propagation with https://github.com/charles-salvia/std_error to enable the library to work in more environments (a future goal is having it work on OS-less embedded platforms and that's a preliminary step).

What's missing is certainly [[nodiscard]] annotations and a documentation update to indicate to the user that the error has to be handled - but over 10-ish years of working with this, the exception handling of the library has caused me much more problems than it solved.

xouillet commented 4 months ago

Hi,

Thanks for your response.

Yes, I'm using the current master. I now understand the external std::error implementation. I will now assume that the correct way to check a return code is the one I provided, for some reason the boolean cast of a stdx::error doesn't work so we have to check using a comparison to stdx:error()...

A documentation update would be nice indeed :)

Thanks