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

Show which file was trying to be closed when error on close #643

Merged
merged 1 commit into from Nov 16, 2023

Conversation

tswfi
Copy link
Contributor

@tswfi tswfi commented Nov 10, 2023

Questions Answers
Description? When backup zip file closing fails, show which zip file was trying to be closed
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company
How to test? try to fail zip close to see the new message.
Before it was just ERROR - Could not close the Zip file properly. Check you are allowed to write on the disk and there is available space on it.
now it is ERROR - Could not close the Zip file: /path/to/admin-dev/autoupgrade/backup/auto-backupfiles_V8.0.5_20231110-142932-5b1cebf4.zip properly. Check you are allowed to write on the disk and there is available space on it.

@tswfi
Copy link
Contributor Author

tswfi commented Nov 10, 2023

This was actually interesting. The reason the close failed was that I had one file with 200 permissions --w-------.. ZipArchive tried to add it to the backup file but closing couldn't do read the file correctly so the close failed..

But anyways, it will be helpful to see the zip filename here :)

@AureRita AureRita self-assigned this Nov 15, 2023
@AureRita
Copy link

Hi @tswfi

Thank you for your PR, I tried to test your PR but curently I can't have the message even if I disable the right of the zip, as you can see :
image

Could you give us an how to test that permite us to have this message ?

Thank you

@AureRita AureRita removed their assignment Nov 15, 2023
@tswfi
Copy link
Contributor Author

tswfi commented Nov 15, 2023

@AureRita try adding a file like "foobar.test" with only write permission somewhere that gets backed up. Its actually another bug (searching for the files to be backed up checks that it can see a file but it doesn't check that the file is readable). Then when the zip library tries to read the file into the zip it fails.

Also the restore step can fail if it tries to restore a file with only read permission and no write permission (for example some hoster have a "statistics" folder that gets awstats or similar generated every night and those files might be read only for the user).

@AureRita AureRita self-assigned this Nov 16, 2023
@AureRita
Copy link

Hi @tswfi

Thank you for your information, yesterday I put the zip on read only, as you can see :

image

Thank you for your PR, I tested it and it seems to works as you can see :

Untitled_.Nov.16.2023.4_21.PM.webm

Because the PR seems to works as expected, It's QA ✔️

Thank you

@nicosomb nicosomb added this to the 4.16.5 milestone Nov 16, 2023
@nicosomb nicosomb merged commit 98cf81d into PrestaShop:dev Nov 16, 2023
26 checks passed
@nicosomb
Copy link
Contributor

Thank you @tswfi !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
6 participants