Handle exceptions while writing to files#4400
Conversation
YoshiRulz
left a comment
YoshiRulz
left a comment
There was a problem hiding this comment.
Better I/O handling has been sorely needed for a while. I suspect the .tasproj corruptions will end with this.
src/BizHawk.Client.EmuHawk/tools/TAStudio/TAStudio.IToolForm.cs
Outdated
Show resolved
Hide resolved
SuuperW
force-pushed
the
dot_saving
branch
2 times, most recently
from
fde3481 to
0798074
Compare
|
Highlights of force push:
|
This comment was marked as outdated.
This comment was marked as outdated.
SuuperW
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
|
I believe this is ready for merge. See edited OP for notes on some behavior changes. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Removed ISaveRam refactor. |
YoshiRulz
added
App: EmuHawk
|
Are there any reasons to not merge this? |
|
I'd like to get more eyes on this, especially the new API's design. |
|
For what it's worth, all my TASing since creating the PR has been done on a branch with these commits and I have not encountered any issues. |
|
If there's been no eyes on this for half a year we shouldn't expect them to appear. Unless something looks broken it should be fine to just merge and see. Conflicts tho. |
…e until writing has completed. Avoids crashing and prevents loss of data in the event of an exception mid-write.
…still broken on close and on TAStudio open.
…celing flow even though not all asking and error handling is implemented yet. Also, fix: respect the cancel button.
…properly checks the autosave config option.
| string prefix = message ?? ""; | ||
| bool? askResult = dialogParent.ModalMessageBox3( | ||
| text: $"{prefix} Do you want to try again?\n\nError details:" + | ||
| $"{fileResult.UserFriendlyErrorMessage()}\n{fileResult.Exception!.Message}", |
There was a problem hiding this comment.
This "friendlier" message omits the important details of the exception type and stacktrace, making it useless for debugging. Fixed in 773f300.
(You also missed a space after the colon, gave the dialog the nondescriptive title "Error", forgot to update this method's documentation to reflect the new return type, and haven't followed the code style conventions.)
3 participants
Most file writes now use a new system that allows for error detection and handling. (includes movies, config file, and all tools)
When an exception occurs, a message box appears alerting the user. If there is an exception while doing a save-on-close, the user is given the option to try again, skip saving, or cancel closing. (Includes a bug fix where an existing dialog offering the cancel option did not actually cancel.)
Addresses #4258.
Fixes an issue where the CheatList was being forcibly autosaved when closing a game, ignoring the autosave config option.
Changes behavior when saving the config file fails. Previously it would silently fail (and report success if "saved" via the menu item), now it has the same error handling as other updated file writes. The silent fail seems to have been intended to allow using a read-only config file. If we want to bring this back, I suggest making a config option to not save on close instead. As-is, a read-only file will still work you just have to click "No" after closing EmuHawk when it tells you it can't save the config file.
Check if completed: