Element-R: reduce log spam when checking server key backup #3826
Conversation
| @@ -460,7 +460,7 @@ export class RustBackupDecryptor implements BackupDecryptor { | |||
| for (const [sessionId, sessionData] of Object.entries(ciphertexts)) { | |||
| try { | |||
| const decrypted = JSON.parse( | |||
| await this.decryptionKey.decryptV1( | |||
| this.decryptionKey.decryptV1( | |||
There was a problem hiding this comment.
this is slightly separate, but a cleanup while I was in the area.
decryptionKey.decryptV1 doesn't return a promise.
There was a problem hiding this comment.
Yeah I have seen it now that I use WebStorm. I am wondering if you shouldn't update the bindings to make it async?
There was a problem hiding this comment.
Added an await immediate() in 131c524 to make sure we don't block the event loop.
Fixes a lot of spam in the logs about "uncaught in promise: No room_keys found".
richvdh
force-pushed
the
rav/element-r/silence_backup_spam
branch
from
c70f48f to
217fef9
Compare
| @@ -460,7 +460,7 @@ export class RustBackupDecryptor implements BackupDecryptor { | |||
| for (const [sessionId, sessionData] of Object.entries(ciphertexts)) { | |||
| try { | |||
| const decrypted = JSON.parse( | |||
| await this.decryptionKey.decryptV1( | |||
| this.decryptionKey.decryptV1( | |||
There was a problem hiding this comment.
Yeah I have seen it now that I use WebStorm. I am wondering if you shouldn't update the bindings to make it async?
| const path = encodeUri("/room_keys/keys/$roomId/$sessionId", { | ||
| $roomId: targetRoomId, | ||
| $sessionId: targetSessionId, | ||
| this.queryKeyBackup(targetRoomId, targetSessionId).catch((e) => { |
There was a problem hiding this comment.
I think we might want to check if we have the private key before marking the check as attempted (sessionLastCheckAttemptedTime = now). Because if we attempt to get the key and you don't have the private key yet (verification not yet done) it will not retry until KEY_BACKUP_CHECK_RATE_LIMIT. So when you verify after login, you'll have to wait?
KEY_BACKUP_CHECK_RATE_LIMIT isn't that high currently, so maybe not too important though.
There was a problem hiding this comment.
I think we might want to check if we have the private key before marking the check as attempted (sessionLastCheckAttemptedTime = now).
The problem with that is that it is racy: if two calls to startQueryKeyBackupRateLimited happen in quick succession (eg, because there are two messages in a sync response), then both will check sessionLastCheckAttemptedTime and find it ok, and go on to make a backup request. We need to make sure there is no await between the check and the update on sessionLastCheckAttemptedTime.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
cypress failures: element-hq/element-web#26437, element-hq/element-web#26447, element-hq/element-web#26452 |
Fixes a lot of spam in the logs about "uncaught in promise: No room_keys found".
This change is marked as an internal change (Task), so will not be included in the changelog.