sbergwall / RobocopyPS

RobocopyPS
MIT License
51 stars 9 forks source link

Remove-RoboItem stops for input if robocopy fails to delete all sub items. #13

Closed fjollberg closed 2 years ago

fjollberg commented 2 years ago

https://github.com/sbergwall/RobocopyPS/blob/ca1653d37b632f0a52614e8cff79a99e18676e47/RobocopyPS/functions/Remove-RoboItem.ps1#L62

If Robocopy for some reason fails to remove all subitems Remove-RoboItem stops executing and waits for input with the following output:

Confirm The item at Microsoft.PowerShell.Core\FileSystem::\some\path has children and the Recurse parameter was not specified. If you continue, all children will be removed with the item. Are you sure you want to continue? [Y] Yes [A] Yes to All [N] No [L] No to All [?] Help (default is "Y"):

Behaviour can be simulated with:

> New-Item test\test -ItemType Directory
> Remove-Item -Path test -ErrorAction Stop

Not sure what the behaviour should be here. I think arguably the best way would be to test if the path is really empty before trying to remove it, so the script doesn't stop for input at this point.

Another way would be a -Recurse, but that has potentially odd effects if there are a lot of data left by Robocopy and sort of interferes with the behaviour of robocopy.

fjollberg commented 2 years ago

Having thought one moment more on this, I think a preferrable solution would be in the order of:

Get-Item -Path $tempDirectory, $Location | ForEach-Object { $_.Delete() }

An error would stop with:

MethodInvocationException: Exception calling "Delete" with "0" argument(s): "The directory is not empty. : '\some\path"

sbergwall commented 2 years ago

This should be fixed in the dev branch with version 0.2.13, but I will continue to work on the error handling for this function.

There current is a non-breaking bug that I know of. Lets say we try to remove a folder containing a file and we do not have access to remove it, if we run Remove-RoboItem with -ErrorAction Continue the error that will be shown will say the error is from Invoke-Robocopy but will still remove the remaining files in the folder we have access to.

PS> Remove-RoboItem -Path "C:\folder"
Invoke-RoboCopy : Deleting Extra File C:\folder\log.txt. Access is denied.

If we instead use -ErrorAction Stop the error handling will be correct but no more files will be removed, even if we have access to do so.

PS> Remove-RoboItem -Path "C:\folder" -ErrorAction Stop
Remove-RoboItem : Deleting Extra File C:\C:\folder\log.txt. Access is denied.

The bug is that even if we use -ErrorAction Continue the correct name of the function should be shown in the error message.

fjollberg commented 2 years ago

Yes, it's a bit non-trivial. The issue with hanging scripts due to remove-item is fixed in dev, much appreciated. But I'm still struggling a bit with error handling.

Currently, it seems like Remove-RoboItem returns on an error, but the Robocopy process is still running, causing the script to wait for the now seemingly detached process to die.

E.g. I get output such as this in some circumstances: Apr 27, 2022 5:29 AM [error] ERROR: RETRY LIMIT EXCEEDED.. *EXTRA Dir -1 \Some\path\some\item Apr 27, 2022 5:49 AM [error] Exception calling "Delete" with "0" argument(s): "The directory is not empty. : '\Some\path'"

After which my script proceeds to the end where it is waiting, and the robocopy.exe process is still running. I'm not 100% sure what it is doing at that point. Killing it causes the script to terminate.

My script proceeding also indicate that Remove-RoboItem returns with the property Success=$true.

I've added -InformationAction Ignore in order to get rid of a lot of ouput (these can be very, very large directory structures). I've now also added -ErrorAction Stop to see if that causes the script to properly detect an issue.

sbergwall commented 2 years ago

All this should be fixed in the master branch, version 0.2.15.