modules: fix lstat error on outDir. Closes #465

[juliangruber]

@bajtos I have finally been able to reproduce the lstat errors that are reported so often. And for me, the only way to recover was this quick fix. I didn't have time to look into it more properly.

Since the diff is indentation heavy, the gist is this: Remove the try/catch that removes outDir when the download failed. I don't know why this fixes it, or whether this introduces regressions.

Closes #465

[bajtos]

Since the diff is indentation heavy

It's easy to tell GitHub to ignore whitespaces and fortunately, your PR is very easy to understand that way.

https://github.com/filecoin-station/core/pull/461/files?diff=unified&w=1

[bajtos]

I have finally been able to reproduce the lstat errors that are reported so often.

Awesome!

And for me, the only way to recover was this quick fix. I didn't have time to look into it more properly.
Since the diff is indentation heavy, the gist is this: Remove the try/catch that removes outDir when the download failed. I don't know why this fixes it, or whether this introduces regressions.

I guess await rm(outDir, { recursive: true }) is calling lstat when recursively deleting files & directories, and that operation fails. When you remove the recursive removal, we no longer call lstat. That is my assumption as to why your fix works.

I found the following GH issues mentioning lstat:

• Core - docker Failed to download voyager source #393
• Failed to download latest Zinnia module source code desktop#1447
• Last week: no rewards / failed to download latest Zinnia module source code desktop#1399

They all are caused by lstat failing on $ROOT/sources/$MODULE.

Regarding regressions: I think your change is going to break the logic detecting when it's safe to skip downloading the version because we have already downloaded it before; see here:

https://github.com/filecoin-station/core/blob/9d112af58a47119c01b39d8940c80ce08577594c/lib/modules.js#L145-L155

[juliangruber]

what about this: ignore enoent errors here

Isn't this going to break #336?

Prevent that source code updates create inconsistent folder structures, when residue files are kept around

[bajtos]

Other than the comment above, the change LGTM 👍🏻

I am fine with shipping it and seeing how it works in practice.

[juliangruber]

Other than the comment above, the change LGTM 👍🏻

I am fine with shipping it and seeing how it works in practice.

Which comment above? Do you mean my comment that you edited? 😅

[juliangruber]

what about this: ignore enoent errors here

Isn't this going to break #336?

Prevent that source code updates create inconsistent folder structures, when residue files are kept around

It isn't going to break #336, because in the case of an ENOENT, there is no directory to clean up in the first place.

Let's ship it and see 👍

[bajtos]

Do you mean my comment that you edited?

😱

I wanted to copy the content from your comment to quote it in my comment. Apparently, I modified your comment instead. Sorry for that! 🙈

what about this: ignore enoent errors here

Isn't this going to break #336?

Prevent that source code updates create inconsistent folder structures, when residue files are kept around

It isn't going to break #336, because in the case of an ENOENT, there is no directory to clean up in the first place.

Ah, I was under the assumption that ENOENT could also be triggered for nested directories. However, all bug reports show only ENOENT for the top-level directory. Your assessment looks correct to me now. 👍🏻