serializedPayload variable here keeps olm-encrypted stringified notification payload. However in the notification that is send to the client we keep in under encryptedPayload key so we need to validate the size of the entire { encryptedPayload: serializedPayload.body } JSON not just the data we get out of olm encrytion.

for each Android notification we include information whether its payload (regardless of whether encryption was successful or not) exceeds some limit. If notification is encrypted and payloadSizeViolated is true then it means that encryption was successful, notification payload after encryption exceeds limit and hence, olm session state in database was not updated. We can further use this information to remove some data from notification, encrypt smaller notification and send it to the client without worrying that they will have to skip and infinitely keep some keys.

Similarly, we have to check the size of entire notification that we send {data: {id, badgeOnly, encryptedPayload: string}} not just raw data we get out of encryption.

This change was never needed. Notification text is obtained with function notifTextsForMessageInfo that does necessary string trimming to at most 300 characters. So if we don't include messageInfos property (which is not trimmed) we are safe within FCM limits.

We shouldn't allow both of messageInfos and blobHash: string, encryptionKey: string to be present in notification at the same time, since the reason we include blobHash and encryptionKey is that notification with messageInfos was too large to be sent.

Is there a reason this one isn't read-only?

Let's stick with >=

Please keep all lines to a max of 80 chars

await should only appear as the first keyword in a line, or the first keyword on the right-hand side of an assignment

Flow generally gets confused when we use constructions like this (cc @bartek who dealt with it a lot)

Instead we should define the shared part and then have a union that spreads the shared part in each joined type:

type SharedAndroidNotificationPayload = {
  +badge: string,
  +body?: string,
  +title?: string,
  +prefix?: string,
  +threadID?: string,
  +encryptionFailed?: '1',
};

export type AndroidNotificationPayload =
  | {
      ...SharedAndroidNotificationPayload,
      +messageInfos?: string,
    }
  | {
      ...SharedAndroidNotificationPayload,
      +blobHash: string,
      +encryptionKey: string,
    };

Do we really want to set it to NEXT_CODE_VERSION and not the FUTURE_ one? Services access token stuff might not be ready by that time

Definitely no reason. My oversight.

That's valid point. Every usage of NEXT_CODE_VERSION should be replaced with FUTURE_CODE_VERSION.

It is not encrypted. blobServiceUpload function AES-encrypts it before sending to blob service.

blobServiceUpload function returns blobUploadError if upload fails or blobHash and encryptionKey if it succeeds. So basically this condition is true if either blobUploadError is truthy or none of blobHash, encryptionKey, blobUploadError is truthy. The latter happens if canQueryBlobService is falsy. This code would be more readable if I defined blobHash, encryptionKey, blobUploadError inside canQueryBlobService if statement. On the other hand it would increase indentation. I wanted to stick to conventions even if it meant writing code that takes a bit more time to understand.

I will introduce this change shortly.

It is possible that for some devices notification encryption fails while for some succeeds. In such case it is possible that unencrypted notification with messageInfos does not exceed size limit while encrypted notification with messageInfos exceeds zie limits.

It is a little bit of an edge-case, so if you consider it an over-engineering I can change this behaviour to check encrypted notification size for only one device. I wasn't sure myself but eventually I decided if it doesn't complicate code too much to cover all cases then we should do so.