Importance of Linter Rules

December 28, 2023

This post contains an analysis of a recently discovered issue in our iOS app, that affected how we processed Live Photos in certain scenarios.

Summary

  • A missing await resulted in zips that were not complete while we processed Live Photos from the iOS app
  • Based on anecdotal evidence, and the discovery time (2+ years), we believe a very small percentage of files & users were affected by this bug
  • Hot fix and client side migration change was pushed (v0.8.13) on iOS. This update would identify and re-upload affected items
  • We have since enabled linter rules to avoid such bugs in the future

What is a Live Photo?

On iOS, a Live Photo consists of two separate assets: one image and the corresponding video. The video duration is typically 2-3 seconds. They're similar to Android's Motion Photos in terms of experience, but there are differences in implementation details.

Personally, I love Live Photos, and more than half of my mobile clicks are live photos. Naturally, this was one of the first major feature that I worked on after joining Ente in July 2021. We had released the mobile app with Live Photo support in Aug 2021.

How Ente stores Live Photos

For maintaining metadata privacy, Ente doesn't want to know the type of file. In Live Photo, both image and video assets belong to same entity, so it made sense to create and encrypt a single zip file for Live Photos that contains both assets. The client apps can decrypt file metadata to identify whether a particular entity is of type: image, video or Live Photo. Based on the entity type, client decrypts the encrypted file, unzips it (in case of Live Photos) and displays the corresponding image and video on the UI.

Here's a reference for the dart code that we used for preparing the zip file.

var encoder = ZipFileEncoder();
encoder.create(livePhotoPath);
encoder.addFile(videoUrl, "video" + extension(videoUrl.path));
encoder.addFile(sourceFile, "image" + extension(sourceFile.path));
encoder.close();

The Bug

We were informed on the 30th of November that data export was failing for 2 files for a customer. These were only happening to Live Photos, and from the logs the export was breaking during the unzipping of the file.

Here, note that before unzipping, we decrypt the file. After decryption, based on file's metadata, the app tried to unzip the decrypted file & encountered above error.

Our immediate guess was that a file's metadata contain incorrect type information, and this file might be just normal photo. This can happen if the user turned off the Live video for a photo using the iOS Photos app. In such cases, the app will detect something has changed and re-upload the file. We assumed that due to a regression, the app might not have updated the fileType correctly.

But since from reports only live photos were facing this issue, and while unzipping, we became suspicious that may be there's something wrong the the zip operation, which we never imagined since in more than 2 years we hadn't run into this. Deeper inspection of the code revealed the issue.

In ZipFileEncoder, the addFile method is marked as async, and we were not awaiting on that operation.

Future<void> addFile(File file, [String? filename, int? level = GZIP]) async {
    // ...
}

After creating the zip, we add both the video and image files and we then delete the items that were copied to the app's storage. So if the delete operation succeeds before the zip creation is completed, it could have resulted in this behaviour. Due to the missing await, the error addFile would get ignored.

The Fix

We fixed the issue immediately & shipped client side migration scripts to re-scan and re-upload all Live Photos if the zip file size on remote is different from file size on the device.

We also enabled linter rule unawaited_futures, which would give us a warning in our IDE for any unawaited futures.

Linter Error Rules

For those who don't know, here's a brief description of Linter (via ChatGPT)

A linter is a tool that analyzes source code to detect and flag programming errors, bugs, stylistic errors, and other issues. It helps maintain code quality and consistency. Linter rules are predefined guidelines or configurations that the linter follows to identify and report specific types of issues in the code. These rules can cover various aspects, such as syntax errors, code style conventions, and potential bugs. Using a linter with appropriate rules can contribute to writing cleaner, more efficient, and error-free code.

You might wonder: Wait, so you are saying that this bug would have never happened if you had setup Linter on your code! Why was this not done already?

We did setup code Linter in July 2021 (before Live Photo implementation), and over the period of time, we have added more linter rules, for both code style and error detection.

Unfortunately, all of this, including Live Photo support, was done before we finished major Null-Safety migration and some of the recommended rules (both of type style & error) were very noisy. This doesn't not mean that the Linter rule was giving incorrect suggestion. It's just, some of their suggestions were not actually bugs, and not acting on those suggestions would not have broken anything.

In the case of unawaited_future linter rule, it would trigger warnings even for side effect free methods related to showing toasts & page navigation.

Unfortunately, we took the decision to not enable unawaited_future rule. In the hindsight, that was an expensive mistake. We have learned our lesson and have started enabling linter error rules across our code bases.

Learnings

  • Never underestimate linter rules that help in catching human errors
  • Regularly run exports on test accounts to ensure data integrity

Conclusion

We are very grateful to the customer who notified us and helped us through the process of finding the root cause.

If you have uploaded Live Photos from our iOS app to Ente, please upgrade your app to v0.8.13 or higher and verify if you were impacted by this issue by exporting your data using our desktop app or CLI. If you were impacted, please reach out to [email protected].

We strive to write safe code, but this was an instance where we should have done better. We apologise, we will strive to do better.