[#1165] Fix ENOENT K3s downloads

[evertonlperes]

Resolves: #1165
Unblock @rak-phillip PR: #1199

Changes

1. Removed unnecessary delete k3s tmp directory action.

1. Replaced deprecated rmdir to rm({recursive: true, maxRetries: 3, force: true}).

How to replicate the issue

The issue can be replicated (with accuracy), after deleting k3s local cache folder. If you have any version locally that matches with you k8s selection (RD K8s version), it won't fail.

[jandubois]

I do not understand why this has become a problem now, when it seems to have worked for a long time before.

I disagree that deleting the temp directory is not necessary; the finally clause exists to clean up when something has failed. So it should only be skipped when the final safeRename has been successful.

I think a better fix would be to use fs.promises.rm instead with the force: true option, which will not throw an exception when the directory does not exist.

Also: it looks like the recursive option for rmdir is deprecated, which would be another reason to use rm instead.

[evertonlperes]

I do not understand why this has become a problem now, when it seems to have worked for a long time before.

I disagree that deleting the temp directory is not necessary; the finally clause exists to clean up when something has failed. So it should only be skipped when the final safeRename has been successful.

I think a better fix would be to use fs.promises.rm instead with the force: true option, which will not throw an exception when the directory does not exist.

Also: it looks like the recursive option for rmdir is deprecated, which would be another reason to use rm instead.

Thanks for your thoughts @jandubois
I'm also not sure which is the root cause, we had a major changes on Electron version + Node update and it will be tough to track it down.
Just a bit more context (and braimstorming :D ), at first glance, this is behaviour before my changes was:

• Create a tmp k3s folder appending random prefixes (line 404)
• Check if the checksum match will pass
• Rename the tmp folder (line 434)
• Delete created tmp folder (line 436) --> where the failure will occurs. there's no more tmp folder, it was renamed before being deleted.
  I could be wrong, but the rmdir was trying to delete a folder that does not exists, that's why it threw an error. The workDir doesn't contains the tmp-prefix anymore.

Could be related to the deprecation of rmdir after we upgraded the Node version and I'll take a look into it.
Thanks for the feedback.

[rak-phillip]

It looks like the deprecated usage of fs.rmdir(path, { recursive: true }) is found in several places. We would probably be proactive and update each instance if this is the root cause of the problem.

Created #1198 to track the work

[jandubois]

I've looked a bit more into the history, and it seems that recursive rmdir was supposed to start throwing an exception when the directory doesn't exist, starting with node@16: nodejs/node#34278 (comment)

This explains why we are only seeing this error now, and that my proposed change seems the correct way to deal with it (instead of not trying to clean up the directory at all).

[jandubois]

• I could be wrong, but the rmdir was trying to delete a folder that does not exists

Yes, that is correct, but the folder is only gone when the try block has finished successfully. If any of the operations before had thrown an error, the directory would still exist. And you definitely wouldn't want to safeRename it in the error case because the pre-processing was not done successfully. You would either just leave it behind, or better, remove it.