Merge remote-tracking branch 'origin/develop' into release-1.0.0

.buildkite/pipeline.yml

@@ -37,7 +37,7 @@ steps:
     branches: "!master !develop !release-*"
     command:
       - "python3 -m pip install towncrier"
-      - "python3 -m towncrier.check"
+      - "python3 -m towncrier.check --compare-with=origin/develop"
     plugins:
       - docker#v3.0.1:
           image: "python:3.6"

changelog.d/239.misc

@@ -0,0 +1 @@
+Update datastore.md with a few more options

changelog.d/251.misc

@@ -0,0 +1 @@
+Fix issue where towncrier would wrongly link to matrix-appservice-slack-issues

changelog.d/253.bugfix

@@ -0,0 +1 @@
+Fix multi-person DMs being marked with the group (private channel) type rather than the mpim type.

changelog.d/254.bugfix

@@ -0,0 +1 @@
+Connecting an account via OAuth will no longer barf on the lack of a `puppeting` parameter

changelog.d/255.bugfix

@@ -0,0 +1 @@
+Don't log stack traces for missing rooms, teams or events

changelog.d/256.bugfix

@@ -0,0 +1 @@
+Don't log the whole response object when an error occurs when sending slack requests

changelog.d/257.bugfix

@@ -0,0 +1 @@
+Fix .toUpperCase() errors due to the bridge trying to handle unknown deleted messages

changelog.d/258.misc

@@ -0,0 +1 @@
+Towncrier should check against develop for changelog changes

docs/datastores.md

@@ -46,12 +46,13 @@ Migrating from an existing NeDB installation
 
 ```bash
 npm run build
-node lib/scripts/migrateToPostgres.js "connectionString" "dbdir"
+node lib/scripts/migrateToPostgres.js "connectionString" "dbDir" "slackPrefix"
 ```
 
-Where you should replace `connectionString` with the value above (such as
-`postgresql://slackbridge_user:somethingverysecret@localhost/slack_bridge?sslmode=require`), and `dbdir`
-*if* you stored your data files in a custom location.
+Where you should replace:
+- `connectionString` with the value above (such as `postgresql://slackbridge_user:somethingverysecret@localhost/slack_bridge?sslmode=require`)
+- `dbDir` with the absolute path to your data files
+- `slackPrefix` with the prefix of your slack ghost users (e.g. "@slack_")
 
 Once this process has completed and no errors have occured, you may begin using
 your brand new PostgreSQL database.

pyproject.toml

@@ -2,7 +2,7 @@
     # The name of your Python package
     filename = "CHANGELOG.md"
     directory = "changelog.d"
-    issue_format = "[\\#{issue}](https://github.com/matrix-org/matrix-appservice-slack-issues/{issue})"
+    issue_format = "[\\#{issue}](https://github.com/matrix-org/matrix-appservice-slack/issues/{issue})"
 
     [[tool.towncrier.type]]
         directory = "feature"
@@ -27,4 +27,4 @@
     [[tool.towncrier.type]]
         directory = "misc"
         name = "Internal Changes"
-        showcontent = true
+        showcontent = true

src/BridgedRoom.ts

@@ -183,10 +183,11 @@ export class BridgedRoom {
         this.setValue("isPrivate", chan.is_private);
         if (chan.is_channel) {
             this.setValue("slackType", "channel");
-        } else if (chan.is_group) {
-            this.setValue("slackType", "group");
         } else if (chan.is_mpim) {
+            // note: is_group is also set for mpims, so order is important
             this.setValue("slackType", "mpim");
+        } else if (chan.is_group) {
+            this.setValue("slackType", "group");
         } else if (chan.is_im) {
             this.setValue("slackType", "im");
         } else {
@@ -275,7 +276,7 @@ export class BridgedRoom {
         log.info(`Reaction :${emojiKeyName}: added to ${event.slackTs}`);
 
         if (!res.ok) {
-            log.error("HTTP Error: ", res);
+            log.error("HTTP Error: ", res.error);
             return;
         }
         // TODO: Add this event to the event store
@@ -300,8 +301,9 @@ export class BridgedRoom {
             ts: event.slackTs,
         });
 
-        if (!res) {
-            log.error("HTTP Error: ", res);
+        if (!res.ok) {
+            log.error("HTTP Error: ", res.error);
+            return;
         }
         return res;
     }
@@ -333,8 +335,8 @@ export class BridgedRoom {
         })) as ChatUpdateResponse;
 
         this.main.incCounter(METRIC_SENT_MESSAGES, {side: "remote"});
-        if (!res) {
-            log.error("HTTP Error: ", res);
+        if (!res.ok) {
+            log.error("HTTP Error: ", res.error);
             return;
         }
         // Add this event to the event store
@@ -412,7 +414,7 @@ export class BridgedRoom {
         this.main.incCounter(METRIC_SENT_MESSAGES, {side: "remote"});
 
         if (!res.ok) {
-            log.error("HTTP Error: ", res);
+            log.error("HTTP Error: ", res.error);
             return;
         }
 

src/Main.ts

@@ -940,7 +940,7 @@ export class Main {
 
             const infoRes = (await slackClient.conversations.info({ channel: opts.slack_channel_id})) as ConversationsInfoResponse;
             if (!infoRes.ok) {
-                log.error(`conversations.info for ${opts.slack_channel_id} errored:`, infoRes);
+                log.error(`conversations.info for ${opts.slack_channel_id} errored:`, infoRes.error);
                 throw Error("Failed to get channel info");
             }
             room.setBotClient(slackClient);

src/OAuth2.ts

@@ -60,7 +60,7 @@ export class OAuth2 {
 
     public makeAuthorizeURL(room: string|BridgedRoom, state: string, isPuppeting: boolean = false): string {
         const redirectUri = this.makeRedirectURL(room);
-        const scopes = isPuppeting ? REQUIRED_SCOPES : PUPPET_SCOPES;
+        const scopes = isPuppeting ? PUPPET_SCOPES : REQUIRED_SCOPES;
 
         const qs = querystring.stringify({
             client_id: this.clientId,

src/Provisioning.ts

@@ -26,7 +26,7 @@ const log = Logging.get("Provisioning");
 type CommandFunc = (main: Main, req: Request, res: Response, ...params: string[]) => void|Promise<void>;
 export const commands: {[verb: string]: Command} = {};
 
-type Param = string;
+type Param = string | { param: string, required: boolean};
 
 export class Command {
     private params: Param[];
@@ -40,12 +40,14 @@ export class Command {
         const body = req.body;
         const args: [Main, Request, Response, ...string[]] = [main, req, res];
         for (const param of this.params) {
-            if (!(param in body)) {
+            const paramName = typeof(param) === "string" ? param : param.param;
+            const paramRequired = typeof(param) === "string" ? true : param.required;
+            if (!(paramName in body) && paramRequired) {
                 res.status(HTTP_CODES.CLIENT_ERROR).json({error: `Required parameter ${param} missing`});
                 return;
             }
 
-            args.push(body[param]);
+            args.push(body[paramName]);
         }
 
         try {
@@ -92,7 +94,7 @@ commands.getbotid = new Command({
 });
 
 commands.authurl = new Command({
-    params: ["user_id", "puppeting"],
+    params: ["user_id", { param: "puppeting", required: false}],
     func(main, req, res, userId, puppeting) {
         if (!main.oauth2) {
             res.status(HTTP_CODES.CLIENT_ERROR).json({
@@ -155,7 +157,7 @@ commands.channels = new Command({
             types: "public_channel", // TODO: In order to show private channels, we need the identity of the caller.
         })) as ConversationsListResponse;
         if (!response.ok) {
-            log.error(`Failed trying to fetch channels for ${teamId}.`, response);
+            log.error(`Failed trying to fetch channels for ${teamId}.`, response.error);
             res.status(HTTP_CODES.SERVER_ERROR).json({error: "Failed to fetch channels"});
             return;
         }

src/SlackEventHandler.ts

@@ -75,7 +75,7 @@ export class SlackEventHandler extends BaseSlackHandler {
             // We must respond within 3 seconds or it will be sent again!
             response(HTTP_OK, "OK");
 
-            let err: string|null = null;
+            let err: Error|null = null;
             try {
                 switch (event.type) {
                     case "message":
@@ -97,36 +97,39 @@ export class SlackEventHandler extends BaseSlackHandler {
                     // XXX: Unused?
                     case "file_comment_added":
                     default:
-                        err = "unknown_event";
+                        err = Error("unknown_event");
                 }
             } catch (ex) {
                 log.warn("Didn't handle event");
                 err = ex;
             }
 
-            if (err === "unknown_channel") {
+            if (err === null) {
+                endTimer({outcome: "success"});
+            } else if (!(err instanceof Error)) {
+                log.warn("Error when handing event:", err);
+                endTimer({outcome: "fail"});
+            } else if (err.message === "unknown_channel") {
                 const chanIdMix = `${event.channel} (${teamId})`;
                 log.warn(`Ignoring message from unrecognised slack channel id: ${chanIdMix}`);
                 this.main.incCounter("received_messages", {side: "remote"});
                 endTimer({outcome: "dropped"});
                 return;
-            } else if (err === "unknown_team") {
+            } else if (err.message === "unknown_team") {
                 log.warn(`Ignoring message from unrecognised slack team id: ${teamId}`);
                 this.main.incCounter("received_messages", {side: "remote"});
                 endTimer({outcome: "dropped"});
                 return;
-            } else if (err === "unknown_event") {
+            } else if (err.message === "unknown_message") {
+                log.warn(`Ignoring event because we couldn't find a referred to message`);
+                endTimer({outcome: "dropped"});
+                return;
+            } else if (err.message === "unknown_event") {
                 endTimer({outcome: "dropped"});
-            } else if (err !== null) {
+            } else {
                 log.warn("Error when handing event:", err);
                 endTimer({outcome: "fail"});
             }
-
-            if (err === null) {
-                endTimer({outcome: "success"});
-            } else {
-                log.error("Failed to handle slack event:", err);
-            }
         } catch (e) {
             log.error("SlackEventHandler.handle failed:", e);
         }
@@ -214,6 +217,8 @@ export class SlackEventHandler extends BaseSlackHandler {
                 const botClient = this.main.botIntent.getClient();
                 return botClient.redactEvent(originalEvent.roomId, originalEvent.eventId);
             }
+            // If we don't have the event
+            throw Error("unknown_message");
         } else if (msg.subtype === "message_replied") {
             // Slack sends us one of these as well as a normal message event
             // when using RTM, so we ignore it.

src/SlackGhost.ts

@@ -136,21 +136,26 @@ export class SlackGhost {
     public async getBotName(botId: string, client: WebClient) {
         const response = (await client.bots.info({ bot: botId})) as BotsInfoResponse;
         if (!response.ok || !response.bot.name) {
-            log.error("Failed to get bot name", response);
+            log.error("Failed to get bot name", response.error);
             return;
         }
         return response.bot.name;
     }
 
     public async getBotAvatarUrl(botId: string, client: WebClient) {
         const response = (await client.bots.info({ bot: botId})) as BotsInfoResponse;
-        if (!response.ok || !response.bot.icons.image_72) {
-            log.error("Failed to get bot name", response);
+        if (!response.ok) {
+            log.error("Failed to get bot name", response.error);
             return;
         }
         const icons = response.bot.icons;
-        return icons.image_original || icons.image_1024 || icons.image_512 ||
+        const icon = icons.image_original || icons.image_1024 || icons.image_512 ||
             icons.image_192 || icons.image_72 || icons.image_48;
+        if (!icon) {
+            log.error("No suitable icon for bot");
+            return;
+        }
+        return icon;
     }
 
     public async lookupUserInfo(client: WebClient) {

src/SlackHookHandler.ts

@@ -391,7 +391,7 @@ export class SlackHookHandler extends BaseSlackHandler {
         })) as ConversationsHistoryResponse;
 
         if (!response.ok || !response.messages || response.messages.length === 0) {
-            log.warn("Could not find history: " + response);
+            log.warn("Could not find history: " + response.error);
             throw Error("Could not find history");
         }
         if (response.messages.length !== 1) {