GetPublii / Publii

The most intuitive Static Site CMS designed for SEO-optimized and privacy-focused websites.
https://getpublii.com
GNU General Public License v3.0
6.21k stars 416 forks source link

[Feature Request]: Consider changes to `sftp` deployments #1185

Open michaellenaghan opened 1 year ago

michaellenaghan commented 1 year ago

Feature Description

I recently ran into a case where an sftp (image-heavy) deployment seemed to pause mid-deployment, and then suddenly complete — without signalling an error, and without actually uploading the images. Subsequent deployments didn't upload them either. In fact, they didn't even try.

Looking at the code, I think I see the following issues:

That combination of issues explains why, once there's an error, there's no recovery; the new list of files will "always" be uploaded, and Publii will "always" download it and assume it's correct. In other words, it will assume that it uploaded files it didn't. (Or, say, that it removed files it didn't.)

I'd be happy to put together a pull request, but there's a third issue that would, I think, require your involvement.

It looks like the entire deployment happens through mutually recursive functions; the deployment "coordinator's" uploadFile() calls the deployment "driver's" uploadFile(), which then calls the deployment "coordinator's" uploadFile()... There doesn't seem to be a way for the deployment driver to tell the deployment coordinator that an error occurred, and that it should abort the deployment? I think you'd need to add that; there would be too many decisions to make.

Btw, there's another possible problem with those mutually recursive functions: the stack will grow in proportion to the number of files being uploaded. Even a "small" Publii image-heavy site could, in theory, have a lot of files, since Publii seems to be able to automatically create many responsive variants of the same image?

P.S. I just spotted another issue: promises aren't always being used correctly. Here, for example, the promise should be returned. See this "common error" in ssh2-sftp-client's README. (All of the calls to this.connection.end() have the same problem; none of them are returned into the promise chain, and all of them should be.)