bug: update command breaks when tldr repo has bad merge

[SethFalco]

On numerous occasions, TL;DR has accepted a merge request which doesn't follow the required directory structure.

The most recent example is accepting a file under /pages.es/common/asar.md/asar.md. When this happens, I would expect the client to continue to handle updates gracefully. This way, when the issue is resolved, users can get an update to the fixed version.

In this instance, it impacted a user that asked a question in our Matrix chat room with the following issue:

Error: EISDIR: illegal operation on a directory, unlink '/home//.tldr/cache/pages.es/common/asar.md'

— https://matrix.to/#/!zXiOpjSkFTvtMpsenJ:gitter.im/$5CbE27s7bbsnwTiQX8UTYliy6lrazT-cx9cxJjkO9TY?via=gitter.im&via=matrix.org&via=one.ems.host

Expected behavior

Verify integrity

The update flow could have an intermediate step to "verify file integrity", whenever an update is fetched.

When grabbing the zip file, store and extract the contents in a separate directory and make sure the directory structure looks as expected before overwriting the real cache.

When pulling from the git repo, store the last good commit hash, and after pulling if the directory structure looks off, reset back.

Clear Cache on Error

If a non-network related error occurs during an update, instruct the user to try to invalidate the cache and try again. (We shouldn't do this automatically, or at least not without doing a HEAD request to the download location to ensure we can connect to it.)

Actual behavior

If the user performs an update and gets a bad version of tldr-pages, the update command stops working.

Log, debug output

Error: EISDIR: illegal operation on a directory, unlink '/home/<user-name>/.tldr/cache/pages.es/common/asar.md'

Environment

• Operating system - ?
• Node.js version (node --version) - ?
• tldr-node-client version (tldr --version) - ?

[agnivade]

I am wondering if we should be adding files that don't adhere to the directory structure in the first place. This should be handled by a CI job in the tldr repo that verifies that the directory structure is not broken. If we can prevent this from happening in the first place, that will be even better. Because otherwise, the client can choose not to update the cache, but then it can never update any further until it has been fixed in the tldr repo.

[SethFalco]

I am wondering if we should be adding files that don't adhere to the directory structure in the first place.

We shouldn't, but it's always going to be possible to do. Also, keep in mind this issue isn't just about directory structures either, it's about any unexpected non-network related IO error that could occur.

This should be handled by a CI job in the tldr repo that verifies that the directory structure is not broken. If we can prevent this from happening in the first place, that will be even better.

I agree we should prevent it from happening in the first place, and in Matrix it was already discussed to improve our pipelines. However, other unexpected issues my arise that aren't caught by CI even if this particular scenario is handled.

1. A maintainer doing a force merge, intentionally or unintentionally.
2. A bug that causes some pipelines to pass falsely even in this scenario.
3. A different issue entirely, like linked files, permissions, or any other potential complication that may lead to an IO error.

Software shouldn't trust dependencies fetched at runtime, or assume they'll return exactly what's expected. As far as this repo is concerned, tldr-pages/tldr is an external resource. Things breaking are 100% fine, but the client not resolving or guiding the user on how to run the very updates that would solve it is unsatisfactory imo.

[agnivade]

Yep I agree. The checks should be there in both places.