Add serialization error handling to settings projection layer

src/cascadia/TerminalApp/ActionAndArgs.cpp

@@ -105,7 +105,7 @@ namespace winrt::TerminalApp::implementation
         { UnboundKey, ShortcutAction::Invalid },
     };
 
-    using ParseResult = std::tuple<IActionArgs, std::vector<::TerminalApp::SettingsLoadWarnings>>;
+    using ParseResult = std::tuple<IActionArgs, std::vector<TerminalApp::SettingsLoadWarnings>>;
     using ParseActionFunction = std::function<ParseResult(const Json::Value&)>;
 
     // This is a map of ShortcutAction->function<IActionArgs(Json::Value)>. It holds
@@ -169,7 +169,7 @@ namespace winrt::TerminalApp::implementation
     // - a deserialized ActionAndArgs corresponding to the values in json, or
     //   null if we failed to deserialize an action.
     winrt::com_ptr<ActionAndArgs> ActionAndArgs::FromJson(const Json::Value& json,
-                                                          std::vector<::TerminalApp::SettingsLoadWarnings>& warnings)
+                                                          std::vector<TerminalApp::SettingsLoadWarnings>& warnings)
     {
         // Invalid is our placeholder that the action was not parsed.
         ShortcutAction action = ShortcutAction::Invalid;
@@ -208,7 +208,7 @@ namespace winrt::TerminalApp::implementation
         // does, we'll try to deserialize any "args" that were provided with
         // the binding.
         IActionArgs args{ nullptr };
-        std::vector<::TerminalApp::SettingsLoadWarnings> parseWarnings;
+        std::vector<TerminalApp::SettingsLoadWarnings> parseWarnings;
         const auto deserializersIter = argParsers.find(action);
         if (deserializersIter != argParsers.end())
         {

src/cascadia/TerminalApp/ActionAndArgs.h

@@ -9,7 +9,7 @@ namespace winrt::TerminalApp::implementation
     {
         static const std::map<std::string_view, ShortcutAction, std::less<>> ActionKeyNamesMap;
         static winrt::com_ptr<ActionAndArgs> FromJson(const Json::Value& json,
-                                                      std::vector<::TerminalApp::SettingsLoadWarnings>& warnings);
+                                                      std::vector<TerminalApp::SettingsLoadWarnings>& warnings);
 
         ActionAndArgs() = default;
         hstring GenerateName() const;

src/cascadia/TerminalApp/ActionArgs.h

@@ -39,7 +39,7 @@
 namespace winrt::TerminalApp::implementation
 {
     using namespace ::TerminalApp;
-    using FromJsonResult = std::tuple<winrt::TerminalApp::IActionArgs, std::vector<::TerminalApp::SettingsLoadWarnings>>;
+    using FromJsonResult = std::tuple<winrt::TerminalApp::IActionArgs, std::vector<TerminalApp::SettingsLoadWarnings>>;
 
     struct ActionEventArgs : public ActionEventArgsT<ActionEventArgs>
     {
@@ -202,7 +202,7 @@ namespace winrt::TerminalApp::implementation
             JsonUtils::GetValueForKey(json, DirectionKey, args->_Direction);
             if (args->_Direction == TerminalApp::Direction::None)
             {
-                return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
+                return { nullptr, { TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
             }
             else
             {
@@ -237,7 +237,7 @@ namespace winrt::TerminalApp::implementation
             JsonUtils::GetValueForKey(json, DirectionKey, args->_Direction);
             if (args->_Direction == TerminalApp::Direction::None)
             {
-                return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
+                return { nullptr, { TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
             }
             else
             {
@@ -299,7 +299,7 @@ namespace winrt::TerminalApp::implementation
             JsonUtils::GetValueForKey(json, InputKey, args->_Input);
             if (args->_Input.empty())
             {
-                return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
+                return { nullptr, { TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
             }
             return { *args, {} };
         }
@@ -395,7 +395,7 @@ namespace winrt::TerminalApp::implementation
             JsonUtils::GetValueForKey(json, NameKey, args->_SchemeName);
             if (args->_SchemeName.empty())
             {
-                return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
+                return { nullptr, { TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
             }
             return { *args, {} };
         }
@@ -486,7 +486,7 @@ namespace winrt::TerminalApp::implementation
             JsonUtils::GetValueForKey(json, CommandlineKey, args->_Commandline);
             if (args->_Commandline.empty())
             {
-                return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
+                return { nullptr, { TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
             }
             return { *args, {} };
         }

src/cascadia/TerminalApp/AppKeyBindings.h

@@ -53,7 +53,7 @@ namespace winrt::TerminalApp::implementation
         static Windows::System::VirtualKeyModifiers ConvertVKModifiers(winrt::Microsoft::Terminal::TerminalControl::KeyModifiers modifiers);
 
         // Defined in AppKeyBindingsSerialization.cpp
-        std::vector<::TerminalApp::SettingsLoadWarnings> LayerJson(const Json::Value& json);
+        std::vector<TerminalApp::SettingsLoadWarnings> LayerJson(const Json::Value& json);
         Json::Value ToJson();
 
         void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch);

src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp

@@ -93,13 +93,13 @@ Json::Value winrt::TerminalApp::implementation::AppKeyBindings::ToJson()
 //   `"unbound"`, then we'll clear the keybinding from the existing keybindings.
 // Arguments:
 // - json: an array of Json::Value's to deserialize into our _keyShortcuts mapping.
-std::vector<::TerminalApp::SettingsLoadWarnings> winrt::TerminalApp::implementation::AppKeyBindings::LayerJson(const Json::Value& json)
+std::vector<SettingsLoadWarnings> winrt::TerminalApp::implementation::AppKeyBindings::LayerJson(const Json::Value& json)
 {
     // It's possible that the user provided keybindings have some warnings in
     // them - problems that we should alert the user to, but we can recover
     // from. Most of these warnings cannot be detected later in the Validate
     // settings phase, so we'll collect them now.
-    std::vector<::TerminalApp::SettingsLoadWarnings> warnings;
+    std::vector<SettingsLoadWarnings> warnings;
 
     for (const auto& value : json)
     {
@@ -121,7 +121,7 @@ std::vector<::TerminalApp::SettingsLoadWarnings> winrt::TerminalApp::implementat
             // TODO: GH#1334 - remove this check.
             if (keys.isArray() && keys.size() > 1)
             {
-                warnings.push_back(::TerminalApp::SettingsLoadWarnings::TooManyKeysForChord);
+                warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord);
             }
 
             if (!validString && !validArray)

src/cascadia/TerminalApp/AppLogic.cpp

@@ -29,7 +29,7 @@ static const winrt::hstring StartupTaskName = L"StartTerminalOnLoginTask";
 // !!! IMPORTANT !!!
 // Make sure that these keys are in the same order as the
 // SettingsLoadWarnings/Errors enum is!
-static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadWarnings::WARNINGS_SIZE)> settingsLoadWarningsLabels {
+static const std::array<std::wstring_view, static_cast<uint32_t>(winrt::TerminalApp::SettingsLoadWarnings::WARNINGS_SIZE)> settingsLoadWarningsLabels {
     USES_RESOURCE(L"MissingDefaultProfileText"),
     USES_RESOURCE(L"DuplicateProfileText"),
     USES_RESOURCE(L"UnknownColorSchemeText"),
@@ -41,7 +41,7 @@ static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadWar
     USES_RESOURCE(L"LegacyGlobalsProperty"),
     USES_RESOURCE(L"FailedToParseCommandJson")
 };
-static const std::array<std::wstring_view, static_cast<uint32_t>(SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
+static const std::array<std::wstring_view, static_cast<uint32_t>(winrt::TerminalApp::SettingsLoadErrors::ERRORS_SIZE)> settingsLoadErrorsLabels {
     USES_RESOURCE(L"NoProfilesText"),
     USES_RESOURCE(L"AllProfilesHiddenText")
 };
@@ -76,7 +76,7 @@ static winrt::hstring _GetMessageText(uint32_t index, std::array<std::wstring_vi
 // - warning: the SettingsLoadWarnings value to get the localized text for.
 // Return Value:
 // - localized text for the given warning
-static winrt::hstring _GetWarningText(::TerminalApp::SettingsLoadWarnings warning)
+static winrt::hstring _GetWarningText(winrt::TerminalApp::SettingsLoadWarnings warning)
 {
     return _GetMessageText(static_cast<uint32_t>(warning), settingsLoadWarningsLabels);
 }
@@ -89,7 +89,7 @@ static winrt::hstring _GetWarningText(::TerminalApp::SettingsLoadWarnings warnin
 // - error: the SettingsLoadErrors value to get the localized text for.
 // Return Value:
 // - localized text for the given error
-static winrt::hstring _GetErrorText(::TerminalApp::SettingsLoadErrors error)
+static winrt::hstring _GetErrorText(winrt::TerminalApp::SettingsLoadErrors error)
 {
     return _GetMessageText(static_cast<uint32_t>(error), settingsLoadErrorsLabels);
 }
@@ -411,8 +411,7 @@ namespace winrt::TerminalApp::implementation
         // Make sure the lines of text wrap
         warningsTextBlock.TextWrapping(TextWrapping::Wrap);
 
-        const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
-        const auto& warnings = settingsImpl->GetWarnings();
+        const auto warnings = _settings.Warnings();
         for (const auto& warning : warnings)
         {
             // Try looking up the warning message key for each warning.
@@ -631,27 +630,25 @@ namespace winrt::TerminalApp::implementation
             auto newSettings = _isUwp ? CascadiaSettings::LoadUniversal() : CascadiaSettings::LoadAll();
             _settings = newSettings;
 
-            const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
-            const auto& warnings = settingsImpl->GetWarnings();
-            hr = warnings.size() == 0 ? S_OK : S_FALSE;
+            if (_settings.GetLoadingError())
+            {
+                _settingsLoadExceptionText = _GetErrorText(_settings.GetLoadingError().Value());
+                return E_INVALIDARG;
+            }
+            else if (!_settings.GetSerializationErrorMessage().empty())
+            {
+                _settingsLoadExceptionText = _settings.GetSerializationErrorMessage();
+                return E_INVALIDARG;
+            }
+
+            hr = _settings.Warnings().Size() == 0 ? S_OK : S_FALSE;
         }
         catch (const winrt::hresult_error& e)
         {
             hr = e.code();
             _settingsLoadExceptionText = e.message();
             LOG_HR(hr);
         }
-        catch (const ::TerminalApp::SettingsException& ex)
-        {
-            hr = E_INVALIDARG;
-            _settingsLoadExceptionText = _GetErrorText(ex.Error());
-        }
-        catch (const ::TerminalApp::SettingsTypedDeserializationException& e)
-        {
-            hr = E_INVALIDARG;
-            std::string_view what{ e.what() };
-            _settingsLoadExceptionText = til::u8u16(what);
-        }
         catch (...)
         {
             hr = wil::ResultFromCaughtException();

src/cascadia/TerminalApp/CascadiaSettings.cpp

@@ -46,7 +46,9 @@ CascadiaSettings::CascadiaSettings() :
 // - addDynamicProfiles: if true, we'll add the built-in DPGs.
 CascadiaSettings::CascadiaSettings(const bool addDynamicProfiles) :
     _globals{ winrt::make_self<implementation::GlobalAppSettings>() },
-    _profiles{ winrt::single_threaded_observable_vector<TerminalApp::Profile>() }
+    _profiles{ winrt::single_threaded_observable_vector<TerminalApp::Profile>() },
+    _warnings{ winrt::single_threaded_vector<SettingsLoadWarnings>() },
+    _serializationErrorMessage{ L"" }
 {
     if (addDynamicProfiles)
     {
@@ -119,9 +121,19 @@ winrt::TerminalApp::GlobalAppSettings CascadiaSettings::GlobalSettings()
 //   knew were bad when we called `_ValidateSettings` last.
 // Return Value:
 // - a reference to our list of warnings.
-std::vector<TerminalApp::SettingsLoadWarnings>& CascadiaSettings::GetWarnings()
+IVectorView<winrt::TerminalApp::SettingsLoadWarnings> CascadiaSettings::Warnings()
 {
-    return _warnings;
+    return _warnings.GetView();
+}
+
+winrt::Windows::Foundation::IReference<winrt::TerminalApp::SettingsLoadErrors> CascadiaSettings::GetLoadingError()
+{
+    return _loadError;
+}
+
+winrt::hstring CascadiaSettings::GetSerializationErrorMessage()
+{
+    return _serializationErrorMessage;
 }
 
 // Method Description:
@@ -136,7 +148,7 @@ std::vector<TerminalApp::SettingsLoadWarnings>& CascadiaSettings::GetWarnings()
 // - <none>
 void CascadiaSettings::_ValidateSettings()
 {
-    _warnings.clear();
+    _warnings.Clear();
 
     // Make sure to check that profiles exists at all first and foremost:
     _ValidateProfilesExist();
@@ -198,7 +210,7 @@ void CascadiaSettings::_ValidateProfilesExist()
         // We can't add the warning to the list of warnings here, because this
         // object is not going to be returned at any point.
 
-        throw ::TerminalApp::SettingsException(::TerminalApp::SettingsLoadErrors::NoProfiles);
+        throw SettingsException(TerminalApp::SettingsLoadErrors::NoProfiles);
     }
 }
 
@@ -252,7 +264,7 @@ void CascadiaSettings::_ValidateDefaultProfileExists()
 
     if (nullDefaultProfile || defaultProfileNotInProfiles)
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::MissingDefaultProfile);
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::MissingDefaultProfile);
         // Use the first profile as the new default
 
         // _temporarily_ set the default profile to the first profile. Because
@@ -295,7 +307,7 @@ void CascadiaSettings::_ValidateNoDuplicateProfiles()
 
     if (foundDupe)
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::DuplicateProfile);
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::DuplicateProfile);
     }
 }
 
@@ -384,7 +396,7 @@ void CascadiaSettings::_RemoveHiddenProfiles()
     {
         // Throw an exception. This is an invalid state, and we want the app to
         // be able to gracefully use the default settings.
-        throw ::TerminalApp::SettingsException(::TerminalApp::SettingsLoadErrors::AllProfilesHidden);
+        throw SettingsException(TerminalApp::SettingsLoadErrors::AllProfilesHidden);
     }
 }
 
@@ -413,7 +425,7 @@ void CascadiaSettings::_ValidateAllSchemesExist()
 
     if (foundInvalidScheme)
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::UnknownColorScheme);
+        _warnings.Append(SettingsLoadWarnings::UnknownColorScheme);
     }
 }
 
@@ -468,12 +480,12 @@ void CascadiaSettings::_ValidateMediaResources()
 
     if (invalidBackground)
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::InvalidBackgroundImage);
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::InvalidBackgroundImage);
     }
 
     if (invalidIcon)
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::InvalidIcon);
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::InvalidIcon);
     }
 }
 
@@ -659,8 +671,11 @@ void CascadiaSettings::_ValidateKeybindings()
 
     if (!keybindingWarnings.empty())
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::AtLeastOneKeybindingWarning);
-        _warnings.insert(_warnings.end(), keybindingWarnings.begin(), keybindingWarnings.end());
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::AtLeastOneKeybindingWarning);
+        for (auto warning : keybindingWarnings)
+        {
+            _warnings.Append(warning);
+        }
     }
 }
 
@@ -679,7 +694,7 @@ void CascadiaSettings::_ValidateNoGlobalsKey()
 {
     if (auto oldGlobalsProperty{ _userSettings["globals"] })
     {
-        _warnings.push_back(::TerminalApp::SettingsLoadWarnings::LegacyGlobalsProperty);
+        _warnings.Append(TerminalApp::SettingsLoadWarnings::LegacyGlobalsProperty);
     }
 }
 

src/cascadia/TerminalApp/CascadiaSettings.h

@@ -84,14 +84,18 @@ namespace winrt::TerminalApp::implementation
         TerminalApp::Profile FindProfile(guid profileGuid) const noexcept;
         TerminalApp::ColorScheme GetColorSchemeForProfile(const guid profileGuid) const;
 
-        std::vector<::TerminalApp::SettingsLoadWarnings>& GetWarnings();
+        Windows::Foundation::Collections::IVectorView<SettingsLoadWarnings> Warnings();
+        Windows::Foundation::IReference<SettingsLoadErrors> GetLoadingError();
+        hstring GetSerializationErrorMessage();
 
         bool ApplyColorScheme(Microsoft::Terminal::TerminalControl::IControlSettings settings, hstring schemeName);
 
     private:
         com_ptr<GlobalAppSettings> _globals;
         Windows::Foundation::Collections::IObservableVector<TerminalApp::Profile> _profiles;
-        std::vector<::TerminalApp::SettingsLoadWarnings> _warnings;
+        Windows::Foundation::Collections::IVector<TerminalApp::SettingsLoadWarnings> _warnings;
+        Windows::Foundation::IReference<SettingsLoadErrors> _loadError;
+        hstring _serializationErrorMessage;
 
         std::vector<std::unique_ptr<::TerminalApp::IDynamicProfileGenerator>> _profileGenerators;
 

src/cascadia/TerminalApp/CascadiaSettings.idl

@@ -3,6 +3,7 @@
 
 import "GlobalAppSettings.idl";
 import "Profile.idl";
+import "TerminalWarnings.idl";
 
 namespace TerminalApp
 {
@@ -17,6 +18,10 @@ namespace TerminalApp
 
         AppKeyBindings Keybindings { get; };
 
+        Windows.Foundation.Collections.IVectorView<SettingsLoadWarnings> Warnings { get; };
+        Windows.Foundation.IReference<SettingsLoadErrors> GetLoadingError { get; };
+        String GetSerializationErrorMessage { get; };
+
         Profile FindProfile(Guid profileGuid);
         ColorScheme GetColorSchemeForProfile(Guid profileGuid);
     }