Catch exceptions when encrypting events

If an exception was thrown by the encryption process, the event would be
queued, but the exception would not be handled. This meant that the event got
stuck as a grey 'sending' event in the UI.

Fixing this correctly is slightly more complex than just handling the exception
correctly. A naive approach would mean that the event would be shown as a red
'unsent' message, and clicking 'resend' would then send the message *in the
clear*. Hence, move the encryption to _sendEvent, where it will be called again
when the event is resent.

lib/client.js

@@ -1117,26 +1117,56 @@ MatrixClient.prototype.sendEvent = function(roomId, eventType, content, txnId,
         room.addPendingEvent(localEvent, txnId);
     }
 
-    if (eventType === "m.room.message" && this.sessionStore && CRYPTO_ENABLED) {
-        var e2eRoomInfo = this.sessionStore.getEndToEndRoom(roomId);
-        if (e2eRoomInfo) {
-            var encryptedContent = _encryptMessage(
-                this, roomId, e2eRoomInfo, eventType, content, txnId, callback
-            );
-            localEvent.encryptedType = "m.room.encrypted";
-            localEvent.encryptedContent = encryptedContent;
-
-            // TODO: Specify this in the event constructor rather than fiddling
-            // with the event object internals.
-            localEvent.encrypted = true;
-        }
-    }
-
     return _sendEvent(this, room, localEvent, callback);
 };
 
-function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content,
+
-                               txnId, callback) {
+/**
+ * Encrypt an event according to the configuration of the room, if necessary.
+ *
+ * @param {MatrixClient} client
+ * @param {module:models/event.MatrixEvent} event  event to be sent
+ *
+ * @private
+ */
+function _encryptEventIfNeeded(client, event) {
+    if (event.isEncrypted()) {
+        // this event has already been encrypted; this happens if the
+        // encryption step succeeded, but the send step failed on the first
+        // attempt.
+        return;
+    }
+
+    if (event.getType() !== "m.room.message") {
+        // we only encrypt m.room.message
+        return;
+    }
+
+    if (!client.sessionStore) {
+        // End to end encryption isn't enabled if we don't have a session
+        // store.
+        return;
+    }
+
+    var roomId = event.getRoomId();
+
+    var e2eRoomInfo = client.sessionStore.getEndToEndRoom(roomId);
+    if (!e2eRoomInfo || !e2eRoomInfo.algorithm) {
+        // not encrypting messages in this room
+        return;
+    }
+
+    var encryptedContent = _encryptMessage(
+        client, roomId, e2eRoomInfo, event.getType(), event.getContent()
+    );
+    event.encryptedType = "m.room.encrypted";
+    event.encryptedContent = encryptedContent;
+    // TODO: Specify this in the event constructor rather than fiddling
+    // with the event object internals.
+    event.encrypted = true;
+}
+
+function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content) {
     if (!client.sessionStore) {
         throw new Error(
             "Client must have an end-to-end session store to encrypt messages"
@@ -1200,6 +1230,16 @@ function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content,
     }
 }
 
+
+/**
+ * Decrypt a received event according to the algorithm specified in the event.
+ *
+ * @param {MatrixClient} client
+ * @param {MatrixEvent} event
+ *
+ * @return {MatrixEvent} a new MatrixEvent
+ * @private
+ */
 function _decryptMessage(client, event) {
     if (client.sessionStore === null || !CRYPTO_ENABLED) {
         // End to end encryption isn't enabled if we don't have a session
@@ -1289,8 +1329,17 @@ function _badEncryptedMessage(event, reason) {
     }, event);
 }
 
+// encrypts the event if necessary
+// adds the event to the queue, or sends it
+// marks the event as sent/unsent
+// returns a promise which resolves with the result of the send request
 function _sendEvent(client, room, event, callback) {
-    var defer = q.defer();
+    // Add an extra q() to turn synchronous exceptions into promise rejections,
+    // so that we can handle synchronous and asynchronous exceptions with the
+    // same code path.
+    return q().then(function() {
+        _encryptEventIfNeeded(client, event);
+
         var promise;
         // this event may be queued
         if (client.scheduler) {
@@ -1309,21 +1358,26 @@ function _sendEvent(client, room, event, callback) {
         if (!promise) {
             promise = _sendEventHttpRequest(client, event);
         }
-
+        return promise;
-    promise.done(function(res) {  // the request was sent OK
+    }).then(function(res) {  // the request was sent OK
         if (room) {
             room.updatePendingEvent(event, EventStatus.SENT, res.event_id);
         }
-
+        if (callback) {
-        _resolve(callback, defer, res);
+            callback(null, res);
+        }
+        return res;
     }, function(err) {
         // the request failed to send.
+        console.error("Error sending event", err.stack || err);
+
         _updatePendingEventStatus(room, event, EventStatus.NOT_SENT);
 
-        _reject(callback, defer, err);
+        if (callback) {
+            callback(err);
+        }
+        throw err;
     });
-
-    return defer.promise;
 }
 
 function _updatePendingEventStatus(room, event, newStatus) {

spec/integ/matrix-client-retrying.spec.js

@@ -1,4 +1,5 @@
 "use strict";
+var q = require("q");
 var sdk = require("../..");
 var HttpBackend = require("../mock-request");
 var utils = require("../test-utils");
@@ -66,6 +67,11 @@ describe("MatrixClient retrying", function() {
         ev2 = tl[1];
 
         expect(ev1.status).toEqual(EventStatus.SENDING);
+        expect(ev2.status).toEqual(EventStatus.SENDING);
+
+        // give the reactor a chance to run, so that ev2 gets queued
+        q().then(function() {
+            // ev2 should now have been queued
             expect(ev2.status).toEqual(EventStatus.QUEUED);
 
             // now we can cancel the second and check everything looks sane
@@ -80,7 +86,8 @@ describe("MatrixClient retrying", function() {
             // fail the first send
             httpBackend.when("PUT", "/send/m.room.message/")
                 .respond(400);
-        httpBackend.flush().then(function() {
+            return httpBackend.flush();
+        }).then(function() {
             expect(ev1.status).toEqual(EventStatus.NOT_SENT);
             expect(tl.length).toEqual(1);