Repository navigation
[flutter_local_notifications_windows] recover from a disconnected notifier instead of terminating the app - #2850
DavidClaudeAI wants to merge 3 commits into
Conversation
…ifier instead of terminating the app The ToastNotifier and ToastNotificationHistory handles are created once in init() and kept for the lifetime of the process. When the COM object behind them goes away while the app is running, every later call on them throws a winrt::hresult_error (CO_E_OBJNOTCONNECTED). The exception crosses the FFI boundary, cannot be caught on the Dart side, and ends in std::terminate. Every call on the handles now goes through withHandles(), which: - recreates the notifier and the history and retries once when the error means the handles are disconnected (CO_E_OBJNOTCONNECTED, RPC_E_DISCONNECTED, RPC_S_SERVER_UNAVAILABLE); - otherwise reports a failure to Dart (false, NativeUpdateResult::failed or an empty array) instead of letting any exception escape. Fixes MaikuB#2666 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ex6NcGBnmnz4jZjb4eAsrV
… widen the retry codes - init() no longer lets an exception escape: when the notification platform is unavailable at launch, it returns false instead of terminating the app. - A tag that is not a number is skipped instead of emptying the whole list returned by getActiveNotifications / getPendingNotifications. - The retry also covers RPC_E_SERVER_DIED_DNE and RPC_S_CALL_FAILED_DNE. Only codes that guarantee the call did not execute are retried, so a retry cannot run a call twice. - The helpers are now local to the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ex6NcGBnmnz4jZjb4eAsrV
|
Thanks for the PR. Is there consistent, reliable approach for me to reproduce the issue that this PR as well? |
There was a problem hiding this comment.
🟡 Changes recommended
Failed initialization remains unrecoverable, scheduling failures are silently ignored, and some failure reporting is inaccurate.
4 open findings
What changed in this PR
Adds Windows notification handle recovery to prevent disconnected WinRT objects from terminating the application.
Changes:
- Recreates disconnected notifier/history handles and retries once.
- Converts native exceptions into FFI-safe failure results.
- Skips malformed notification tags when listing notifications.
| File | Description |
|---|---|
flutter_local_notifications_windows/src/ffi_api.cpp |
Adds handle recovery, exception containment, and safer notification-list conversion. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } catch (...) { | ||
| // The notification platform can be unavailable at launch. Report it instead of terminating. | ||
| return false; |
| return withHandles(plugin, [&] { | ||
| ToastNotification notification(doc); | ||
| const auto data = dataFromMap(bindings); | ||
| notification.Tag(winrt::to_hstring(id)); | ||
| notification.Data(data); | ||
| plugin->notifier.value().Show(notification); | ||
| }); |
| return withHandles(plugin, [&] { | ||
| ScheduledToastNotification notification(doc, winrt::clock::from_time_t(time)); | ||
| notification.Tag(winrt::to_hstring(id)); | ||
| plugin->notifier.value().AddToSchedule(notification); | ||
| }); |
| static void addId(vector<int>& ids, const winrt::hstring& tag) { | ||
| try { | ||
| ids.push_back(std::stoi(winrt::to_string(tag))); | ||
| } catch (const std::exception&) { | ||
| } | ||
| } |
|
Thanks for taking a look. No, unfortunately. It happened twice, at random, within five days on one machine (Windows 11 x64, unpackaged app, release build, Flutter 3.47.1): once in We are running the patched build day to day and will report back here if anything changes. |
|
Thanks I'll wait for an update but in meantime I'll double check things as usual and allow workflow to kick off as well |
| } catch (const winrt::hresult_error& error) { | ||
| if (!isDisconnected(error)) return false; | ||
| } catch (...) { | ||
| return false; |
There was a problem hiding this comment.
I know the plugin wasn't doing this before but is there an opportunity for it to catch an exception that can be propagated on the Dart so it can be caught by apps and dealt with appropriately? This is a general question btw so applies to the next catch block below too
There was a problem hiding this comment.
Yes, that's possible. An exception can't cross the FFI boundary, but the native side can keep the failure and the Dart side can throw it. Roughly:
- when a call still fails after the reconnect-and-retry,
withHandlesstores the error (HRESULT and message) in the plugin instead of only returningfalse; - a small FFI getter exposes it, and the Dart wrapper throws a dedicated exception (for example
WindowsNotificationExceptionwithcodeandmessage) whenever a native call returnsfalse; - this also covers two points Copilot raised:
showRawXmlwould no longer report a platform failure as invalid XML, andzonedSchedule/zonedScheduleRawXmlwould no longer complete silently when nothing was scheduled. A failedinitialize()could become retryable at the same time.
It does change behaviour: calls such as cancelAll, which used to either succeed or terminate the app, would now throw, so apps may need a try/catch. Would you rather have it in this PR, or keep this one as the crash fix for the 22.x hotfix and do the error propagation in a follow-up PR on master? Either way I'll fix the std::stoi prefix issue Copilot found here.
There was a problem hiding this comment.
Thanks for the info, it can be done in a follow up PR
There was a problem hiding this comment.
Thanks, I'll do it in a follow-up PR on master once this one is merged.
The std::stoi issue Copilot raised is fixed in 2ccc046: the tag is now parsed with std::from_chars and kept only when every character was consumed, so a tag such as 12abc is skipped instead of being listed as ID 12. Tags written by the plugin still parse, negative IDs included.
…numeric std::stoi accepts a numeric prefix, so a tag such as "12abc" was listed as notification ID 12 instead of being skipped. addId now parses with std::from_chars and keeps the ID only when the whole tag was consumed, which also rejects leading or trailing whitespace and a leading '+'. Tags written by the plugin (winrt::to_hstring of an int) still parse, negative IDs included. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ex6NcGBnmnz4jZjb4eAsrV


Fixes #2666.
Problem
init()creates theToastNotifierand theToastNotificationHistoryonce and keeps them for the lifetime of the process. When the COM object behind them goes away while the app is running, the next call on them throws awinrt::hresult_error(0x800401FD,CO_E_OBJNOTCONNECTED). The exception crosses the FFI boundary, cannot be caught on the Dart side, and the app ends instd::terminate→abort. We hit it twice on an unpackaged app (AUMID branch), once incancelAlland once inshowNotification, as described in the issue.Change
Everything is in
src/ffi_api.cpp:withHandles(). If the error means the handles are disconnected, the notifier and the history are created again (the same wayinit()does it) and the call is retried once. Any other exception is turned into a failure for Dart:false,NativeUpdateResult::failedor an empty array. No exception can cross the FFI boundary anymore.CO_E_OBJNOTCONNECTED,RPC_E_DISCONNECTED,RPC_E_SERVER_DIED_DNE,RPC_S_SERVER_UNAVAILABLE,RPC_S_CALL_FAILED_DNE. Codes such asRPC_E_SERVER_DIED("the call may have executed") are reported as a failure instead.init()returnsfalseinstead of throwing when registration or the creation of the handles fails, for example when the notification platform is unavailable at launch.getActiveNotifications/getPendingNotifications, a tag that is not a number is skipped instead of throwing. Without this, one unexpected entry in the history would now empty the whole list.Testing
flutter_local_notifications_windows3.1.1: no error and no new warning.winerror.h. It was not run against a real disconnected notifier.WpnUserServicewhile the app runs does not reproduce it: the existing notifier keeps working. We are now running the patched build day to day and will report back here if the crash comes back, or if it stops.Questions
master; the same commits apply unchanged on top offlutter_local_notifications_windows-v3.1.1.zonedScheduleandzonedScheduleRawXmlignore theboolreturned byscheduleNotification, so a failure there is now silent, where it used to terminate the app. Throwing inzonedSchedule, likeshowdoes, would be safe. Doing the same inzonedScheduleRawXmlwould change existing behaviour for invalid XML. I left both untouched and can add either if you want.initialize()returnsfalse, a second call throws aLateInitializationError, because_pluginis alate finalfield. This is not new, butinit()can now returnfalseat launch where it used to terminate the app. Making it retryable would be a small Dart change, if you want it.🤖 Generated with Claude Code
https://claude.ai/code/session_01Ex6NcGBnmnz4jZjb4eAsrV