Details
- Call revalidateAccountPrekeys() to make sure checks work properly
- Call publishNewPrekeys() to make sure upload works properly.
- Check Identity logs for Refreshing prekeys for user: ...
- Check if time (last_prekey_publish_time) is updated after successful upload.
Test error:
- Delete user credentials (identity will fail to auth while refreshing keys)
- Trigger cron job
- check logs for encountered error while trying to validate prekeys
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Looks fine assuming that we always want to rotate both even if only one actually needs rotation. Make sure you get @varun acceptance before landing since I don't have enough context to review publishNewPrekeys
| keyserver/src/utils/olm-utils.js | ||
|---|---|---|
| 204 ↗ | (On Diff #31799) | We can take this console.log out |
Refreshing prekeys for user: ... just means the Identity service received the RefreshUserPreKeys request. Shouldn't we make sure that we actually received a successful response from the Identity service?
Also what happens if, say, the Identity service is offline? Does publishNewPrekeys fail gracefully? Maybe it doesn't matter since we only call it from a cron job?
We do. We throw an error when refresh_user_pre_keys call fails.
Checking refresh_user_pre_keys response doesn't make sense because it returns an empty response.
Checking rustAPI.publishPrekeys() response also doesn't make sense, because it always returns true. (Probably this is some convention in Rust codebase? Was introduced in D8473)
So basically we await rustAPI.publishPrekeys() and this means success or it throws an error.
Also what happens if, say, the Identity service is offline? Does publishNewPrekeys fail gracefully? Maybe it doesn't matter since we only call it from a cron job?
When identity is offline, or anything went wrong on identity, the cron job fails silently (see it's surrounded via try {} catch() {}. I think we can keep publishNewPrekeys to throw error and make caller's responsibility to handle this.
Checking rustAPI.publishPrekeys() response also doesn't make sense, because it always returns true. (Probably this is some convention in Rust codebase? Was introduced in D8473)
yeah not sure why we need a bool here at all, actually... could just be Result<()> instead