Avni Entity Observation Media fields not present in S3

[himeshr]

Describe the bug
We see that there are several Media objects that are missing in S3 folder of the organisation, corresponding to Avni entities (Subject , Encounter, etc..) observation fields.

These show up as broken image-panes in Media-viewer.

To Reproduce
Root-cause is unknown and also steps for reproduction are not known as of now.

Expected behavior
As part of Avni-client sync, we should always upload the Media to S3 and update the Observation fields with the S3 url, after that. If not done, fail Sync till we can do that.
There after, we should not have missing S3 media content corresponding to observation fields of any entity.

Screenshots

Additional context
We had recently done some optimizations with Avni-client Media Upload. This could have resulted in missing out some media content uploads in corner case scenrios. This is just an assumption, which should be taken up as a starting point.

**APK details **

• Avni-client: 5.0.3 Playstore version
  - Only use Goonj Revamp UAT org users for attempting issue reproduction. Do not use Goonj Prod users for any debug or development.

Developer Checklist
Developer fixing the bug should fill this checklist.

• Does the fix require extensive regression testing?
• Are you mentioning the required scenarios that could be affected?

[1t5j0y]

Able to reproduce this issue by interrupting network during media sync (turn data off/on, airplane mode on/off) while on a slow connection. This results in the images not being successfully upload and a 'Broken pipe' message in the logs after approx 10 mins which is not treated as an error by the library we are using to upload images (rn-fetch-blob) and the happy path (updating observations with s3 url etc) proceeds.
Additionally, even if the media upload throws an error, it was only being logged and hence data sync would continue even with media upload failures.

Fixes tried:

1. Use a newer fork of the library in use (since it is not being maintained) to see if this has been fixed. It has not.
2. Set a timeout for the upload so it fails before the 'Broken pipe' message. Doesn't seem to work reliably.

Fix being worked on:

• Use the uploadProgress callback to fail fast if the upload is stuck so the media remains in the media queue to be processed in the next sync.
• Also, propagate errors during media sync so that data sync does not proceed if media sync fails.

[vinayvenu]

This is a behaviour change from before.

Earlier, we did not treat s3 failures as a blocker for sync of regular transactional data. Because of this, if you have an S3 downtime, users will not be affected. These images will be uploaded at a later time when S3 becomes available.

This change could potentially bring in probems we might earlier have had with the s3 upload to the foreground.

Propose moving forward with this comment.

[1t5j0y]

We have the option of including the change to timeout uploads that are not making progress and removing the error propagation if we want to retain previous sync behaviour (don't fail sync if media sync has issues).

Few potential issues if we do so are:

1. Enhanced Validation (if enabled) will fail image observations since it expects a URL and in this case, only a uuid will be sent.
2. Media viewer will have missing images.

[1t5j0y]

Test scenarios:

1. Sync with media upload - regression to check no negative impact
2. Sync with media over slow connection
3. Sync with media over slow connection and interrupt connection (airplane mode / mobile data connection on off)

Check to see that data sync does not proceed if media sync fails.
Check that media eventually syncs (for slow connection scenarios above) and data sync proceeds after that.

[himeshr]

We have the option of including the change to timeout uploads that are not making progress and removing the error propagation if we want to retain previous sync behaviour (don't fail sync if media sync has issues).

Few potential issues if we do so are:

1. Enhanced Validation (if enabled) will fail image observations since it expects a URL and in this case, only a uuid will be sent.
2. Media viewer will have missing images.

Hi Joy, Vinay has stated in his comment, that though this is a change in behaviour, we should still go ahead with it.
"Propose moving forward with this comment." adheres to this, i think the comment led to some confusion.

[1t5j0y]

Yes, I got that. No confusion. Was just highlighting that we still have the option to do this in an incremental way without changing the sync behaviour and the potential issues.

[himeshr]

Yes, I got that. No confusion. Was just highlighting that we still have the option to do this in an incremental way without changing the sync behaviour and the potential issues.

Oh okay. While making the chocie, i would like to reiterate Goonj's perspective that, its critical that media uplaod failures are recorded correctly and there are no missing files in S3.