From dafd47a7bb55c68dd70f5e25ad26e5c5d1857d56 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 22 Oct 2020 11:41:20 +0100 Subject: [PATCH 1/3] Build before testing --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index fa156b61..57d9ad90 100644 --- a/package.json +++ b/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" From 6c4a2355c89806c85dbc99b6843ec40a1e69cd0a Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 22 Oct 2020 11:41:31 +0100 Subject: [PATCH 2/3] Remove pointless onlyCheck --- src/GatewayHandler.ts | 4 +--- src/bifrost/Events.ts | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/GatewayHandler.ts b/src/GatewayHandler.ts index b96aaef0..4ef0bb58 100644 --- a/src/GatewayHandler.ts +++ b/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")); diff --git a/src/bifrost/Events.ts b/src/bifrost/Events.ts index 324a8cac..cbaa2960 100644 --- a/src/bifrost/Events.ts +++ b/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 { From 685ab22fffa25bb7a53ea6895c9f1ea582f4cce0 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 22 Oct 2020 11:41:56 +0100 Subject: [PATCH 3/3] Add support for custom error text --- changelog.d/178.misc | 1 + src/xmppjs/ServiceHandler.ts | 10 +++++----- src/xmppjs/Stanzas.ts | 12 +++++++++--- test/xmppjs/test_Stanzas.ts | 12 +++++++++++- 4 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 changelog.d/178.misc diff --git a/changelog.d/178.misc b/changelog.d/178.misc new file mode 100644 index 00000000..1756875d --- /dev/null +++ b/changelog.d/178.misc @@ -0,0 +1 @@ +Some errors now report helpful error text diff --git a/src/xmppjs/ServiceHandler.ts b/src/xmppjs/ServiceHandler.ts index bcae3807..3f6b9254 100644 --- a/src/xmppjs/ServiceHandler.ts +++ b/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 { + private queryRoom(roomAlias: string): Promise { 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")); } } diff --git a/src/xmppjs/Stanzas.ts b/src/xmppjs/Stanzas.ts index 37b1945c..14e90de1 100644 --- a/src/xmppjs/Stanzas.ts +++ b/src/xmppjs/Stanzas.ts @@ -143,6 +143,7 @@ export class StzaPresenceError extends StzaPresence { public by: string, public errType: string, public innerError: string, + public text?: string, ) { super(from, to, id, "error"); @@ -150,7 +151,9 @@ export class StzaPresenceError extends StzaPresence { public get presenceContent() { return `<${this.innerError}` - + ` xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>`; + + " xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>" + + (this.text ? `${he.encode(this.text)}` : "") + + ""; } } @@ -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 `` + `<${this.innerError} ` + - "xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>"; + "xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>" + + (this.text ? `${he.encode(this.text)}` : "") + + ""; } } diff --git a/test/xmppjs/test_Stanzas.ts b/test/xmppjs/test_Stanzas.ts index 2531f271..ed54fc36 100644 --- a/test/xmppjs/test_Stanzas.ts +++ b/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( + "" + + "" + + "Something isn't right" + + "", + ); + }); });