Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more details to FileManager error report #1895

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

MrStevns
Copy link
Member

@MrStevns MrStevns commented Nov 24, 2024

The purpose of this PR has been to make the error report cover more steps, introduce "Error" to make it more explicit what's a information, a warning and an error.

I've also tried to make it fail faster rather than spitting out all kinds of irrelevant information when the actual error happened 4 lines above.

Here's an example of what our current report looks like:

Raw file path: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
Resolved file path: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
  File name: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
  Recognized New zipped Pencil2D File Format (*.pclx) !
    Miniz found an error while reading the file. - 7, failed finding central directory
    Miniz found an error while closing file file. - 24, invalid parameter
  XML file: /private/var/folders/5c/2bmlz9fx2kzbd56j81ktpghr0000gn/T/Pencil2D/test-save-corruption_Y2xD_cplu6r9q/main.xml
  Data folder: /private/var/folders/5c/2bmlz9fx2kzbd56j81ktpghr0000gn/T/Pencil2D/test-save-corruption_Y2xD_cplu6r9q/data
  Working folder: /private/var/folders/5c/2bmlz9fx2kzbd56j81ktpghr0000gn/T/Pencil2D/test-save-corruption_Y2xD_cplu6r9q/
  Main XML file does not exist
System Info
Pencil2D version: 0.7.0.911 (stable)
Build ABI: x86_64-little_endian-lp64
Kernel: darwin, 21.6.0
Operating System: macOS 12.7
end

And here's the new one:

Raw file path: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
Resolved file path: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
  
[Project LOAD diagnostics]

  File name: /Users/mrstevns/Desktop/p2d testing/test-save-corruption.pclx
  Working dir: /private/var/folders/5c/2bmlz9fx2kzbd56j81ktpghr0000gn/T/Pencil2D/test-save-corruption_Y2xD_719f2v7t/
  Project format: .pclx
    
[Miniz sanity check]

    Found an error while reading the file. Error code: 7, reason: failed finding central directory
    Found an error while closing the file. Error code: 24, reason: invalid parameter
  
Error: Unable to extract project, miniz sanity check failed.

[System Info]

  Pencil2D version: 0.0.0.0 (dev)
  Build ABI: x86_64-little_endian-lp64
  Kernel: darwin, 21.6.0
  Operating System: macOS 12.7
  Language: en_GB

In addition the error dialog now also has a "Copy to clipboard" button, so the user can more easily paste the error report.

image

The problem with not aborting here is that while one file can fail, it will be noted down but if the next file succeeds, then it will seem like the project got compressed successfully.
And get rid of useless or confusing information.
The logic didn't account for existing files with "backup" suffix, eg.
A folder may contain:

MyProject.pclx
MyProject.backup1.pclx

If the user opened MyProject.pclx, and failed to save it later, the logic would not count existing backup files like: "MyProject.backup1.pclx" and as such the result would be no backup being made.
…non existent files

This is caused by the new logic actually catching when a file has gone missing while saving. While it may not fix any bugs inheritly, it should make it easier to diagnose problems in the future.
I'm blown away by the fact that this worked at all... wtf. The only one who reported a problem here was windows but technically the file should never have been closed on any of them.

Frightening..
And move some closer to the open call to make sure it happens when the scope ends.
@MrStevns MrStevns force-pushed the improvement/saving-error-handling branch from bb18358 to d4bdc23 Compare November 24, 2024 13:11
@MrStevns MrStevns changed the title Improve FileManager error handling for readability Add more details to FileManager error report Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

1 participant