#647 OWLMaker crashes when creating non-existing file from the MRU list

[Vidar Hasfjord]

This issue only occurs when the parent of the file in question has been moved or removed.

To reproduce:

1. Create a test file "test.txt" in a new folder "Test".
2. Open the file in OWLMaker.
3. Observe that the MRU list on the File menu now includes "test.txt".
4. Delete the "Test" folder.
5. In OWLMaker, select "test.txt" from the MRU list on the File menu.
6. Message box is shown: "File '...' doesn't exist. Create it?" (Yes/No).
7. Select Yes.
8. Observe the following:
   • Build 8642 (latest release): Crash; no message, just immediate shutdown.
   • Build 7704 and 7498: "Unable to read file '...' from disk" shown twice, followed by empty editor titled "File - Untitled".
   • Build 7349-6049: Same behaviour, but message shown once only.
     
     

Proposed resolution

If the parent folder of the MRU file is not valid, do not ask the user whether to recreate the file. Just show a message informing the user that the file no longer exists, and just remove the MRU entry.

[Ognyan Chernokozhev]

It seems to me part of the problem is that this code is buried too deep in the DocView framework and does not allow user code an easy way to catch and handle the exceptions.

Thus, DocView should handle all these errors gracefully and not throw unhandled exceptions that would bring the whole application down.
Or there should be some easy mechanism that would allow user code to be notifed and for it to decide how to handle the error condition.

[Vidar Hasfjord]

@jogybl wrote:

part of the problem is that this code is buried too deep in the DocView framework and does not allow user code an easy way to catch and handle the exceptions.

While the exception handling within DocView may be poor and need improvement, the faulty logic for this particular problem is implemented in TOWLMakerApp::CmOpenRecentFile.

Draft proposed fix (not tested):

auto TOWLMakerApp::CmOpenRecentFile(TParam1 p1, TParam2) -> TResult
{
  const auto w = GetMainWindow(); PRECONDITION(w);
  const auto msgCaption = _T("MRU file open error");
  try // Don't let exceptions escape handler.
  {
    const auto menuItemId = static_cast<int>(p1);
    const auto menuItemText = GetMenuText(menuItemId);
    const auto f = filesystem::path{menuItemText}; // MRU entry (filename)
    const auto removeMruEntry = [&]
    {
      RemoveMenuChoice(menuItemText);
    };

    if (filesystem::exists(f))
    {
      OpenFile(f);
    }
    else if (!filesystem::exists(f.parent_path())
    {
      removeMruEntry();
      throw runtime_error{"File and parent folder no longer exist"};
    }
    else // Ask the user whether to recreate the file.
    {
      auto msg = tostringstream{};
      msg << _T("File \"") << to_tstring(f) << _T("\" doesn't exist. Create it?");
      const auto r = w->MessageBox(msg.str(), msgCaption, MB_YESNO | MB_ICONQUESTION);
      if (r != IDYES)
      {
        removeMruEntry();
      }
      else if (auto ofs = ofstream{f}) // Recreate OK?
      {
        ofs.close();
        OpenFile(f);
      }
      else // Recreation failed.
      {
        removeMruEntry();
        throw runtime_error{"Attempt to recreate MRU file failed"};
      }
    }
  }
  catch (const TDiagException&) { throw; } // Let diagnostics through.
  catch (const exception& x)
  { 
    w->MessageBox(to_tstring(x.what()), msgCaption, MB_OK | MB_ICONERROR); 
  }
  return 0;
}