Fix LightSwitch crash in update_sun_times due to unhandled WinRT exception#47394
Fix LightSwitch crash in update_sun_times due to unhandled WinRT exception#47394Copilot wants to merge 3 commits into
Conversation
…uard add_property against missing properties key Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/8940919c-1eef-4a10-9abc-a0defbc156d8 Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the LightSwitch service caused by WinRT JSON APIs throwing exceptions that were not handled by the existing std::exception catch, particularly when settings JSON does not contain the expected "properties" object.
Changes:
- Harden
PowerToyValues::add_propertyto create the"properties"JSON object when missing. - Broaden exception handling in LightSwitch
update_sun_timesto prevent unhandled termination on non-std::exceptionthrows.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp | Broadens exception handling around sun-time settings update to avoid process termination. |
| src/common/SettingsAPI/settings_objects.h | Makes PowerToyValues::add_property resilient to missing "properties" key in loaded settings JSON. |
| if (!m_json.HasKey(L"properties")) | ||
| { | ||
| m_json.SetNamedValue(L"properties", json::JsonObject{}); | ||
| } |
There was a problem hiding this comment.
Fixed in 53dcab7.
add_property now uses json::has(..., JsonValueType::Object) before calling GetNamedObject, so corrupted properties values (null/array/string/etc.) get replaced with a fresh object instead of throwing.
| catch (...) | ||
| { | ||
| std::string msg = e.what(); | ||
| std::wstring wmsg(msg.begin(), msg.end()); | ||
| Logger::error(L"[LightSwitchService] Exception during sun time update: {}", wmsg); | ||
| Logger::error(L"[LightSwitchService] Exception during sun time update."); | ||
| } |
There was a problem hiding this comment.
Fixed in 53dcab7.
update_sun_times now catches winrt::hresult_error first and logs both the HRESULT and WinRT message, keeps a std::exception catch for what(), and leaves the final catch-all to log an unknown exception.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot please re-review the latest changes. |
Reviewed commit
No issues found. Ready to merge. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
This PR doesn't seem to be addressing the root cause, as original issue comments indicate properties field is not missing from the settings json. |
Summary of the Pull Request
PowerToys.LightSwitchService.execrashes withFAST_FAIL_FATAL_APP_EXITwhenupdate_sun_timescallsadd_propertyon aPowerToyValuesobject loaded from a settings file that lacks the"properties"JSON key. The WinRT exception thrown byGetNamedObjectis not astd::exceptionsubclass and escapes thecatch (const std::exception&)guard, killing the process.Fix 1 —
src/common/SettingsAPI/settings_objects.h: Guardadd_propertyagainst a missing or corrupted"properties"key (e.g. fresh install, corrupted file).load_from_settings_fileloads raw JSON with no schema guarantee, so the key may not exist or may have a non-Object type. The guard now usesjson::has(..., JsonValueType::Object)to handle both cases — a missing key and a key present with a non-Object type (null/array/string/number) — replacingpropertieswith a freshJsonObjectwhen needed.Fix 2 —
src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp: Expand exception handling inupdate_sun_timeswith typed catches for maximum diagnostics, plus a final catch-all as defense-in-depth.winrt::hresult_erroris caught first and logs both the HRESULT code and WinRT message;std::exceptionlogswhat(); the finalcatch (...)handles any other unexpected exception type.PR Checklist
Detailed Description of the Pull Request / Additional comments
Crash chain:
update_sun_timeson a new day or mode switch.PowerToyValues::load_from_settings_file(L"LightSwitch"), which loads the raw JSON blob without validating its schema.add_propertycallsm_json.GetNamedObject(L"properties")— if the key is absent or has a non-Object type (first run, migration, corruption), the WinRT JSON API throwswinrt::hresult_error.winrt::hresult_errordoes not inherit fromstd::exception, so thecatch (const std::exception&)guard is bypassed.terminate()→FAST_FAIL_FATAL_APP_EXIT (0xC0000409).Note:
has_property(insettings_objects.cpp) already handled this safely viajson::has(..., JsonValueType::Object).add_propertynow follows the same pattern.Validation Steps Performed
winrt::hresult_erroris not derived fromstd::exception(confirming the catch miss).has_propertyalready usesjson::has(..., JsonValueType::Object);add_propertynow follows suit.json::haschecks both key presence and value type, handling corruptedpropertiesvalues (null/array/string/number) in addition to the missing-key case.winrttypes are available through thejson.hinclude chain.