Address review: handle NavigationFailed gracefully without rethrowing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/modules/powerrename/PowerRenameUILib/PowerRenameXAML/App.xaml.cpp

@@ -243,7 +243,6 @@ void App::OnLaunched(LaunchActivatedEventArgs const&)
         BOOL bSuccess;
         WCHAR chBuf[BUFSIZE];
         DWORD dwRead;
-        std::wstring accumulated;
         for (;;)
         {
             // Read from standard input and stop on error or no data.
@@ -252,34 +251,18 @@ void App::OnLaunched(LaunchActivatedEventArgs const&)
             if (!bSuccess || dwRead == 0)
                 break;
 
-            // Append the newly-read data to any leftover from the previous iteration.
-            // A file path may span two consecutive reads, so we must not split on chunk
-            // boundaries; instead we scan for the '?' delimiter that separates entries.
-            accumulated.append(chBuf, dwRead / sizeof(wchar_t));
+            std::wstring inputBatch{ chBuf, dwRead / sizeof(wchar_t) };
 
+            std::wstringstream ss(inputBatch);
+            std::wstring item;
             wchar_t delimiter = '?';
-            size_t pos = 0;
-            size_t found;
-            while ((found = accumulated.find(delimiter, pos)) != std::wstring::npos)
+            while (std::getline(ss, item, delimiter))
             {
-                std::wstring item = accumulated.substr(pos, found - pos);
-                if (!item.empty())
-                {
-                    g_files.push_back(std::move(item));
-                }
-                pos = found + 1;
+                g_files.push_back(item);
             }
-            // Keep whatever comes after the last '?' for the next iteration.
-            accumulated = accumulated.substr(pos);
-        }
-        // Flush any trailing data that was not terminated by a '?'.
-        // In normal operation every entry is '?'-terminated by the writer, so reaching
-        // here with a non-empty accumulator means the pipe was closed mid-write (e.g.
-        // the sending process crashed). Discard the incomplete path rather than adding
-        // a potentially corrupt entry to the file list.
-        if (!accumulated.empty())
-        {
-            Logger::warn(L"PowerRename: pipe closed with unterminated path fragment, discarding: {}", accumulated);
+
+            if (!bSuccess)
+                break;
         }
         CloseHandle(hStdin);
 #endif

src/modules/powerrename/PowerRenameUILib/PowerRenameXAML/MainWindow.xaml.cpp

@@ -16,9 +16,7 @@
 #include <common/Telemetry/EtwTrace/EtwTrace.h>
 
 #include <atlstr.h>
-#include <algorithm>
 #include <exception>
-#include <iterator>
 #include <string>
 #include <sstream>
 #include <vector>
@@ -277,29 +275,14 @@ namespace winrt::PowerRenameUI::implementation
                         // To test PowerRename uncomment DEBUG_BENCHMARK_100K_ENTRIES define
                         if (!g_files.empty())
                         {
-                            // Process files in batches to avoid Windows Shell API limits
-                            // that can cause crashes (STATUS_STACK_BUFFER_OVERRUN) with
-                            // very large file counts (30,000+).
-                            constexpr size_t BATCH_SIZE = 5000;
-                            const size_t totalFiles = g_files.size();
-                            for (size_t batchStart = 0; batchStart < totalFiles; batchStart += BATCH_SIZE)
+                            if (SUCCEEDED(CreateShellItemArrayFromPaths(std::move(g_files), &shellItemArray)))
                             {
-                                size_t batchEnd = std::min(batchStart + BATCH_SIZE, totalFiles);
-                                std::vector<std::wstring> batch(
-                                    std::make_move_iterator(g_files.begin() + batchStart),
-                                    std::make_move_iterator(g_files.begin() + batchEnd));
-
-                                shellItemArray = nullptr;
-                                if (SUCCEEDED(CreateShellItemArrayFromPaths(std::move(batch), &shellItemArray)))
+                                CComPtr<IEnumShellItems> enumShellItems;
+                                if (SUCCEEDED(shellItemArray->EnumItems(&enumShellItems)))
                                 {
-                                    CComPtr<IEnumShellItems> enumShellItems;
-                                    if (SUCCEEDED(shellItemArray->EnumItems(&enumShellItems)))
-                                    {
-                                        EnumerateShellItems(enumShellItems);
-                                    }
+                                    EnumerateShellItems(enumShellItems);
                                 }
                             }
-                            g_files.clear();
                         }
                         else
                         {
@@ -513,43 +496,15 @@ namespace winrt::PowerRenameUI::implementation
         itemList = new (std::nothrow) PIDLIST_ABSOLUTE[files.size()];
         HRESULT hr = itemList ? S_OK : E_OUTOFMEMORY;
         UINT itemsCnt = 0;
-        if (SUCCEEDED(hr))
+        for (const auto& file : files)
         {
-            for (const auto& file : files)
+            const DWORD BUFSIZE = 4096;
+            TCHAR buffer[BUFSIZE] = TEXT("");
+            auto retval = GetFullPathName(file.c_str(), BUFSIZE, buffer, NULL);
+            if (retval != 0 && PathFileExists(buffer))
             {
-                const DWORD BUFSIZE = 4096;
-                TCHAR buffer[BUFSIZE] = TEXT("");
-                std::wstring fullPath;
-                const DWORD needed = GetFullPathName(file.c_str(), BUFSIZE, buffer, nullptr);
-                if (needed == 0)
-                {
-                    continue;
-                }
-
-                if (needed >= BUFSIZE)
-                {
-                    std::vector<wchar_t> dynBuffer(needed);
-                    const DWORD result = GetFullPathName(file.c_str(), needed, dynBuffer.data(), nullptr);
-                    if (result == 0 || result >= needed)
-                    {
-                        continue;
-                    }
-
-                    fullPath = std::wstring(dynBuffer.data());
-                }
-                else
-                {
-                    fullPath = std::wstring(buffer);
-                }
-
-                if (PathFileExists(fullPath.c_str()))
-                {
-                    PIDLIST_ABSOLUTE pidl = nullptr;
-                    if (SUCCEEDED(SHParseDisplayName(fullPath.c_str(), nullptr, &pidl, 0, nullptr)))
-                    {
-                        itemList[itemsCnt++] = pidl;
-                    }
-                }
+                hr = SHParseDisplayName(buffer, nullptr, &itemList[itemsCnt], 0, nullptr);
+                ++itemsCnt;
             }
         }
         if (SUCCEEDED(hr) && itemsCnt > 0)
@@ -565,15 +520,11 @@ namespace winrt::PowerRenameUI::implementation
             else
             {
                 Logger::error(L"Creating ShellItemArray from path list failed.");
-                for (UINT i = 0; i < itemsCnt; i++)
-                {
-                    CoTaskMemFree(itemList[i]);
-                }
             }
         }
-        else if (SUCCEEDED(hr) && itemsCnt == 0)
+        else
         {
-            Logger::error(L"No valid file paths could be resolved; all entries either do not exist or could not be parsed by the shell.");
+            Logger::error(L"Parsing path list display names failed.");
             hr = E_FAIL;
         }
 

src/settings-ui/Settings.UI/ViewModels/ShellViewModel.cs

@@ -9,6 +9,7 @@ using System.Linq;
 using System.Threading.Tasks;
 using System.Windows.Input;
 
+using ManagedCommon;
 using Microsoft.PowerToys.Settings.UI.Helpers;
 using Microsoft.PowerToys.Settings.UI.Library;
 using Microsoft.PowerToys.Settings.UI.Library.Helpers;
@@ -135,7 +136,18 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
 
         private void Frame_NavigationFailed(object sender, NavigationFailedEventArgs e)
         {
-            throw e.Exception;
+            var sourcePage = e.SourcePageType?.FullName ?? "<unknown>";
+
+            if (e.Exception is null)
+            {
+                Logger.LogWarning($"Navigation to '{sourcePage}' failed without an exception.");
+            }
+            else
+            {
+                Logger.LogError($"Navigation to '{sourcePage}' failed.", e.Exception);
+            }
+
+            e.Handled = true;
         }
 
         private void Frame_Navigated(object sender, NavigationEventArgs e)