diff --git a/config/config.sample.yaml b/config/config.sample.yaml
index 10f6d2fb97e7f9ca24fff97f962683894245c11b..510195fdf0fe34bd91e1fc56ac3357d7d2bc9afd 100644
--- a/config/config.sample.yaml
+++ b/config/config.sample.yaml
@@ -85,10 +85,12 @@ channel:
limits:
# Delay in milliseconds between discord users joining a room.
roomGhostJoinDelay: 6000
- # Delay in milliseconds before sending messages to discord to avoid echos.
- # (Copies of a sent message may arrive from discord before we've
+ # Lock timeout in milliseconds before seinding messages to discord to avoid
+ # echos. Default is rather high as the lock will most likely time out
+ # before anyways.
+ # echos = (Copies of a sent message may arrive from discord before we've
# fininished handling it, causing us to echo it back to the room)
- discordSendDelay: 750
+ discordSendDelay: 1500
ghosts:
# Pattern for the ghosts nick, available is :nick, :username, :tag and :id
nickPattern: ":nick"
diff --git a/src/bot.ts b/src/bot.ts
index 22791a07b812ff8983d86de5863642d1b3def0ff..0990a3e23e05b4ea9ce19f5146bd47e9b083714f 100644
--- a/src/bot.ts
+++ b/src/bot.ts
@@ -38,6 +38,7 @@ import * as mime from "mime";
import { IMatrixEvent, IMatrixMediaInfo } from "./matrixtypes";
import { DiscordCommandHandler } from "./discordcommandhandler";
import { MetricPeg } from "./metrics";
+import { Lock } from "./structures/lock";
const log = new Log("DiscordBot");
@@ -82,8 +83,7 @@ export class DiscordBot {
/* Handles messages queued up to be sent to matrix from discord. */
private discordMessageQueue: { [channelId: string]: Promise<void> };
- private channelLocks: Map<string, {i: NodeJS.Timeout|null, r: (() => void)|null}>;
- private channelLockPromises: Map<string, Promise<{}>>;
+ private channelLock: Lock<string>;
constructor(
private botUserId: string,
private config: DiscordBridgeConfig,
@@ -107,8 +107,7 @@ export class DiscordBot {
// init vars
this.sentMessages = [];
this.discordMessageQueue = {};
- this.channelLocks = new Map();
- this.channelLockPromises = new Map();
+ this.channelLock = new Lock(this.config.limits.discordSendDelay);
this.lastEventIds = {};
}
@@ -140,43 +139,6 @@ export class DiscordBot {
return this.provisioner;
}
- public lockChannel(channel: Discord.Channel) {
- if (this.channelLocks.has(channel.id)) {
- return;
- }
-
- this.channelLocks[channel.id] = {i: null, r: null, p: null};
- const p = new Promise<{}>((resolve) => {
- const i = setTimeout(() => {
- log.warn(`Lock on channel ${channel.id} expired. Discord is lagging behind?`);
- this.unlockChannel(channel);
- }, this.config.limits.discordSendDelay);
- const o = Object.assign({r: resolve, i, p: null}, this.channelLocks.get(channel.id) || {});
- this.channelLocks.set(channel.id, o);
- });
- this.channelLockPromises.set(channel.id, p);
- }
-
- public unlockChannel(channel: Discord.Channel) {
- if (!this.channelLocks.has(channel.id)) {
- return;
- }
- const lock = this.channelLocks.get(channel.id)!;
- if (lock.i !== null) {
- lock.r!();
- clearTimeout(lock.i);
- }
- this.channelLocks.delete(channel.id);
- this.channelLockPromises.delete(channel.id);
- }
-
- public async waitUnlock(channel: Discord.Channel) {
- const promise = this.channelLockPromises.get(channel.id);
- if (promise) {
- await promise;
- }
- }
-
public GetIntentFromDiscordMember(member: Discord.GuildMember | Discord.User, webhookID?: string): Intent {
if (webhookID) {
// webhookID and user IDs are the same, they are unique, so no need to prefix _webhook_
@@ -240,7 +202,7 @@ export class DiscordBot {
client.on("messageDelete", async (msg: Discord.Message) => {
try {
- await this.waitUnlock(msg.channel);
+ await this.channelLock.wait(msg.channel.id);
this.clientFactory.bindMetricsToChannel(msg.channel as Discord.TextChannel);
this.discordMessageQueue[msg.channel.id] = (async () => {
await (this.discordMessageQueue[msg.channel.id] || Promise.resolve());
@@ -261,7 +223,7 @@ export class DiscordBot {
msgs.forEach((msg) => {
promiseArr.push(async () => {
try {
- await this.waitUnlock(msg.channel);
+ await this.channelLock.wait(msg.channel.id);
this.clientFactory.bindMetricsToChannel(msg.channel as Discord.TextChannel);
await this.DeleteDiscordMessage(msg);
} catch (err) {
@@ -276,7 +238,7 @@ export class DiscordBot {
});
client.on("messageUpdate", async (oldMessage: Discord.Message, newMessage: Discord.Message) => {
try {
- await this.waitUnlock(newMessage.channel);
+ await this.channelLock.wait(newMessage.channel.id);
this.clientFactory.bindMetricsToChannel(newMessage.channel as Discord.TextChannel);
this.discordMessageQueue[newMessage.channel.id] = (async () => {
await (this.discordMessageQueue[newMessage.channel.id] || Promise.resolve());
@@ -293,7 +255,7 @@ export class DiscordBot {
client.on("message", async (msg: Discord.Message) => {
try {
MetricPeg.get.registerRequest(msg.id);
- await this.waitUnlock(msg.channel);
+ await this.channelLock.wait(msg.channel.id);
this.clientFactory.bindMetricsToChannel(msg.channel as Discord.TextChannel);
this.discordMessageQueue[msg.channel.id] = (async () => {
await (this.discordMessageQueue[msg.channel.id] || Promise.resolve());
@@ -417,10 +379,10 @@ export class DiscordBot {
if (!msg) {
return;
}
- this.lockChannel(channel);
+ this.channelLock.set(channel.id);
const res = await channel.send(msg);
await this.StoreMessagesSent(res, channel, event);
- this.unlockChannel(channel);
+ this.channelLock.release(channel.id);
}
public async send(
@@ -451,7 +413,7 @@ export class DiscordBot {
}
}
try {
- this.lockChannel(chan);
+ this.channelLock.set(chan.id);
if (!botUser) {
// NOTE: Don't send replies to discord if we are a puppet.
msg = await chan.send(embed.description, opts);
@@ -473,7 +435,7 @@ export class DiscordBot {
}
// Don't block on this.
this.StoreMessagesSent(msg, chan, event).then(() => {
- this.unlockChannel(chan);
+ this.channelLock.release(chan.id);
}).catch(() => {
log.warn("Failed to store sent message for ", event.event_id);
});
@@ -504,9 +466,9 @@ export class DiscordBot {
const msg = await chan.fetchMessage(storeEvent.DiscordId);
try {
- this.lockChannel(msg.channel);
+ this.channelLock.set(msg.channel.id);
await msg.delete();
- this.unlockChannel(msg.channel);
+ this.channelLock.release(msg.channel.id);
log.info(`Deleted message`);
} catch (ex) {
log.warn(`Failed to delete message`, ex);
@@ -658,12 +620,12 @@ export class DiscordBot {
/* tslint:disable-next-line no-any */
} as any, // XXX: Discord.js typings are wrong.
`Unbanned.`);
- this.lockChannel(botChannel);
+ this.channelLock.set(botChannel.id);
res = await botChannel.send(
`${kickee} was unbanned from this channel by ${kicker}.`,
) as Discord.Message;
this.sentMessages.push(res.id);
- this.unlockChannel(botChannel);
+ this.channelLock.release(botChannel.id);
return;
}
const existingPerms = tchan.memberPermissions(kickee);
@@ -672,13 +634,13 @@ export class DiscordBot {
return;
}
const word = `${kickban === "ban" ? "banned" : "kicked"}`;
- this.lockChannel(botChannel);
+ this.channelLock.set(botChannel.id);
res = await botChannel.send(
`${kickee} was ${word} from this channel by ${kicker}.`
+ (reason ? ` Reason: ${reason}` : ""),
) as Discord.Message;
this.sentMessages.push(res.id);
- this.unlockChannel(botChannel);
+ this.channelLock.release(botChannel.id);
log.info(`${word} ${kickee}`);
await tchan.overwritePermissions(kickee,
diff --git a/src/config.ts b/src/config.ts
index 2da5b7734e721eca3d77126750cf16957349deb4..9d966b544b8f4dee85f80121e30da15c1bec477b 100644
--- a/src/config.ts
+++ b/src/config.ts
@@ -133,7 +133,7 @@ export class DiscordBridgeConfigChannelDeleteOptions {
class DiscordBridgeConfigLimits {
public roomGhostJoinDelay: number = 6000;
- public discordSendDelay: number = 750;
+ public discordSendDelay: number = 1500;
}
export class LoggingFile {
diff --git a/src/structures/lock.ts b/src/structures/lock.ts
new file mode 100644
index 0000000000000000000000000000000000000000..f400aab092a3a6f49381d9c47b43112d0500be9d
--- /dev/null
+++ b/src/structures/lock.ts
@@ -0,0 +1,60 @@
+export class Lock<T> {
+ private locks: Map<T, {i: NodeJS.Timeout|null, r: (() => void)|null}>;
+ private lockPromises: Map<T, Promise<{}>>;
+ constructor(
+ private timeout: number,
+ ) {
+ this.locks = new Map();
+ this.lockPromises = new Map();
+ }
+
+ public set(key: T) {
+ // if there is a lock set.....we don't set a second one ontop
+ if (this.locks.has(key)) {
+ return;
+ }
+
+ // set a dummy lock so that if we re-set again before releasing it won't do anthing
+ this.locks.set(key, {i: null, r: null});
+
+ const p = new Promise<{}>((resolve) => {
+ // first we check if the lock has the key....if not, e.g. if it
+ // got released too quickly, we still want to resolve our promise
+ if (!this.locks.has(key)) {
+ resolve();
+ return;
+ }
+ // create the interval that will release our promise after the timeout
+ const i = setTimeout(() => {
+ this.release(key);
+ }, this.timeout);
+ // aaand store to our lock
+ this.locks.set(key, {r: resolve, i});
+ });
+ this.lockPromises.set(key, p);
+ }
+
+ public release(key: T) {
+ // if there is nothing to release then there is nothing to release
+ if (!this.locks.has(key)) {
+ return;
+ }
+ const lock = this.locks.get(key)!;
+ if (lock.r !== null) {
+ lock.r();
+ }
+ if (lock.i !== null) {
+ clearTimeout(lock.i);
+ }
+ this.locks.delete(key);
+ this.lockPromises.delete(key);
+ }
+
+ public async wait(key: T) {
+ // we wait for a lock release only if a promise is present
+ const promise = this.lockPromises.get(key);
+ if (promise) {
+ await promise;
+ }
+ }
+}
diff --git a/test/structures/test_lock.ts b/test/structures/test_lock.ts
new file mode 100644
index 0000000000000000000000000000000000000000..114ddb9065f0f11a9acb288b43f71c58227ba827
--- /dev/null
+++ b/test/structures/test_lock.ts
@@ -0,0 +1,45 @@
+/*
+Copyright 2019 matrix-appservice-discord
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { expect } from "chai";
+import { Lock } from "../../src/structures/lock";
+import { Util } from "../../src/util";
+
+const LOCKTIMEOUT = 300;
+
+describe("Lock", () => {
+ it("should lock and unlock", async () => {
+ const lock = new Lock<string>(LOCKTIMEOUT);
+ const t = Date.now();
+ lock.set("bunny");
+ await lock.wait("bunny");
+ const diff = Date.now() - t;
+ expect(diff).to.be.greaterThan(LOCKTIMEOUT - 1);
+ });
+ it("should lock and unlock early, if unlocked", async () => {
+ const SHORTDELAY = 100;
+ const DELAY_ACCURACY = 5;
+ const lock = new Lock<string>(LOCKTIMEOUT);
+ setTimeout(() => lock.release("fox"), SHORTDELAY);
+ const t = Date.now();
+ lock.set("fox");
+ await lock.wait("fox");
+ const diff = Date.now() - t;
+ // accuracy can be off by a few ms soemtimes
+ expect(diff).to.be.greaterThan(SHORTDELAY - DELAY_ACCURACY);
+ expect(diff).to.be.lessThan(SHORTDELAY + DELAY_ACCURACY);
+ });
+});
diff --git a/test/test_discordbot.ts b/test/test_discordbot.ts
index 918ff46863b19b6f70a106d96228576ec1cab580..2cfa4cacbe818f0a03577e83d292aba6f38cdf7c 100644
--- a/test/test_discordbot.ts
+++ b/test/test_discordbot.ts
@@ -416,48 +416,6 @@ describe("DiscordBot", () => {
assert.equal(expected, ITERATIONS);
});
});
- describe("locks", () => {
- it("should lock and unlock a channel", async () => {
- const bot = new modDiscordBot.DiscordBot(
- "",
- config,
- mockBridge,
- {},
- ) as DiscordBot;
- const chan = new MockChannel("123") as any;
- const t = Date.now();
- bot.lockChannel(chan);
- await bot.waitUnlock(chan);
- const diff = Date.now() - t;
- expect(diff).to.be.greaterThan(config.limits.discordSendDelay - 1);
- });
- it("should lock and unlock a channel early, if unlocked", async () => {
- const discordSendDelay = 500;
- const SHORTDELAY = 100;
- const MINEXPECTEDDELAY = 95;
- const bot = new modDiscordBot.DiscordBot(
- "",
- {
- bridge: {
- domain: "localhost",
- },
- limits: {
- discordSendDelay,
- },
- },
- mockBridge,
- {},
- ) as DiscordBot;
- const chan = new MockChannel("123") as any;
- setTimeout(() => bot.unlockChannel(chan), SHORTDELAY);
- const t = Date.now();
- bot.lockChannel(chan);
- await bot.waitUnlock(chan);
- const diff = Date.now() - t;
- // Date accuracy can be off by a few ms sometimes.
- expect(diff).to.be.greaterThan(MINEXPECTEDDELAY);
- });
- });
// });
// describe("ProcessMatrixMsgEvent()", () => {
//