[lib] Handle identity timeout when fetching device lists
ClosedPublic
Actions

Authored by inka on Oct 18 2024, 3:36 AM.

Details

Reviewers

tomek
kamil

Commits

rCOMM601124623861: [lib] Handle identity timeout when fetching device lists

Summary

issue: ENG-9725

Test Plan

Tested that if the timeout occurs and the error is thrown, the accountMissingStatus of users is not updated and the user is not deleted, even if they were missing a device list

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Oct 18 2024, 3:36 AM

Herald added a subscriber: ashoat. · View Herald TranscriptOct 18 2024, 3:36 AM

inka added a parent revision: D13741: [lib] Delete users for whom identity didnt return a device list for a long time.Oct 18 2024, 3:36 AM

inka added inline comments.Oct 18 2024, 3:42 AM

lib/handlers/user-infos-handler.react.js
171–173 ↗	(On Diff #45276)	If a timeout occurred, we actually want to query for those users again
lib/hooks/peer-list-hooks.js
60–64 ↗	(On Diff #45276)	I don't think it's worth merging this with the function used in `UserIdentityCacheProvider`. They are so simple that merging them would just introduce complexity at the call site.
lib/utils/identity-service.js
48 ↗	(On Diff #45276)	The timeout before an error on the native side is thrown is 30s D13355 Ideally we would be getting informed by the native side about the timeout and only handling it on the JS side. But because the native error handling is broken for now (ENG-9730) I am using this solution that is mimicking the solution from `UserIdentityCacheProvider` which is also working around this problem. Using the same value as `UserIdentityCacheProvider`.

Harbormaster completed remote builds in B32305: Diff 45276.Oct 18 2024, 3:54 AM

inka requested review of this revision.Oct 18 2024, 3:54 AM

tomek accepted this revision.Oct 21 2024, 3:52 AM

tomek added inline comments.

lib/handlers/user-infos-handler.react.js
171–173 ↗	(On Diff #45276)	Sounds risky. After the request timed out, the next request would be more expensive, which is going to increase the service's load. When a service is overloaded, we're going to make it worse.

This revision is now accepted and ready to land.Oct 21 2024, 3:52 AM

inka added inline comments.Oct 21 2024, 6:07 AM

lib/handlers/user-infos-handler.react.js
171–173 ↗	(On Diff #45276)	Talked about this offline and concluded that this should be left. This handler makes sure to request every user exactly once. But if a timeout occurred it's as if we didn't really query fort them. Moreover, deleting all `usersWithMissingDeviceList` from `deviceListIDsToRemove` doesn't result in an immediate request to identity. They will simply be queried for the next time this effect is run.