Explorar o código

Refactor message acknowledgements (and error handling upon reception)

With the upcoming ARP change, the Web Client will request that most
messages must be acknowledged after they have been *processed* by the
app. This is backwards compatible to the previous temporaryId stuff
which is now deprecated.

While comparing with the ARP documentation, a bunch of unhandled or
incorrectly handled error codes have been found.
Lennart Grahl %!s(int64=7) %!d(string=hai) anos
pai
achega
7c06391d44

+ 3 - 0
RELEASING.md

@@ -1,5 +1,8 @@
 # Releasing
 
+Major release with backwards incompatible changes? Check for `TODO` comments
+with deprecations. Remove them if possible.
+
 Set variables:
 
     $ export VERSION=X.Y.Z

+ 1 - 1
src/controller_model/avatar.ts

@@ -39,7 +39,7 @@ export class AvatarControllerModel {
                 $log.debug(this.logTag, 'loadAvatar: Requesting high res avatar from app');
                 webClientService.requestAvatar(receiver, true)
                     .then((data: ArrayBuffer) => resolve(data))
-                    .catch(() => reject());
+                    .catch((error) => reject(error));
             } else {
                 $log.debug(this.logTag, 'loadAvatar: Returning cached version');
                 resolve(receiver.avatar.high);

+ 3 - 1
src/controller_model/contact.ts

@@ -141,7 +141,9 @@ export class ContactControllerModel implements threema.ControllerModel<threema.C
             .then(() => {
                 this.isLoading = false;
             })
-            .catch(() => {
+            .catch((error) => {
+                // TODO: Handle this properly / show an error message
+                this.$log.error(`Cleaning receiver conversation failed: ${error}`);
                 this.isLoading = false;
             });
     }

+ 6 - 2
src/controller_model/distributionList.ts

@@ -125,7 +125,9 @@ export class DistributionListControllerModel implements threema.ControllerModel<
             .then(() => {
                 this.isLoading = false;
             })
-            .catch(() => {
+            .catch((error) => {
+                // TODO: Handle this properly / show an error message
+                this.$log.error(`Cleaning receiver conversation failed: ${error}`);
                 this.isLoading = false;
             });
     }
@@ -163,7 +165,9 @@ export class DistributionListControllerModel implements threema.ControllerModel<
         this.isLoading = true;
         this.webClientService.deleteDistributionList(this.distributionList).then(() => {
             this.isLoading = false;
-        }).catch(() => {
+        }).catch((error) => {
+            // TODO: Handle this properly / show an error message
+            this.$log.error(`Deleting distribution list failed: ${error}`);
             this.isLoading = false;
         });
     }

+ 9 - 3
src/controller_model/group.ts

@@ -140,7 +140,9 @@ export class GroupControllerModel implements threema.ControllerModel<threema.Gro
             .then(() => {
                 this.isLoading = false;
             })
-            .catch(() => {
+            .catch((error) => {
+                // TODO: Handle this properly / show an error message
+                this.$log.error(`Cleaning receiver conversation failed: ${error}`);
                 this.isLoading = false;
             });
     }
@@ -178,7 +180,9 @@ export class GroupControllerModel implements threema.ControllerModel<threema.Gro
             .then(() => {
                 this.isLoading = false;
             })
-            .catch(() => {
+            .catch((error) => {
+                // TODO: Handle this properly / show an error message
+                this.$log.error(`Leaving group failed: ${error}`);
                 this.isLoading = false;
             });
     }
@@ -212,7 +216,9 @@ export class GroupControllerModel implements threema.ControllerModel<threema.Gro
                     this.onRemovedCallback(this.group.id);
                 }
             })
-            .catch(() => {
+            .catch((error) => {
+                // TODO: Handle this properly / show an error message
+                this.$log.error(`Deleting group failed: ${error}`);
                 this.isLoading = false;
             });
     }

+ 1 - 1
src/controller_model/me.ts

@@ -156,7 +156,7 @@ export class MeControllerModel implements threema.ControllerModel<threema.MeRece
                 return this.webClientService.modifyProfile(
                     this.nickname,
                     this.avatarController.avatarChanged ? this.avatarController.getAvatar() : undefined,
-                ).then((val) => {
+                ).then(() => {
                     // Profile was successfully updated. Update local data.
                     this.webClientService.me.publicNickname = this.nickname;
                     if (this.avatarController.avatarChanged) {

+ 4 - 4
src/controllers/status.ts

@@ -278,12 +278,12 @@ export class StatusController {
                 };
             }
 
-            // ... if there is at least one pending request.
-            const pendingRequests = this.webClientService.pendingRequests;
-            if (pendingRequests > 0) {
+            // ... if there is at least one unacknowledged wire message.
+            const pendingWireMessages = this.webClientService.unacknowledgedWireMessages;
+            if (pendingWireMessages > 0) {
                 return {
                     send: true,
-                    reason: `${pendingRequests} pending requests`,
+                    reason: `${pendingWireMessages} unacknowledged wire messages`,
                 };
             }
 

+ 3 - 1
src/directives/avatar.ts

@@ -153,7 +153,9 @@ export default [
                                         $rootScope.$apply(() => {
                                             this.isLoading = false;
                                         });
-                                    }).catch(() => {
+                                    }).catch((error) => {
+                                        // TODO: Handle this properly / show an error message
+                                        $log.error(this.logTag, `Avatar request has been rejected: ${error}`);
                                         $rootScope.$apply(() => {
                                             this.isLoading = false;
                                         });

+ 2 - 1
src/directives/message.ts

@@ -159,7 +159,8 @@ export default [
                                 });
                             })
                             .catch((error) => {
-                                $log.error(this.logTag, 'Error downloading blob:', error);
+                                // TODO: Handle this properly / show an error message
+                                $log.error(this.logTag, `Error downloading blob: ${error}`);
                                 this.downloading = false;
                             });
                     };

+ 6 - 1
src/directives/message_media.ts

@@ -175,7 +175,12 @@ export default [
                                             .then((img) => $timeout(() => {
                                                 setThumbnail(img);
                                                 this.thumbnailDownloading = false;
-                                            }));
+                                            }))
+                                            .catch((error) => {
+                                                // TODO: Handle this properly / show an error message
+                                                const message = `Thumbnail request has been rejected: ${error}`;
+                                                this.$log.error(this.logTag, message);
+                                            });
                                     }, 1000);
                                 }
                             }

+ 5 - 4
src/partials/messenger.ts

@@ -595,7 +595,7 @@ class ConversationController {
                                 msg.caption = caption;
                             }
                             msg.sendAsFile = sendAsFile;
-                            this.webClientService.sendMessage(this.$stateParams, type, true, msg)
+                            this.webClientService.sendMessage(this.$stateParams, type, msg)
                                 .then(() => {
                                     nextCallback(index);
                                 })
@@ -618,7 +618,7 @@ class ConversationController {
                         // TODO: This should probably be moved into the
                         //       WebClientService as a specific method for the
                         //       type.
-                        this.webClientService.sendMessage(this.$stateParams, type, true, msg)
+                        this.webClientService.sendMessage(this.$stateParams, type, msg)
                             .then(() => {
                                 nextCallback(index);
                             })
@@ -1225,8 +1225,9 @@ class ReceiverDetailController {
                     this.hasSystemEmails = contactReceiver.systemContact.emails.length > 0;
                     this.hasSystemPhones = contactReceiver.systemContact.phoneNumbers.length > 0;
                 })
-                .catch(() => {
-                    // do nothing
+                .catch((error) => {
+                    // TODO: Redirect or show an alert?
+                    $log.error(this.logTag, `Contact detail request has been rejected: ${error}`);
                 });
 
             this.isWorkReceiver = contactReceiver.identityType === threema.IdentityType.Work;

+ 9 - 5
src/services/message.ts

@@ -140,13 +140,16 @@ export class MessageService {
     }
 
     /**
-     * Create a message object with a temporaryId
+     * Create a message object with a temporary id
      */
-    public createTemporary(receiver: threema.Receiver, msgType: string,
-                           messageData: threema.MessageData): threema.Message {
-        const now = new Date();
+    public createTemporary(
+        temporaryId: string,
+        receiver: threema.Receiver,
+        msgType: string,
+        messageData: threema.MessageData,
+    ): threema.Message {
         const message = {
-            temporaryId: receiver.type + receiver.id + Math.random(),
+            temporaryId: temporaryId,
             type: msgType,
             isOutbox: true,
             state: 'pending',
@@ -165,6 +168,7 @@ export class MessageService {
         }
 
         // Add delay for timeout checking
+        // TODO: This should be removed. It either works or it doesn't. There's nothing in between.
         this.$timeout(() => {
             // Set the state to timeout if it is still pending.
             // Note: If sending the message worked, by now the message object

A diferenza do arquivo foi suprimida porque é demasiado grande
+ 302 - 255
src/services/webclient.ts


+ 8 - 0
src/threema.d.ts

@@ -26,12 +26,20 @@ declare namespace threema {
         high?: ArrayBuffer;
     }
 
+    interface WireMessageAcknowledgement {
+        id: string,
+        success: boolean,
+        error?: string,
+    }
+
     /**
      * Messages that are sent through the secure data channel as encrypted msgpack bytes.
      */
     interface WireMessage {
         type: string;
         subType: string;
+        id?: string;
+        ack?: WireMessageAcknowledgement;
         args?: any;
         data?: any;
     }

Algúns arquivos non se mostraron porque demasiados arquivos cambiaron neste cambio