Make yabridge.toml parsing failures non-fatal

CHANGELOG.md

@@ -8,6 +8,12 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Changed
+
+- Parsing failures for `yabridge.toml` files will no longer cause plugins to
+  fail to load. If that happens, you'll now get a desktop notification and the
+  plugin will use the default settings instead.
+
 ### yabridgectl
 
 - Some outdated warning messages have been updated to make yabridge's current

src/common/configuration.cpp

@@ -29,7 +29,7 @@ Configuration::Configuration() noexcept {}
 Configuration::Configuration(const fs::path& config_path,
                              const fs::path& yabridge_path)
     : Configuration() {
-    // Will throw a `toml::parsing_error` if the file cannot be parsed. Better
+    // Will throw a `toml::parse_error` if the file cannot be parsed. Better
     // to throw here rather than failing silently since syntax errors would
     // otherwise be impossible to spot. We'll also have to sort all tables by
     // the location in the file since tomlplusplus internally uses ordered maps

src/common/configuration.h

@@ -30,9 +30,9 @@
  * plugins that will be hosted in the same process rather than individually so
  * they can share resources. Configuration file loading works as follows:
  *
- * 1. `load_config_for(path)` from `src/plugin/utils.h` gets called with a path
- *    to the copy of or symlink to `libyabridge-{clap,vst2,vst3}.so` that the
- *    plugin host has tried to load.
+ * 1. `load_config_for(path, ...)` from `src/plugin/utils.h` gets called with a
+ *    path to the copy of or symlink to `libyabridge-{clap,vst2,vst3}.so` that
+ *    the plugin host has tried to load.
  * 2. We start looking for a file named `yabridge.toml` in the same directory as
  *    that `.so` file, iteratively continuing to search one directory higher
  *    until we either find the file or we reach the filesystem root.
@@ -64,7 +64,7 @@ class Configuration {
      * configuration file. Will leave the object empty if the plugin cannot be
      * matched to any of the patterns. Not meant to be used directly.
      *
-     * @throw toml::parsing_error If the file could not be parsed.
+     * @throw toml::parse_error If the file could not be parsed.
      *
      * @see ../plugin/utils.h:load_config_for
      */

src/plugin/bridges/common.h

@@ -81,14 +81,14 @@ class PluginBridge {
     PluginBridge(PluginType plugin_type,
                  const ghc::filesystem::path& plugin_path,
                  F&& create_socket_instance)
-        // This is still correct for VST3 plugins because we can configure an
-        // entire directory (the module's bundle) at once
-        : config_(load_config_for(plugin_path)),
-          info_(plugin_type, plugin_path, config_.vst3_prefer_32bit),
-          io_context_(),
+        : io_context_(),
           sockets_(create_socket_instance(io_context_, info_)),
           generic_logger_(Logger::create_from_environment(
               create_logger_prefix(sockets_.base_dir_))),
+          // This works for both individual files (VST2 and CLAP) and entire
+          // directories (VST3)
+          config_(load_config_for(plugin_path, generic_logger_)),
+          info_(plugin_type, plugin_path, config_.vst3_prefer_32bit),
           plugin_host_(
               config_.group
                   ? std::unique_ptr<HostProcess>(std::make_unique<GroupHost>(
@@ -438,19 +438,6 @@ class PluginBridge {
         }
     }
 
-    /**
-     * The configuration for this instance of yabridge. Set based on the values
-     * from a `yabridge.toml`, if it exists.
-     *
-     * @see ../utils.h:load_config_for
-     */
-    Configuration config_;
-
-    /**
-     * Information about the plugin we're bridging.
-     */
-    const PluginInfo info_;
-
     asio::io_context io_context_;
 
     /**
@@ -471,6 +458,19 @@ class PluginBridge {
      */
     Logger generic_logger_;
 
+    /**
+     * The configuration for this instance of yabridge. Set based on the values
+     * from a `yabridge.toml`, if it exists.
+     *
+     * @see ../utils.h:load_config_for
+     */
+    Configuration config_;
+
+    /**
+     * Information about the plugin we're bridging.
+     */
+    const PluginInfo info_;
+
     /**
      * The Wine process hosting our plugins. In the case of group hosts a
      * `PluginBridge` instance doesn't actually own a process, but rather either

src/plugin/utils.cpp

@@ -23,6 +23,8 @@
 #include <config.h>
 
 #include "../common/configuration.h"
+#include "../common/notifications.h"
+#include "../common/toml++.h"
 #include "../common/utils.h"
 
 namespace fs = ghc::filesystem;
@@ -396,7 +398,7 @@ ghc::filesystem::path generate_group_endpoint(
     return get_temporary_directory() / socket_name.str();
 }
 
-Configuration load_config_for(const fs::path& yabridge_path) {
+Configuration load_config_for(const fs::path& yabridge_path, Logger& logger) {
     // First find the closest `yabridge.tmol` file for the plugin, falling back
     // to default configuration settings if it doesn't exist
     const std::optional<fs::path> config_file =
@@ -405,5 +407,25 @@ Configuration load_config_for(const fs::path& yabridge_path) {
         return Configuration();
     }
 
-    return Configuration(*config_file, yabridge_path);
+    try {
+        return Configuration(*config_file, yabridge_path);
+    } catch (const toml::parse_error& error) {
+        // Parsing failures should be non-fatal since that leads to a pretty
+        // confusing user experience (see
+        // https://github.com/robbert-vdh/yabridge/issues/282). They should,
+        // however, still result in a visible error.
+        logger.log("");
+        logger.log("The configuration file at '" + config_file->string() +
+                   "' could not be parsed:");
+        logger.log(error.what());
+        logger.log("");
+
+        send_notification(
+            "Failed to parse yabridge.toml file",
+            "The configuration file at '" + config_file->string() +
+                "' could not be parsed: " + std::string(error.description()),
+            std::nullopt);
+
+        return Configuration();
+    }
 }

src/plugin/utils.h

@@ -19,6 +19,7 @@
 #include <variant>
 
 #include "../common/configuration.h"
+#include "../common/logging/common.h"
 #include "../common/plugins.h"
 #include "../common/process.h"
 #include "../common/utils.h"
@@ -238,12 +239,17 @@ ghc::filesystem::path generate_group_endpoint(
  * See the docstrong on the `Configuration` class for more details on how to
  * choose the config file to load.
  *
- * This function will take any optional compile-time features that have not been
- * enabled into account.
+ * If the configuration file has syntax errors, then an error will be logged and
+ * the defaults will be used instead.
+ *
+ * This function will also take any optional compile-time features that have not
+ * been enabled into account.
  *
  * @param yabridge_path The path to the .so file that's being loaded.by the VST
  *   host. This will be used both for the starting location of the search and to
  *   determine which section in the config file to use.
+ * @param logger The logger used to log parsing errors to. Parsing errors are
+ *   non-fatal, but they should still be very visible.
  *
  * @return Either a configuration object populated with values from matched glob
  *   pattern within the found configuration file, or an empty configuration
@@ -252,7 +258,8 @@ ghc::filesystem::path generate_group_endpoint(
  *
  * @see Configuration
  */
-Configuration load_config_for(const ghc::filesystem::path& yabridge_path);
+Configuration load_config_for(const ghc::filesystem::path& yabridge_path,
+                              Logger& logger);
 
 /**
  * Starting from the starting file or directory, go up in the directory