SaveAs/Save failures are non-atomic, mutate document state
- Dominant language
- C++
- Stars
- 33.6k
- Forks
- 6k
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 196
Description
### Problem description
A failure during a save operation alters the Document, leaving it in an inconsistent state. A failed save operation should have no side effects on the working Document.
---
Specifically here: https://github.com/FreeCAD/FreeCAD/blob/main/src/App/Document.cpp#L1902-L1906
``` c++
this->FileName.getStrValue() != file) {
this->FileName.setValue(file);
this->Label.setValue(fi.fileNamePure());
this->Uid.touch(); // this forces a rename of the transient directory
```
Several Document properties are altered before the save() call. Those changes are not rolled back if the save fails.
### Workbench affected?
None/All? unsure of scope.
### Steps to reproduce
- Make a new Parametric Body
- `File` -> `Save As...`
- Choose a nonsensical destination for your operating system.
- On Linux try to Overwrite `/dev/null`
- On OSX I duplicated this by selecting a path on a network share and pulling the share before pressing the save button.
- On Windows you might be able to force it into a path like B:/I_Dont_have_a_Floppy
- Receive "Failed to open file" error.
**At this point the in memory document is in a tainted state.**
UI evidence of this is "Unnamed" becoming "Null", however more problematically several document properties have been altered: FileName, Label, and Uid (List possibly not exhaustive, will need to dig.)
Any subsequent attempt to `Save` (not `Save As...`) will fail with the same "Failed to open file" message instead of prompting the user to SaveAs again.
### Expected behavior
- Make a new Parametric Body
- `File` -> `Save As...`
- Choose a nonsensical destination for your operating system.
- Receive an error message.
- Document remains unchanged
- Follow up `Save` (not `Save As...`) actions trigger the `Save As...` dialog as if the document was still fresh.
### Actual behavior
- Make a new Parametric Body
- `File` -> `Save As...`
- Choose a nonsensical destination for your operating system.
- Receive an error message.
- Document is renamed, document path is set, uid changes
- Follow up `Save` (not `Save As...`) actions trigger the "Failed to open file" error.
### Development version About Info (in Safe Mode)
```shell
Verified on Main branch
```
Contributor guide
Research direction
Start in src/App/Document.cpp around lines 1902-1906 and trace how Save As updates FileName, Label, and Uid before save(). Reproduce the failure by saving a new document to an invalid destination, then inspect the subsequent Save behavior. Done means a failed save leaves the document unchanged and a later Save opens Save As again.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100