Add support for helpful error text in stanzas

changelog.d/178.misc

@@ -0,0 +1 @@
+Some errors now report helpful error text

package.json

@@ -9,6 +9,7 @@
     "lint": "tslint --project tsconfig.json --format stylish",
     "start": "node build/src/Program.js -c config.yaml",
     "genreg": "node build/src/Program.js -r -c config.yaml",
+    "pretest": "npm run build",
     "test": "mocha -r ts-node/register test/test.ts test/*.ts test/**/*.ts",
     "changelog": "scripts/towncrier.sh",
     "coverage": "nyc mocha -r ts-node/register test/test.ts test/*.ts test/**/*.ts"

src/GatewayHandler.ts

@@ -220,9 +220,7 @@ export class GatewayHandler {
         try {
             const res = await this.bridge.getIntent().getClient().resolveRoomAlias(ev.roomAlias);
             log.info(`Found ${res.room_id}`);
-            if (ev.onlyCheck) {
-                ev.result(null, res.room_id);
-            }
+            ev.result(null, res.room_id);
         } catch (ex) {
             log.warn("Room not found:", ex);
             ev.result(Error("Room not found"));

src/bifrost/Events.ts

@@ -85,8 +85,7 @@ export interface IGatewayRequest {
 }
 
 export interface IGatewayRoomQuery extends IGatewayRequest {
-    onlyCheck: boolean;
-    result: (err: Error|null, res?: string|IGatewayRoom) => void;
+    result: (err: Error|null, res?: string) => void;
 }
 
 export interface IGatewayPublicRoomsQuery extends IGatewayRequest {

src/xmppjs/ServiceHandler.ts

@@ -220,7 +220,8 @@ export class ServiceHandler {
             log.warn(`Failed to search rooms: ${ex}`);
             // XXX: There isn't a very good way to explain why it failed,
             // so we use service unavailable.
-            await this.xmpp.xmppSend(new SztaIqError(from, to, id, "cancel", 503, "service-unavailable"));
+            await this.xmpp.xmppSend(new SztaIqError(from, to, id, "cancel", 503, "service-unavailable", undefined,
+                `Failure fetching public rooms from ${homeserver}`));
             return;
         }
 
@@ -237,11 +238,10 @@ export class ServiceHandler {
         await this.xmpp.xmppSend(response);
     }
 
-    private queryRoom(roomAlias: string, onlyCheck: boolean = false): Promise<string|IGatewayRoom> {
+    private queryRoom(roomAlias: string): Promise<string|IGatewayRoom> {
         return new Promise((resolve, reject) => {
             this.xmpp.emit("gateway-queryroom", {
                 roomAlias,
-                onlyCheck,
                 result: (err, res) => {
                     if (err) {
                         reject(err);
@@ -262,7 +262,7 @@ export class ServiceHandler {
             log.debug(`Running room discovery for ${toStr}`);
             let roomId = this.existingAliases.get(alias);
             if (!roomId) {
-                roomId = await this.queryRoom(alias, true) as string;
+                roomId = await this.queryRoom(alias) as string;
                 this.existingAliases.set(alias, roomId);
             }
             log.info(`Response for alias request ${toStr} (${alias}) -> ${roomId}`);
@@ -283,7 +283,7 @@ export class ServiceHandler {
             });
             await this.xmpp.xmppSend(discoInfo);
         } catch (ex) {
-            await this.xmpp.xmppSend(new SztaIqError(toStr, from, id, "cancel", 404, "item-not-found"));
+            await this.xmpp.xmppSend(new SztaIqError(toStr, from, id, "cancel", 404, "item-not-found", undefined, "Room could not be found"));
         }
     }
 

src/xmppjs/Stanzas.ts

@@ -143,14 +143,17 @@ export class StzaPresenceError extends StzaPresence {
         public by: string,
         public errType: string,
         public innerError: string,
+        public text?: string,
 
     ) {
         super(from, to, id, "error");
     }
 
     public get presenceContent() {
         return `<error type='${this.errType}' by='${this.by}'><${this.innerError}`
-             + ` xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>`;
+             + " xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>"
+             + (this.text ? `<text>${he.encode(this.text)}</text>` : "")
+             + "</error>";
     }
 }
 
@@ -415,7 +418,8 @@ export class SztaIqError extends StzaBase {
         private errorType: string,
         private errorCode: number|null,
         private innerError: string,
-        private by: string|null = null,
+        private by?: string,
+        private text?: string,
     ) {
         super(from, to, id);
     }
@@ -434,7 +438,9 @@ export class SztaIqError extends StzaBase {
         }
         return `<iq from='${this.from}' to='${this.to}' id='${this.id}' type='error' xml:lang='en'>`
         + `<error type='${this.errorType}'${errorParams}><${this.innerError} ` +
-          "xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error></iq>";
+          "xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>" +
+          (this.text ? `<text>${he.encode(this.text)}</text>` : "") +
+          "</error></iq>";
     }
 }
 

test/xmppjs/test_Stanzas.ts

@@ -94,7 +94,7 @@ describe("Stanzas", () => {
         });
     });
     describe("SztaIqError", () => {
-        it("should create a valid stanza", () => {
+        it("should create a an error", () => {
             const xml = new SztaIqError("foo@bar", "baz@bar", "someid", "cancel", null, "not-acceptable", "foo").xml;
             assertXML(xml);
             expect(xml).to.equal(
@@ -104,4 +104,14 @@ describe("Stanzas", () => {
             );
         });
     });
+    it("should create a an error with custom text", () => {
+        const xml = new SztaIqError("foo@bar", "baz@bar", "someid", "cancel", null, "not-acceptable", "foo", "Something isn't right").xml;
+        assertXML(xml);
+        expect(xml).to.equal(
+            "<iq from='foo@bar' to='baz@bar' id='someid' type='error' xml:lang='en'>" +
+             "<error type='cancel' by='foo'><not-acceptable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>" +
+             "<text>Something isn&#x27;t right</text>" +
+             "</error></iq>",
+        );
+    });
 });