FreeCAD / FreeCAD/FreeCAD

SaveAs/Save failures are non-atomic, mutate document state

Open
#32,482 2 comments 2 reactions 0 assignees View on GitHub
Mod: Core Status: Needs 'About' info Status: Needs confirmation
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.**

Image

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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.